feat(tabs): add the ability to set the direction of the tabs

[lgalfaso]

Add the ability to set the direction of the tabs, the possible
values are 'right', 'left' and 'below'. If no direction is defined
the tabs are rendered on the top as they do now

[ghost]

in master it is already possibly to specify direction through <tabset class="tabs-<orientation>"/> see #584

[lgalfaso]

@bk1te at master there is the property vertical that sets the class nav-stacked on the <ul>, this adds the class tabs-below, tabs-right or tabs-left on the parent of this element.

This is, what is at trunk is for "Stackable" and this patch is for "Tabbable in any direction"

http://twitter.github.io/bootstrap/components.html#navs

Maybe I am missing something

[ghost]

update comment

[pkozlowski-opensource]

@lgalfaso we had an issue opened #102 which was about adding this direction / orientation attribute. But then I've realized that it is possible with a CSS attribute so closed the issue - no one was screaming at the time :-)

I'm in favor of merging it - it makes tabs more semantic and Bootstrap's CSS independent.
@ajoslin @bekos WDYT?

[bekos]

@pkozlowski-opensource +1 for replacing Bootstrap classes with semantic attributes. For this one, looking at the code I think the below option does not work as it is supposed (same problem as #102, need to change elements order).

[lgalfaso]

6616b51eb83f4eeff2505815ce06aed29b5304bb fixes the issue of tabs below, it would be simpler to use ng-if but that would add a hard dependency on angular 1.1.5+, so decided to go with the proposed solution at the patch

[lgalfaso]

@pkozlowski-opensource With 02664aad9548f2a916a1d4a3bb00887d38ba40d8 the extra <div> will be hidden, without an ng-if I am not sure on how to fully remove it

[ajoslin]

Hi @lgalfaso - thanks for coming in with a feature PR :-)

Sorry for the late reply I've been on vacation.

This is a good feature to have - but I dunno if I like the direction of having two ng-repeats.

Can't we just make one ng-repeat and have the <ul> of tab titles be either appended or prepended depending on the option? We can even make it customizable inside the template. Here is how I would probably do it:

tabset.html

<div class="tabbable">
  <div tabset-titles=" tabsAbove "></div>
  <div class="tab-content">
    <div class="tab-pane" 
           ng-repeat="tab in tabs" 
           ng-class="{active: tab.active}"
           tab-content-transclude="tab">
    </div>
  </div>
  <div tabset-titles=" !tabsAbove "></div>
</div>

tabset-titles.html

<ul class="nav {{type && 'nav-' + type}}" ng-class="{'nav-stacked': vertical}">
</ul>

tabsetTitles directive

.directive('tabsetTitles', function($http) {
  return {
    restrict: 'A',
    require: '^tabset',
    templateUrl: 'template/tabs/tabset-titles.html',
    replace: true,
    link: function(scope, elm, attrs, tabsetCtrl) {
      if (!scope.$eval(attrs.tabsetTitles)) {
        elm.remove();
      } else {
        //now that tabs location has been decided, transclude the tab titles in
        tabsetCtrl.$transcludeFn(scope, function(node) {
          elm.append(node);
        });
      }
    }
  };
});

Then tabset directive would just expose its transclude function on its controller.

[lgalfaso]

Hi @ajoslin
Updated the patch so there is only one repeat (in fact, there is only one of everything), but there is some DOM manipulation during link to flip things if the direction is below

[ajoslin]

Hi @lgalfaso,

Your solution looks the best and simplest at first... but the problem with it is that we want these templates fully customizable. If I make my own completely custom CSS framework that uses a different layout than bootstrap's, I should be able to take tabs.js from this repo, and put my own template in.

So think of it this way: If I had to completely change the css and add a few layers of wrapper divs or some really weird thing, would it still work?

The solution I proposed in the comment above allows a customizable template.

[ajoslin]

If you don't quite understand what 'transcludeFn' in my example means tell me & I'll explain it (many people don't understand it, it can be confusing 🌝 )

[lgalfaso]

Hi @ajoslin the code you posted is easy to understand, the parts that I dot not fully like are that there is an extra template and there is still some duplication, but agree that the patch I posted will have a tendency to break if a user customizes the templates. Will post a new patch in a few

[lgalfaso]

e0ddec6 implements the changes proposed by @ajoslin

[ajoslin]

edit: nevermind :-)

[ajoslin]

Nevermind my above comment! I will merge this in later this week. Thanks @lgalfaso !

[mfrye]

@ajoslin I was trying to work on the same thing independently and I just came across this issue. I'm a bit of a noob with Angular, so I'm sorry if this is a stupid question...

I pulled the code from the master, but I can't seem to get the tabs directions to work right and I'm having trouble understanding your code. I'm adding direction="below" to the tabset, but it's not triggering the tabsAbove correctly. I guess I'm still trying to wrap my head around transclude, but can you provide an example of this working?

[bekos]

@mfrye Have you tried with quotes? diection="'below'"

[mfrye]

ah that was it. Thank you @bekos

[mfrye]

@bekos actually one more question. Say I wanted to dynamically change between top and bottom with a button. What would be the optimal way to do that / like how would I reference that attribute in the controller?

[bekos]

The direction attribute is not watched by the directive (because until we found an easy way to distinct between watchable/unwatchable attributes, we cannot penalty other user's performance for edge cases like this one).

So, you can either edit the tabset's linking function to watch this attribute, or better, you can destroy and recreate your tabs with the click of the button.

[mfrye]

Okay, thanks for the help.