Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch tabs iif tab state is inactive, closes #10 #11

Merged
merged 1 commit into from
Feb 5, 2015

Conversation

tlvince
Copy link
Contributor

@tlvince tlvince commented Feb 3, 2015

No description provided.

@rpocklin
Copy link
Owner

rpocklin commented Feb 5, 2015

Thanks for the PR. It's a good attempt, but if I open the example and click Settings it works the first time, but I then re-click Settings and it performs the original behaviour. Might need more work.

If I have a chance in the next few days i'll go over the code but feel free to amend the PR.

@tlvince
Copy link
Contributor Author

tlvince commented Feb 5, 2015

Hmm, strange. I can't reproduce this. Are you loading ui-router-tabs.js from my fork?

@rpocklin
Copy link
Owner

rpocklin commented Feb 5, 2015

Sorry, my mistake - I checked out your master not bug/active-inactive.

It looks good, fancy writing a testcase in jasmine? If not i'll accept anyway...

@tlvince
Copy link
Contributor Author

tlvince commented Feb 5, 2015

Sorry, I'm pretty tied up right now, but agreed, it could probably use a test.

NB, if you merge this, could you release a new tag?

rpocklin added a commit that referenced this pull request Feb 5, 2015
Switch tabs iif tab state is inactive, closes #10
@rpocklin rpocklin merged commit 752054b into rpocklin:master Feb 5, 2015
rpocklin added a commit that referenced this pull request Feb 6, 2015
@rpocklin
Copy link
Owner

rpocklin commented Feb 6, 2015

I've added an assertion to a test which failed before this commit was added, so that's been covered off now. Released 1.2.0.

@tlvince
Copy link
Contributor Author

tlvince commented Feb 6, 2015

Great, thanks! 👍

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

Successfully merging this pull request may close these issues.

None yet

2 participants