-
Notifications
You must be signed in to change notification settings - Fork 4.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
Update: Make OffCanvasEditor use LeafMoreMenu by default. #47844
Update: Make OffCanvasEditor use LeafMoreMenu by default. #47844
Conversation
450b495
to
b02c606
Compare
Size Change: +257 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Flaky tests detected in 41b6fba. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4137356597
|
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.
I'm not sure about this approach. Long term we want to remove the OffCanvasEditor component and use the ListView component. This change will make it harder to align the OffCanvasEditor with ListView as it makes them diverge more.
In that case, would an alternative approach where we keep the OffCanvasEditor as it is, but move LeafMoreMenu to the block editor packages as a locked experiment that is used by the navigation block and navigation sidebar work? I want to avoid LeafMoreMenu being duplicated and want to make sure the navigation sidebar and the navigation block have the same menu. |
That sounds ideal! |
b02c606
to
0e5d52a
Compare
Hi @scruffian, I updated this PR to follow the discussed approach. |
84339ae
to
41b6fba
Compare
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.
LGTM
Cherry-picked this PR to the wp/6.2 branch. |
This PR makes OffCanvasEditor use LeafMoreMenu by default.
We have two use cases of OffCanvasEditor and in both of them we want to use LeafMoreMenu so the items match. I think we can avoid a property for custom menus until we have a case where we we need something different from this default.
This change also reduces the items that appear on the navigation sidebar.
cc: @draganescu, @scruffian