Navigation: Add an edit menu button to the block controls #45772
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
Size Change: +768 B (0%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
getdave
reviewed
Nov 15, 2022
There was a problem hiding this comment.
Toggle pressed state
In the site editor the Edit buttons seems to be permanently "pressed". This isn't a toggle so I don't think that's how it should behave.
Focus handling with keyboard
Also I noticed that using keyboard to click the button results in focus on the sidebar rather than the specific Menu panel within the sidebar.
Comment on lines
731
to
732
| ? __( 'Close list view' ) | ||
| : __( 'Open list view' ) |
There was a problem hiding this comment.
We should probably re-consider the naming conventions at some point to ensure they are distinct from the main "global" List View.
getdave
reviewed
Nov 15, 2022
| return { | ||
| isNavigationSidebarOpen: | ||
| select( interfaceStore ).getActiveComplementaryArea( | ||
| editSiteStore.name |
There was a problem hiding this comment.
@scruffian If it helps we could consider whether it's worth importing the entire store just to access its name property. You should just hard code the value and have done.
There was a problem hiding this comment.
That's a good point. Maybe we can extract the name to a const
Comment on lines
731
to
732
| ? __( 'Close list view' ) | ||
| : __( 'Open list view' ) |
There was a problem hiding this comment.
Suggested change
| ? __( 'Close list view' ) | |
| : __( 'Open list view' ) | |
| ? __( 'Close editable menu list view' ) | |
| : __( 'Open editable menu list view' ) |
Just an idea...
Comment on lines
-181
to
-183
| if ( process.env.IS_GUTENBERG_PLUGIN ) { | ||
| MaybeNavMenuSidebarToggle = NavMenuSidebarToggle; | ||
| } |
There was a problem hiding this comment.
So just to be clear the "global" navigation sidebar is now officially abandoned? Is just want to be sure before we start removing all of this code.
There was a problem hiding this comment.
No I don't think this change means that, it's just this isn't the way we want to access it.
| @@ -690,6 +696,51 @@ function Navigation( { | |||
| /> | |||
| ); | |||
|
|
|||
| const NAVIGATION_SIDEBAR_NAME = 'edit-site/block-inspector'; | |||
There was a problem hiding this comment.
Are we happy for this to be intended not to work in other editors?
There was a problem hiding this comment.
No, I think it should work in both. I'm not sure how to achieve that.
There was a problem hiding this comment.
Presumably the Post Editor also has a sidebar right? We should toggle that one if it's available.
There was a problem hiding this comment.
It does with my latest changes :)
This is not a good idea because it means "edit-site" will be loaded everywhere the block editor is loaded (aka edit-post as well). In general we should avoid behavior in a block that is specific to an implementation of a block editor. (edit-widgets, edit-post, edit-site). So if we find an alternative using the block editor package, that would be better. In some situations we do accept these "hacks" but we need to do two things:
Here's a precedent gutenberg/packages/block-library/src/calendar/edit.js Lines 58 to 70 in 2572104 |
scruffian
force-pushed
the
add/edit-menu-button
branch
from
e93ac9e
to
7e1f838
Compare
scruffian
force-pushed
the
add/edit-menu-button
branch
from
7e1f838
to
b4fc383
Compare
|
Thanks for the help on this one. I think it's good for another review. It would be good to get some design input as well on this new button. @SaxonF @mtias @jasmussen |
youknowriad
reviewed
Nov 17, 2022
| ); | ||
| const { enableComplementaryArea } = useDispatch( interfaceStore ); | ||
| // eslint-disable-next-line @wordpress/data-no-store-string-literals | ||
| const { openGeneralSidebar } = useDispatch( 'core/edit-post' ); |
There was a problem hiding this comment.
What happens if it's neither edit-post nor edit-post. How should the navigation block behave?
youknowriad
reviewed
Nov 17, 2022
| }; | ||
| } | ||
| ); | ||
| const { enableComplementaryArea } = useDispatch( interfaceStore ); |
There was a problem hiding this comment.
I'd probably avoid importing the store here as well as this will bundle the interface package into block-library and that package is only meant to be used in top level packages (edit-post, edit-site, edit-widgets...)
|
Thanks for the PR, this is a quick GIF:
Conceptually this makes enough sense to me, insofar as it gives you a pathway from just the block toolbar (when the inspector is closed) to get access to the main editing interface. As evident perhaps in the GIF, a few issues are surfaced:
Note that the gray background not extending to the bottom isn't present in trunk, but I think it was a recently fixed regression, so a rebase will likely fix it. |
|
I've been thinking about this a bit, and I wonder if this should be built as a "hook/extension" to the block from both "edit-post" and "edit-site" instead of bundling this behavior into the raw block in block-library. In other words, try to hook into the registration of the block and add the block controls. Conceptually it seems very close to the |
1 task
|
Closing in favour of #45772 |
|
🤦 yes |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This adds an "Edit menu" button to the navigation block, which opens the Inspector Controls.
Why?
Based on the designs in #42257, this is something we'd like to see in the block:
How?
We have a button that opens the global navigation editor in the navigation block. This is removed and replaced with a button that simply opens the inspector controls. When we added a tabbed interface to the inspector controls we'll need to ensure that this button opens the correct tab.
This requires us to access the
edit-sitestore from the navigation block, so I have exported it from the package. I'm not sure if this is a good idea cc @youknowriad.One issue with this approach is that it doesn't work in the post editor. Is there a way to detect whether we are in the post editor or site editor? I fear I may be doing it wrong!
Testing Instructions
Screenshots or screencast