Skip to content
This repository was archived by the owner on May 29, 2019. It is now read-only.

tab selection from controllers #87

Closed
wants to merge 3 commits into from
Closed

tab selection from controllers #87

wants to merge 3 commits into from

Conversation

just-boris
Copy link
Contributor

I refactor directive api.
This approach makes code simply and clear, thanks

Selection sets through selected-tab attribute. There puts tab's name when it was selected
@ajoslin
Copy link
Contributor

ajoslin commented Jan 24, 2013

Hey again boris,

Thanks for refactoring it. It looks good, just a couple things:

  1. Could you change 'selected' to 'active'? Since accordion and carousel use 'active' attribute, it's more consistent.
  2. Why not just use '=' binding for the attribute? You're basically making it yourself (eg scope: { 'active': '=' })
  3. Could you add a few more unit tests for weird cases? Like when multiple panes evaluate to active=true

@just-boris
Copy link
Contributor Author

  1. Done. But you using "selected" property to bind class, because I renamed only attribute name.
  2. It makes selected attrubute optional. Angular.js has issue directives that have '=' binding, changing scope value causes error if not specified(optional) angular/angular.js#1435 about it
  3. Added 2 test cases: multiple set active=true and set active=false for all tabs

@ajoslin
Copy link
Contributor

ajoslin commented Jan 28, 2013

You're right about the '=' binding. It looks good to me, Boris :-). Thanks!

Landed in a75a85d

@ajoslin ajoslin closed this Jan 28, 2013
codedogfish pushed a commit to codedogfish/angular-ui-bootstrap that referenced this pull request Sep 15, 2015
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.

2 participants