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

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

Closed
wants to merge 1 commit into from

Conversation

kborchers
Copy link
Member

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

@scottgonzalez
Copy link
Member

Is there a race condition that can occur with multiple autocomplete instances since you made this a shared variable across all instances?

@kborchers
Copy link
Member Author

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.

@kborchers
Copy link
Member Author

@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?

@scottgonzalez
Copy link
Member

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.

@kborchers
Copy link
Member Author

@scottgonzalez I just added another commit with that addition of a mousemove handler.

isNewMenu = false;
event.stopImmediatePropagation();

$( document ).one( 'mousemove', function( event ) {
Copy link
Member

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...

Copy link
Member Author

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?

Copy link
Member

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?

@kborchers
Copy link
Member Author

Made those formatting changes and rebased into a single commit.

@kborchers
Copy link
Member Author

Added comments

@scottgonzalez
Copy link
Member

This doesn't seem to do anything for me in Firefox. Chrome doesn't appear to have this issue anymore.

@scottgonzalez
Copy link
Member

HAHAHA. It doesn't do anything because of the event.stopImmediatePropagation() in menu :-P

@scottgonzalez
Copy link
Member

@kborchers Now that you've cleaned up menu, can you rebase and see if this is still working?

@kborchers
Copy link
Member Author

Sure, I'll take a look tonight

… page load in Firefox. Fixed #7024 - Autocomplete menu options are activated even if mouse is not moved
@scottgonzalez
Copy link
Member

Landed in 098dd14 :-)

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