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

fix(uiSelectCtrl): correcting input focus #1517

Merged
merged 2 commits into from
Mar 27, 2016

Conversation

patrickhousley
Copy link
Contributor

Corrected input focus when ui-select contains no elements. Also corrected a memory leaked caused by event handlers never being removed.

Closes #1253

Corrected input focus when ui-select contains no elements. Also corrected a memory leaked caused by event handlers never being removed.

Closes angular-ui#1253
@patrickhousley
Copy link
Contributor Author

This PR corrects two issues:

  1. The last modification I made to this section of the code created a memory leak where by events were repeatedly added to the .ui-select-choices-content element but never removed. Those will now be removed once the event is fired.
  2. With regards to issue on focus not working for first click #1253, in the event the ui-select has no elements, the enter event is never fired for the .ui-select-choices-content element. To work around this, the code will fall back to using the removeClass event on the .ui-select-search element. Testing has shown that we could rely solely on this event but I cannot test all possible browsers.

@user378230
Copy link
Contributor

@wesleycho any chance you could review this if/when you get a chance? I don't have any experience with angular animate.

// Only focus input after the animation has finished
$timeout(function () {
ctrl.$animate.off('removeClass', searchInput[0], animateHandler);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this needs to be in the $timeout

Moved the removal of activate events out of the $timeout. This removes additional unneccessary calls to the event handler once the $timeout has been initiated.
@hitchcockwill
Copy link

Just chiming in that it would be awesome to see this fix in the next release!

@wesleycho
Copy link
Contributor

This LGTM @user378230 - feel free to merge, unless you feel like this needs a test.

@user378230 user378230 merged commit 2cc07dc into angular-ui:master Mar 27, 2016
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.

None yet

4 participants