Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#8929 closed bug (fixed)

Menu needs adjustement for use in Selectmenu

Reported by: Felix Nagel Owned by:
Priority: minor Milestone: git
Component: ui.menu Version: git (not yet released)
Keywords: Cc:
Blocked by: Blocking:

Description (last modified by Jörn Zaefferer)

Selectmenu utilizes Menu to render its option list. When selectmenu is opened the selected item looses focus after 300ms. This is a usability problem which is currently fixed by changing this.delay in the Menu instance.

Problem is the collapseAll call on document click (create method) and the following blur event. I would remove that on click event but the mouseHandled reset is needed.

Detailed bug report by joshuahiggins: https://github.com/jquery/jquery-ui/issues/492#issuecomment-9994772

More info about the current fix in selectmenu: https://github.com/jquery/jquery-ui/pull/492/files#r2266217

I assume kborchers should look into this.

PR from Kris: https://github.com/jquery/jquery-ui/pull/1031

Change History (12)

comment:1 Changed 7 years ago by Scott González

Milestone: 1.10.0git
Status: newopen

comment:2 Changed 6 years ago by k_borchers

So I've been thinking about this for a bit and the solutions I have thought might work almost all involve a new option in which I don't like. The other thing I thought about was maybe making the delay configurable and setting a delay to false would effectively set the delay to 0 but also prevent the collapseAll when clicking outside of the menu. This would allow Selectmenu to handle the menu hiding, etc. but keep the selected item highlighted. Thoughts?

comment:3 Changed 6 years ago by Felix Nagel

We'll see if it works out but it definitely sounds promising. Does this need a lot of changes on Menu side?

comment:4 Changed 6 years ago by Jörn Zaefferer

The delay property is used in multiple places. Would the check for a boolean-false be in all of them? Or just inside the document-click handler?

If that works (either way), how about making the delay property an option and documenting it? "Configure the delay for auto-closing submenus. Set to false to prevent the auto-closing[ on clicks outside the menu]."

comment:5 Changed 6 years ago by Jörn Zaefferer

Description: modified (diff)

comment:6 Changed 6 years ago by Scott González

I can't see this being used outside of the creation of a new widget. Can we just move the conditional from inside the document click handler to a separate function and let selectmenu override that? I'd only want the conditional moved since the resetting of this.mouseHandled should still occur.

comment:7 Changed 6 years ago by Felix Nagel

The patch works pretty nice.

Why not add a var similar to this.delay?

comment:8 Changed 6 years ago by Jörn Zaefferer

Can we just move the conditional from inside the document click handler to a separate function and let selectmenu override that?

Sounds good to me. Kris, can you implement that?

Btw. it may be a good idea to land the change in master first, merge that to selectmenu, and apply the second commit there. Or figure out how git behaves when you merge a branch that contains a cherry-picked commit.

comment:9 Changed 6 years ago by Kris Borchers

Resolution: fixed
Status: openclosed

Menu: Make check for click outside of menu a function which can be overridden. Fixes #8929 - Menu needs adjustement for use in Selectmenu

Changeset: cceb163548eea78525a3a60ada95a5af6e3ddf25

comment:10 Changed 6 years ago by Kris Borchers

Menu: Make check for click outside of menu a function which can be overridden. Fixes #8929 - Menu needs adjustement for use in Selectmenu (cherry picked from commit cceb163548eea78525a3a60ada95a5af6e3ddf25)

Changeset: 2a7896ec3eac09aa13fc6e0f8bf8316e7985387e

comment:11 Changed 6 years ago by Scott González

Revert "Menu: Make check for click outside of menu a function which can be overridden. Fixes #8929 - Menu needs adjustement for use in Selectmenu"

This reverts commit 2a7896ec3eac09aa13fc6e0f8bf8316e7985387e.

Changeset: 2135008d2cf7c4a706ce641349db86eee7cf86ce

comment:12 Changed 6 years ago by Scott González

Reverted from 1-10-stable since this is an API change.

Note: See TracTickets for help on using tickets.