fix(tabs): keep stable tab order (when filtering ng-repeat)

[darwin]

The pane directive should call tabsCtrl.addPane with index, because link function is not guaranteed to be called sequentially. DOM panes may appear or disappear on one-by-one basis in more advanced scenarios with applied ng-repeat filtering.

[ajoslin]

Thanks, @darwin! Good fix.

Could you add some evil test cases for when more elements are between panes, to be sure it doesn't goof up? The index values might mess up in this case, but to see if the tabs still work as intended.

Eg:

<tabs>
  <pane>One</pane>
  <div ng-repeat="i in weirdDivs">Hello!</div>
  <pane ng-repeat="p in panes | filter:paneIsAvailable"></pane>
  <div ng-repeat="i in moreWeirdDivs">I'm gonna mess you up!</div>
  <pane>Last pane</pane>
</tabs>

[darwin]

You are right. Those extra divs mess up the indexing. Now I filter children via .tab-pane class. But it depends on template, I don't know how to distinguish tab panes. Do have a better idea?

[darwin]

Ok, found a better way. Is it ok now? Let me know if you want me to squash those commits into a single one.

[ajoslin]

That works! Though I'm not sure about adding the variable to the scope. It's probably fine though.

I'd say definitely rename the variable to isPane instead of pane, since usually when we use pane as a variable it refers to a pane scope. And maybe prefix it with $ too (__$isPane), so we can be sure no one will want to use it in a controller. This way it looks like a private variable with the underscores and an internal variable with the $, but not an angular core variable. lol 😄

And we can squash it on our end before pushing - you'd have to open a new PR with a new branch and squashed commits otherwise.

[darwin]

It does not have to be stored in the scope. I could store it on raw DOM element similar way.

It would be great to find a way if directive's X linking phase was applied on a DOM element. Maybe such information is already available in Angular...

[petebacondarwin]

Actually the more I think about this directive the more I think it should fundamentally change. Instead of using a repeater inside the template we should be using the actual pane element as the tabs and then transcluding their content into the pane content area. This would also help with transclusion of headings.

What do you think?

[ajoslin]

That sounds interesting Pete. We could also think about transcluding only the currently selected tab's content (load a tab's content when selected, throw it out when not selected and load the new one). The transclusion way would fix this problem and I'm sure some future problems.

[pkozlowski-opensource]

Hmmm, I'm really nervous about the fact that it links directives JavaScript code to DOM structure. I'm not sure if I understand correctly the whole problem / solution, but wouldn't it break if we introduce grouping of tabs as discussed in #150?

Ideally directive's JavaScript code should know nothing about CSS and DOM structure. I know it is hard and practically impossible in many cases but this is something we should strive for.

I need to look into this more but I would like to search for an alternative solution.

[pkozlowski-opensource]

@darwin This is great work and test are good so you are on something. But I would really like to discuss it a bit more and try really hard to keep CSS / DOM knowledge out of JavaScript.

The way I like to think about it is this: let's say that I would like to use a directive with the http://foundation.zurb.com/ or similar. Can I do this by only changing a template?

[darwin]

I created more general implementation. Now the only assumption is that pane is in the DOM somewhere under tabs, but not necessarily all panes must be on the same DOM level relative to tabs.

The implementation is more complex. I've introduced a way how to sort panes by computing their DOM "order" in relation to tabs element.

[ajoslin]

Sorry @darwin! I think the real way to fix this is to make the headers be the actual elements instead of the tab contents. So then the headers will always be in the right order without any of this confurmuddledness 😕 (made up word).

I may be able to work on a refactor soon...

[darwin]

Ok, no hard feelings. You may close it if you want.