Skip to content
This repository has been archived by the owner on May 5, 2018. It is now read-only.

Added ui-tabs directive #237

Closed
wants to merge 3 commits into from
Closed

Conversation

typesafe
Copy link

I've added a uiTabs directive that allows you to add and control tabs with minimal effort.

<div ui-tabs="{tab1 : {label : 'this is the first tab'}, tab2 : {label : 'this is the second tab'}}">
    <div>This is the content of the second tab</div>
    <div ng-include="/more/likely/tabs/are/includes.htm"></div>
</div>

View the README (https://github.com/typesafe/angular-ui/blob/master/modules/directives/tabs/README.md) for more details.

Added uiTabs directive that add twitter bootstrap style tabs for each
direct child of decorated element.
@pkozlowski-opensource
Copy link
Member

@typesafe There is already a directive for tabs in the boostrap repo: https://github.com/angular-ui/bootstrap/blob/master/src/tabs/tabs.js

Could you elaborate on how your implementation is different / better?

@typesafe
Copy link
Author

OK, totally missed that one! Wouldn't have started this if I had known...

Anyway, since you asked: My version does not rely on templates and explicit elements, instead it applies behaviour to any element and its children by providing a definition object (which is extended and exposed through the current scope).

@ProLoser
Copy link
Member

There are a couple of ways I could think of suggesting improvements, but before that should we close this PR and consider instead moving this to bootstrap?

@typesafe a few of my concerns:

I have thought about doing tabs using an object definition, but this only seems to serve to obfuscate things needlessly. Having {tab1: ... , tab2 : ... , tab3 : ... } does not feel a lot better than <li>Tab1</li><li>Tab2</li><li>Tab3</li>

Instead, the only tedium becomes adding click events, keeping track of which tab is open and adding some sort of 'active' class.

If I was developing a tabs directive myself from scratch, I might instead focus on solving those issues alone. Something like ui-tabs="groupName" and ui-tab="tab1" that automatically becomes something like

<li ng-click="groupName='tab1'" ng-class="{active:groupName=='tab1'}>

Repeaters, conditionals, and other such nonsense could then be handled as developers see fit. Of course the elegance of this solution could also be disputed when it comes to something like changing the active tab programmatically or from outside the traditional controls.

@pkozlowski-opensource
Copy link
Member

+1 for closing this one and focusing on what we've got in the bootstrap's repo. What we've got in there is already using templates so people could customize look & feel if needed.

I also do agree with @ProLoser that using object literals is not necessarily better over using plain old HTML. If one looks for cutting down number of characters to type there is always option of Jade or something similar.

@typesafe
Copy link
Author

(@pkozlowski-opensource, you beat me by a few minutes in responding, but I'm gonna leave it as is)

Thanks for your feedback.

Indeed, I agree, the definition object is probably not the best idea and delegating the creation of it to a controller is even worse.

Going over this again this a bit, I think that something like the following would be a nice improvement:

<div ui-tabs>
    <div title='this is tab 1'>...</div>
    <div title='this is tab 2' ng-include="..."></div>
    <any-tag title='this is tab 3'>...</any-tag>
</div>

function ZeController($scope) {
    // all tabs are exposed through tabs and get the default names tab1, tab2, etc (since there's no id attr)
    $scope.tabs.tab2.activate();
    $scope.tabs.tab2.title = "I'm active now";
}

Optionally, one could name the tabs by providing ids...

<div ui-tabs>
    <div id="general" title='this is tab 1'>...</div>
    <div id="someDetails" title='this is tab 1'>...</div>
    <div id="moreDetails" title='this is tab 1'>...</div>
</div>

function MyController($scope){
    // tabs are now named
    $scope.tabs.someDetails.activate();
}

...and indicate the initially active tab

<div ui-tabs>
    <div id="general" title='this is tab 1'>...</div>
    <div id="someDetails" title='this is tab 1'>...</div>
    <div id="moreDetails" title='this is tab 1' class=active>...</div>
</div>

Adding a remove() method to a tab would be trivial and adding a tabs.add method could be handled by using ng-includes like so:

$scope.tabs.add(title, ngIncludeUrl[, options]) // where options could contain controller, index, etc.

But I'm getting carried away :-).

Anyway, this PR was the result of me being a bit over-enthousiast (I hadn't even noticed the bootstrap repo!). So I totally agree that it should be closed. I'm not sure if it should be moved over to the bootstrap repo, through. I think it's more important to aim for consistency that is in line with the overall philosophy of the project, which I'm not familiar with (yet)...

Like I said before, I wouldn't have started this if I had known bootstrap already had it.

@typesafe typesafe closed this Oct 24, 2012
@pkozlowski-opensource
Copy link
Member

@typesafe Thnx for your constructive approach, it is a pleasure to see your comments! And above all, please do have a look at tabs in the bootstrap repo!

What we are trying to do is very similar to what you have been proposing here, that is:

<tabs>
    <pane title='this is tab 1'>...</pane>
    <pane title='this is tab 2'>...</pane>
    <pane title='this is tab 3>...</pane>
</tabs>

Having the ability to select a tab from a controller would be awesome, I've started to look into this a bit (same for accordion).

So please, if you've got any suggestions / improvements / PRs don't hesitate to contribute in the boostrap repo! Your input / contributions will be very valuable, I'm sure of it!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants