-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Autocomplete: Added check to determine if menu has just been created to override unintended mouseover event and reset that variable when closed. Fixed #7024 - Autocomplete menu options are activated even if mouse is not moved #288
Conversation
Is there a race condition that can occur with multiple autocomplete instances since you made this a shared variable across all instances? |
I do not believe so because as soon as you switch from instance A to instance B, instance A's menu closes, causing the variable to be reset before instance B's menu is created by typing a letter. I have not been able to create that issue in my tests. |
@scottgonzalez Did you get a chance to think on this one. I am unable to create a race condition. Is there any way that 2 autocompletes would ever be open at the same time? |
I think we'd want to add a mousemove handler to focus the element. Right now you have to hover over a different item before something gets focused. We should focus as soon as you move your mouse. |
@scottgonzalez I just added another commit with that addition of a mousemove handler. |
isNewMenu = false; | ||
event.stopImmediatePropagation(); | ||
|
||
$( document ).one( 'mousemove', function( event ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part is confusing me a bit, why is it needed - it seems that the mouseover
should fire naturally shouldn't it? Why are we simulating it in this one( "mousemove" )
on the document?
Also - Prefer double quotes on strings...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, the bug was that if someone focused the autocomplete and their mouse is below, when they started typing what ever item the mouse was over would be selected and placed in the text box. This fix prevents that but as a side-affect, it disables that menu mouseover until the mouse enters another menu item. The mousemove, determines they are actually ready to select one from the list and fires that mouseover that was stopped. Make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, that makes perfect sense.
Can we get a comment referencing the ticket / reason for behavior maybe?
Made those formatting changes and rebased into a single commit. |
Added comments |
This doesn't seem to do anything for me in Firefox. Chrome doesn't appear to have this issue anymore. |
HAHAHA. It doesn't do anything because of the |
@kborchers Now that you've cleaned up menu, can you rebase and see if this is still working? |
Sure, I'll take a look tonight |
… page load in Firefox. Fixed #7024 - Autocomplete menu options are activated even if mouse is not moved
Landed in 098dd14 :-) |
Autocomplete: Added check to determine if menu has just been created to override unintended mouseover event and reset that variable when closed. Fixed #7024 - Autocomplete menu options are activated even if mouse is not moved
Replaces #280