add a select attribute on tabs pane to trigger a callback

[revolunet]

Hi :)

I needed to have a callback on tab selection so i added a select attribute on the tabs pane directive.

eg: <pane select="updateImage(tab.thumbnail)">

Comments appreciated.

Thanks !

Julien

[pkozlowski-opensource]

@revolunet thnx for the PR, it looks good. Before merging it I would like to ask if you saw that tabs got the active property that you could $watch and execute any logic when the watched value becomes true. Would this work for your use case?

[revolunet]

@pkozlowski-opensource i dont have any idea how i could watch easily the pane.active property for each pane of the tabs as the active attribute is related to pane and not tabs. i would set multiple watchers ?

in my case, i'm in a ng-repeat :

<pane ng-repeat="tab in tabs" heading="{{ tab.title }}" active="tab.active" select="changeThumbnail(tab.thumbnail)">...</pane>

Also, can you please explain why heading attribute have double curly braces syntax and not active attribute ? looks like inconsistency to me, isnt it ?

thanks :)

[pkozlowski-opensource]

@revolunet yeh, you are right, I was playing with the $watch solution and although I've managed to make it work it is ugly...

Will squash commits and merge it unless others got different idea...

[revolunet]

thanks :)

fix me please : why that different syntax between heading and active attributes ?
(example copied from your demo page)

[pkozlowski-opensource]

Regarding different syntax: {{ }} are used for interpolation, so in the cases where you want to have several dynamic parts evaluated withing one attribute. It allows you to write heading="My heading {{$index}}" instead of heading="'My heading ' +$index". This is the same syntax used by ngSrc for example.

The syntax without curly is used where there is only one value to evaluate. This is typically used for non-string values (boolean, event handlers etc.). So this "inconsistency" exists in AngularJS already and is about offering the most user-friendly syntax.

[revolunet]

ok thanks for clarification :)
so i f i understand well, both attributes behave the same and we could use header="tab.title" without curly braces ?

[darwin]

I have an alternative proposal:

• id of active pane should be a bindable variable on tabs directive. Say active-id (optional)
• pane ids may be defined on pane directives via pane-id (optional)
• by default each pane will get numeric id (ordering in parent tabs)

    <tabs active-id="model.activePaneId">
        <pane heading="Static title" pane-id="static-pane">Static content</pane> <!-- pane-id == "static-pane" -->
        <pane heading="Static title2">Static content</pane> <!-- pane-id == 1 -->
        <pane ng-repeat="pane in panes" pane-id="{{pane.id}}" heading="{{pane.title}}" active="pane.active">{{pane.content}}</pane>
    </tabs>

I believe this is more flexible. @revolunet's solution works fine for ng-repeat but must be repeated manually for static panes. Also his approach does not support $watch and binding. His example would reduce to active-id="something" and then setting $watch on something. When using default index-based ids, he can lookup active pane directly with the index. In more complex scenarios it is up to the user to handle mapping between ids and panes. Alternatively more complex data structure could be stored in pane-id.

If you agree I could try to implement it. I need it for my code anyway. Right now I have to do this in my controllers:

  $scope.findActivePane = ->
    _.find @panes, (pane) -> pane.active

  checkIfActivePaneHasChanged = ->
    activePane = $scope.findActivePane()
    return false unless activePane
    dirty = activePane.id isnt $scope.activePaneId
    $scope.activePaneId = activePane.id if dirty
    dirty

  $scope.$watch checkIfActivePaneHasChanged, ->
    $location.path "/settings/#{$scope.activePaneId}"

[pkozlowski-opensource]

@darwin I agree that it is a bit more flexible but AngularJS gives as so much more ability to work with objects as compared to relaying on ids that I almost never use them in AngularJS web apps.

Couldn't we do something like this in a pane:

<pane on-select="myexpression"> where the expression would be evaluated when a pane is selected. Then, in this expression you could do either myfunction('customId') or myfunction($index) (if used with ngRepeat) or even activeTab = 'myId' + $index. Should be really easy to implement with $parse.

How does it sound?

[darwin]

Looks good. But I still think that you want to do this handling on tabs directive. It seems more logical to me. And the main argument it that you don't want to repeat on-select for each pane. In most cases it would contain the same expression.

[darwin]

Maybe my "id" naming is confusing. Maybe you can look at it as "payload". Whatever you put there, you get back.

[darwin]

@pkozlowski-opensource Please look at implementation of tabbable directive in angular.js repo:
https://github.com/angular/angular.js/blob/master/src/bootstrap/bootstrap.js#L52

They expose my "id" on tabbable directive via ngModel and call it "value", here is the code how they get it:
https://github.com/angular/angular.js/blob/master/src/bootstrap/bootstrap.js#L109

Actually I've switched from their implementation to yours. I'm happy, yours is much more cleaner. But I'm really missing a single place where I can hook to observe tab changes. I don't want to attach handler to each pane individually. I think it should be on tabs and be flexible for different scenarios (binding, static value, function call, etc.).

[revolunet]

@darwin : i understand your two-way data binding for the tabs.active attribute. you could use pane index number (0, 1, 2...) or, as you say, if they have pane-id defined, their custom id directly instead of the natural index. Will be a nice addition.

I thinks the pane select attribute is still useful though as its very easy to use as you dont need to setup watchers to trigger something.

[seanpdoyle]

👍

This would be a very desirable feature.

[pkozlowski-opensource]

@revolunet We've got a PR now from @ajoslin that addresses many issues with tabs, this one being one of them. As such I'm going to close this PR and focus on Andy's one (#287). Would be totally awesome if you could have a look at the code from #287 to see if it works for you.

[revolunet]

Hi! awesome work, and yes it fits perfectly with my needs :)

Thanks guys !

[saschaklatt]

It seems like the select callback of all tabs will be triggered when the route changes. So everytime I leave the view that includes the tabs a select event from every single tab is fired. Is this a bug?