Update: Auto insert page-link on navigation sidebar. #48745
Closed
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
jorgefilipecosta
added
[Type] Enhancement
|
Size Change: +35 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 20c1b48. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4326246689
|
|
Nice, this makes it much easier to add Page Links to the menu which is a very common use case. Before before.mp4After after.mp4 |
|
@jorgefilipecosta, @jameskoster is this alternative to #48724? |
Mamaduka
reviewed
Mar 4, 2023
| function PageLinkInserter( { rootClientId } ) { | ||
| const { insertBlock } = useDispatch( blockEditorStore ); | ||
| const { getBlockOrder } = useSelect( blockEditorStore ); | ||
| const insertPageLink = useCallback( () => { |
There was a problem hiding this comment.
Nit: Probably no need for useCallback here. The SidebarButton component isn't memoized and will re-render whenever its parent re-renders. So we don't need to pass a referentially stable callback to it.
No, this is in addition to. 48724 is important because when building any navigation menu, you're likely to be using Page Links. The way it behaves is that if you only ever have menu items in your menu, you get this "light weight" navigation building flow where navigation items are inserted directly, skipping the quick inserter. This is similar to how the Buttons block defaults to inserting buttons, skipping the need to pick "button" from the inserter. As soon as you have other block types inside the navigation wrapper, such as a site logo or a search field, you get the regular quick inserter, which per 48724 should still pin "page links" for quick access. |
|
Speaking of: this PR is working well for me in that it defaults to the Page Links:
It doesn't have the same behavior as the navigation block, though, as described here, where as soon as you have a block inside the nav wrapper that isn't a menu item, it falls back to the regular inserter. It should probably be the same behavior here. Can we use the exact same componentry as the off-canvas navigation editor has? Ideally the code would be reused. |
I don't see the logic for this behavior in the code. Currently, PR replaces block inserter with a custom inserter that only handles Page Links. |
We currently don't have this logic, but it is a simple change to include. I'm unsure if this PR is needed, given that #48740 was already merged. |
Mamaduka
added
[Feature] Site Editor
|
I've removed backport label until we have a final decision for this enhancement. |
|
Using WP 6.2 beta 4 Before: After:
I am puzzled that clicking the appender a Page Link is automatically added. I would believe that we would through consistency use a similar method as in template page Navigation would use. Making the Browse sidebar Navigation a kind of duplicate of the in page template Navigation. Example from the in page template Navigation.
Btw |
I don't really understand why we do this 🤔 Adding a Page Link is inherently the most common behavior, regardless of whether the menu contains other block types, no? |
|
With this PR, we can only add the |
I think the idea, yes, the idea was only to insert page links. If people want other blocks, the page link creation popover allows changing to other blocks. |
|
Closing this PR, as right now the inserter is disable and what the inserter will look like is not yet decided. |
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.
This PR auto inserts page links on the navigation sidebar.
cc: @jasmussen, @jameskoster