Try show navigation menu name in the block inspector, toolbar, list view and breadcrumbs #46457
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
talldan
added
[Type] Enhancement
|
Size Change: +118 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
carolinan
reviewed
Jan 11, 2023
| @@ -47,6 +55,27 @@ export const settings = { | |||
| }, | |||
| ], | |||
| }, | |||
| __experimentalLabel: ( { ref } ) => { | |||
| // Attempt to find entity title if block is a template part. | |||
There was a problem hiding this comment.
Nit: I think this should say navigation, not template part?
|
The PR works well for me. If I update a name, it is correctly displayed after saving. I think we need to discuss if the inspector should show both the customized name and the normal block title. |
apeatling
reviewed
Jan 16, 2023
There was a problem hiding this comment.
After save, this works well.
The one thing that was unexpected was changing the name under the advanced menu and it not updating everything until I hit save. I would expect everything to change visually before save, and only be persisted after hitting save. This is the pattern for everything else in the editor.
One quirk with the current way it works -- when I update the name the dropdown in the menu panel changes but not the selected state:
|
@draganescu I think this is related to the discussion around showing the menu name in the list view. |
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?
Related #43261
Fixes #42154
Shows the navigation menu name in the block inspector, list view and breadcrumbs.
Why?
If we explore the navigation block being considered a 'synced' block
How?
__experimentalLabelfor the navigation block, in much the same way as the template part block implements this.useBlockDisplayTitlehook for showing the title in the inspector. Does this for template part, reusable and navigation blocks.A lot of this code could be improved. For example
useBlockDisplayInformationshould really return the data thatuseBlockDisplayTitledoes. Hard coded checks for block types also shouldn't be necessary in the long run.For accessibility, I think it'd be good if some of these places did still include the normal block title as part of the aria label or description. Sighted users can often infer this from the block logo and other visual clues, but that's not always possible when using a screen reader.
It's an existing bug, so a fix could be attempted in a separate PR.
Testing Instructions
Prerequisite: Ensure your navigation block menu isn't called 'Navigation'.
Testing Instructions for Keyboard
Screenshots or screencast
Toolbar and Inspector
List View
Breadcrumbs