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

fix(dropdown): remove extra uib-keyboard-nav #4891

Closed

Conversation

Foxandxss
Copy link
Contributor

BREAKING CHANGE: keyboard-nav for the dropdown is not longer a directive and to use it you have to use keyboard-nav instead of uib-keyboard-nav.

@Foxandxss
Copy link
Contributor Author

Tell me that I am not crazy and that the guy that implemented this, did it twice... Why do we need the whole keyboard nav both in a directive and also inside the main controller? Works directly as is.

Tweaked a bit the demo to include another keyboard-nav.

And don't worry about the docs, I have to rewrite them entirely.

PS: Made it a fix so the BC will show up

@RobJacobs
Copy link
Contributor

LGTM

@icfantv
Copy link
Contributor

icfantv commented Nov 13, 2015

You're not crazy. I noticed that before and it was the cause of a bunch of WTFs when trying to write tests because the code that you thought should be called was never getting called. E.g. when you trigger the keydown event in jasmine it wasn't using the event handler on the directive - it was using the main one. But in an actual web page, the opposite is true. It took me a days to figure that out. Very frustrating.

@Foxandxss
Copy link
Contributor Author

It took me a while too to figure it out.

@Foxandxss
Copy link
Contributor Author

Hold on this one, I saw a big stupidity on the tests.

@Foxandxss Foxandxss force-pushed the fix/dropdownkeyboardnav branch from 36a7250 to 96d8abe Compare November 13, 2015 16:54
@Foxandxss
Copy link
Contributor Author

Alright, fixed.

@Foxandxss Foxandxss closed this in 57f72b2 Nov 13, 2015
@Foxandxss Foxandxss deleted the fix/dropdownkeyboardnav branch January 9, 2016 16:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants