-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Dropdown / Block Settings Menu: Ensure only one block settings menu is open at a time #50988
Comments
As you pointed out, we're working on a new version of The new version is also going to be "modal" by default — ie. the rest of the page should not be interactive while a dropdown menu is open — would that inherently solve the issue? In case this issue is too urgent to be ignored until DropdownMenu v2 is working well, we could look at other alternatives like the one you're suggesting |
Thanks @ciampo!
Oh, yes, I think it should! Provided that the modal behaviour works via any kind of click event outside of the currently open modal allowing for the modal to close, that should work nicely.
I think this can happily be ignored until DropdownMenu v2 is working well. For the moment, I imagine almost no-one would notice the existing issue, and it only appears to be a blocker to implementing right click behaviour as in #50273, and I believe the idea for that is to hopefully use the v2 dropdown menu, anyway, to enable flyout sub menus. If there's any urgency to get right click behaviour in sooner, then we could always look into some of the alternatives. Let me know if you need any reviews on |
Thank you! I'll definitely ping you soon for an extra pair of eyes when testing |
A shorter-term fix for this using state within the block editor (and allowing the DropdownMenu to be controlled) is ready for review over in #54083. I think this could potentially be a good solution for now (and to unblock work on right-click in the list view) to side-step waiting on the v2 for DropdownMenu. |
What problem does this address?
While working on adding right click support to the list view over in #50273 I've run into an issue with the block settings menu (or dropdowns in general) when right clicking into the browser window while another browser window is active. When doing so (on a Mac at least), right clicking doesn't shift focus to the browser window, so a click event will happen, but focusing inside a dropdown does not. This means that the focus outside behaviour of dropdown menus doesn't work, so it's possible to open multiple block settings menu within the list view.
To replicate without that PR, you can CMD+click from another browser window into an inactive post editor browser window, and click in the general area of the block settings menu buttons. This will open up multiple block settings menus without the already open ones closing.
Screenshot of the issue
It's a little hard to see in this screenshot, but there are multiple block settings menus open:
Screengrab of the issue
This should hopefully be a little clearer. In this screengrab, I have another browser window active, and then I CMD+Click into the inactive window with the block editor open, and click on the list view block settings menu buttons to open multiple block settings menus. The issue feels pretty edge-casey in isolation, but with #50273 (introducing right-click behaviour) the issue will be more prominent, so I'm hoping we can look into a fix.
2023-05-26.16.32.04.mp4
What is your proposed solution?
I think it'd be good if we can somehow ensure that only one block settings menu is open at a time, possibly by storing state somewhere of the instance of the latest opened block settings menu? Then we could have a
useEffect
or something of the like the block settings menu that checks if the currently opened block settings menu id matches the current id, and if not, it closes the menu.I'm not sure how do-able that is, and I see that the v2 DropdownMenu introduced in #49473 allows for
open
as a controlled value, so perhaps that component might make it easier to achieve.I was a little stumped attempting to fix this over in #50273, so thought I'd open up this issue in isolation in case folks have ideas on how to implement it. Would love to hear ideas!
The text was updated successfully, but these errors were encountered: