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

Closes a dropdown when another one is opened #481

Merged
merged 3 commits into from
Dec 9, 2014

Conversation

asafdav
Copy link
Contributor

@asafdav asafdav commented Dec 7, 2014

Fixes #384

@amcdnl
Copy link
Contributor

amcdnl commented Dec 8, 2014

This has been annoying me... Just commented on a similar issue this morning.

@amcdnl
Copy link
Contributor

amcdnl commented Dec 8, 2014

Can you add a quick test for this? Just 2 selects, open one, open other, and then verify first is hidden would work perfect. Other than that, looks good i'll merge once I get a test.

@asafdav
Copy link
Contributor Author

asafdav commented Dec 8, 2014

Hi, I started to test my code and found some "focus" issues (in some cases the old select was still focused), It turned out to be a much bigger change. anyway, I managed to solve it and also removed the "ugly" 50 milli timeout. My problem now is that although it seems to work perfectly on my manual testing, I can't get my "karma" test case to work, it seems like "document" events are not raised and therefore I can't really test my feature. I pushed my code (It won't pass travis-ci) so you'll be able to see my test case.

Do you have any idea how can I solve it?

Thanks in advance

@amcdnl
Copy link
Contributor

amcdnl commented Dec 9, 2014

I suspected writing the test would cause you to remove the 50ms timeout hehe.

I'm not super familiar with karma, I know you have to use $document for testing but other than that I wouldn't know. @dimirc any thoughts?

@asafdav
Copy link
Contributor Author

asafdav commented Dec 9, 2014

OK I figured it out, you have to add the element to the DOM in order to properly test event handlers. the unit tests should pass now.

amcdnl added a commit that referenced this pull request Dec 9, 2014
Closes a dropdown when another one is opened
@amcdnl amcdnl merged commit 0eb69f0 into angular-ui:master Dec 9, 2014
@amcdnl
Copy link
Contributor

amcdnl commented Dec 9, 2014

Thanks! 👍

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.

Two or more close dropdowns overlap instead of closing the first and opening the second etc.
2 participants