List View: Allow right-click to open block settings dropdown, add editor setting #50273
Conversation
andrewserong
added
[Feature] List View
|
Size Change: +399 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
I believe I can reliably reproduce this in the following situation:
This appears to be because focus was never in the first popover (or quite possibly within the window at all), so opening another popover doesn't cause the focus outside logic to fire and close the original popover. I'm not too sure how best to fix this just yet, but one possible idea could be to keep track of an |
|
Update: you can replicate the same thing via CMD-click and left clicking on the settings dropdown buttons, so I think there could be an argument for fixing this outside of the right-click use case, too |
andrewserong
force-pushed
the
try/allow-right-click-in-list-view
branch
2 times, most recently
from
8faf37f
to
61ef62a
Compare
|
Update: in 61ef62a I've fixed up the initial flicker of the list view row being focused before opening up the settings menu. The last blocker appears to be the issue of the block settings menu not closing when focus isn't on the browser window. I'll continue digging into that. |
|
Flaky tests detected in 4523767. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7204841512
|
|
I've written up an issue for the problem of multiple block settings menus being displayed over in: #50988 |
|
Thanks for picking this one up again! It will be even better when we combine with the new flyout menu component so we can reduce how long the initial state is and group things a bit better |
Absolutely! And from the sounds of it, the new |
|
Update: it looks like the refactor to using the v2 of Do let me know if anyone feels differently, though, and I can give this PR a rebase / get it as close to ready as it can be for now. |
andrewserong
force-pushed
the
try/allow-right-click-in-list-view
branch
from
61ef62a
to
a04bf22
Compare
|
Update: in 130c0d7, I've had a go at introducing some state in the block editor to track the clientId of the block settings menu that is currently open. If an id is present and it doesn't match a particular block settings menu, then we can force it to close. This seems to fix the issue: 2023-08-31.14.43.36.mp4That fix is a little beyond the scope of the overall intention behind this PR (introduce right-click support), so I'll open up separate PR(s) for that fix to see if it winds up looking viable, and will link over in #50988. Once that issue is resolved in |
andrewserong
force-pushed
the
try/allow-right-click-in-list-view
branch
2 times, most recently
from
51bbf34
to
bfc6f26
Compare
|
Update: now that #54083 has landed, I've given this a rebase and it seems to mostly be working properly now. I'm not sure if we've 100% eradicated issues involving the block settings menu opening and closing correctly, so it'd be worth giving this a manual test to see how it feels for folks in actual usage. |
There was a problem hiding this comment.
This seems to be working well for me, I could not seem to break it and cause multiple menus to show. It works in both the site and post editor (although didn't work when editing a page -- I think by design).
Not shown in the gif is multi block select which I also confirmed to work correctly.
|
Tested in FF, Safari, Chrome and Edge
2023-09-25.09.52.30.mp4Pretty impressive! When you come back to this PR, would it be worth creating a small E2E test that runs through a right mouse click to ensure the context menu appears? Other than that I'd say it's good to go unless you're waiting on further UX feedback ? |
|
|
||
| // If no custom position is set, the settings dropdown will be anchored to the | ||
| // DropdownMenu toggle button. | ||
| if ( ! settingsAnchorRect || ! ownerDocument ) { |
There was a problem hiding this comment.
Is it that ownerDocument can never be falsy due to the fallback value of {}?
There was a problem hiding this comment.
This one's slightly subtle — ownerDocument is set via destructuring, so if the fallback value after the assignment operator is {} then ownerDocument will be undefined as it doesn't exist on {} 🙂
There was a problem hiding this comment.
Oh right, sorry I read that line wrong. {} is the fallback for rowRef?.current 🤦🏻
| @@ -86,6 +86,13 @@ export default function EditSitePreferencesModal() { | |||
| ) } | |||
| label={ __( 'Display block breadcrumbs' ) } | |||
| /> | |||
| <EnableFeature | |||
| featureName="allowRightClickOverrides" | |||
There was a problem hiding this comment.
To me, enableListViewRightClick or allowListViewContextualMenus or the equivalent specific descriptor might describe the feature. allowRightClickOverrides might be too generic?
There was a problem hiding this comment.
Good question! My thinking was to keep it generic so that if we wish to expand other areas to also have right click overrides, we don't wind up with a proliferation of allowRightClick settings in the preferences 🤔
|
After #56481 lands, let's keep this setting in general in both editors. |
Thank you, will do! |
andrewserong
force-pushed
the
try/allow-right-click-in-list-view
branch
from
a9fc04d
to
007a1ed
Compare
|
Now that #56481 has landed, I've given this PR a rebase and retested that it still appears to be working well. So, I think this should be ready for a final review! 🤞 The goal with this PR is that we're adding in right-click in the list view, to open the block settings menu for the currently hovered list view item. In follow-ups we could look at expanding right click behaviour into the editor canvas, and see what might make for good behaviour there, too. |
There was a problem hiding this comment.
Checked in site and post editors:
✅ With the preferences turned on the list view contextual menu appears on right click. All menu items function as expected.
✅ Turned off, I see the browser contextual menu only.
Works in FF, Safari, Chrome and Edge.
Great stuff!!
I left a comment about being specific about where the menu shows, maybe to set expectations? 😄
|
Thanks for reviewing @ramonjd! 🙇 It looks like the e2e failures are legit — it's an unfortunate e2e test that passes for me locally but not in the Github Action because our Github Actions use a different window size to my local environment, and the test expected that the "General" tab in preferences wasn't long enough to cause a scrollbar. I think that test likely needs to be rewritten a little (or at least to have comments added) as it's highly likely to break when adding or removing preferences from the panel. I'll see if I can fix it up for this PR, though. In terms of functionality in this PR, nothing's breaking, it's just that the test is failing 🙂 |
|
I kicked them off again, but I think maybe the failing e2e tests in test/e2e/specs/editor/various/a11y.spec.js are related because the preferences pane has been updated? |
|
Sorry, my page hadn't updated by the time you wrote the comment!
Makes sense, thanks for explaining. |
|
I've pushed a temporary fix for the e2e test in 35d42be to to hopefully resolve the issue for this PR (assuming the actions pass now!). Separately, it could be good to come up with another way of testing that particular part of the preferences panel as it should ideally test the behaviour but not be dependent on the current content of the preferences panels 🤔 |
Yeah, that seems like a flakey approach. What you've seems like a good compromise, while still preserving the test. Thanks! LGTM |
|
Thanks for the reviews, folks! I'll merge this in and then open up a follow-up issue for ideas for other areas that could benefit from a custom right-click dropdown. |
|
Follow-up issue for discussing other places where right-click could work: #57091 |
|
Love this! |
|
Same! Good stuff! |
…tor setting (#50273) * List View: Allow right-click to open block settings dropdown, add setting * Fix flicker of focus state when initially right clicking * Tidy up useSelect * Fix e2e test (hopefully) * Update help text for preferences items
What?
Fixes #40287, based on a previous attempt in #41041
This PR adds right-click behaviour to the list view, so that right-clicking on a list view item will open the block settings menu, with the menu positioned over the mouse cursor.
It also adds an editor setting to the post and site editors so that users can disable the setting if they wish.
Why?
The general idea, as raised in #40287, is that in other web apps that people are used to, right-clicking an item opens up a contextual menu that is part of the app. The more that using Gutenberg feels like using an app, the more it might be appropriate that right-click feels like more of an in-app kind of behaviour. That's the idea, at least!
How?
allowRightClickOverridessetting in the post and site editors.onContextMenuhandler, and if the settings allow it, simulate a click on the settings dropdown when the button is right-clicked.Testing Instructions
Screenshots or screencast
Right-click items in the list view:
2023-12-14.13.30.39.mp4
The setting: