-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Add View button to navigation link blocks #70986
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
Conversation
|
Size Change: +219 B (+0.01%) Total Size: 1.91 MB
ℹ️ View Unchanged
|
- Move View button implementation from navigation-link block to editor hook - Create navigation-link-view-button.js hook following pattern-overrides.js pattern - Hook targets core/navigation-link and core/navigation-submenu blocks - Use __unstableBlockToolbarLastItem to position button as last item - Remove router dependency issues by using editor hook context - Register hook in editor hooks index
- Replace router-based navigation with editor's onNavigateToEntityRecord - Add conditional focusMode support to onNavigateToEntityRecord - Disable focusMode for View button navigation for better UX - Maintain backwards compatibility for existing usage - Only show View button when block is selected
- Add type === 'page' condition to restrict View button to pages only - Prevents button from appearing on other post types (books, posts, etc.) - Maintains existing behavior for custom links (no button shown)
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Flaky tests detected in 793c1e4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/16907400019
|
mikachan
left a comment
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.
This is testing well for me, nice work!
Following the testing instructions:
- Verify the "View" button appears in the block toolbar ✅
- Verify it navigates to the page in edit mode ✅
- Test in both site editor and post editor contexts ✅
- Verify the button does not appear for custom URL links ✅
- Verify navigation shows the full editor interface (not focused mode) ✅
I've left some minor inline comments, but otherwise this is working great.
|
Thanks for review @mikachan 👍 |
andrewserong
left a comment
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 really like the idea for this feature — making it easier for folks to quickly navigate to edit a page without having to go through the menus 👍
A couple of things potentially to tweak (or maybe get feedback from @WordPress/gutenberg-design) — should we call it "Edit" or "Edit postType" instead of "View"? View made me think that clicking on it would open a preview, whereas Edit might be clearer / more consistent with the Edit button for template parts?
Here's the template part controls:
And here's the View button on a link:
The other issue I ran into is that by switching focusMode to false, we lose the Back button in the document bar, so it's not showing after switching to the page we want to edit:
E.g. Back button when editing a template part:
I've left a separate comment about that — I wonder if there's a way to preserve it somehow? Not necessarily a blocker, but when navigating to another page, it'd be good to make it easy for a user to get back to where they were.
| onNavigateToEntityRecord( { | ||
| postId: id, | ||
| postType: type, | ||
| focusMode: false, |
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.
While switching focusMode to false is good for ensuring we land in the editor with the page extending to the edges of the canvas, this appears to remove the Back button in the DocumentBar. It would usually show here:
I think it'd be great if we can (somehow) preserve that so that it's easy for users to get back to the template they were editing.
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 wasn't sure whether we'd want that border around the content which is why I added the ability to conditionalise focusMode. I think it's worth considering but I'd say this is a call for @richtabor.
Rich using @andrewserong's screen grab as a guide would you be happy for clicking View to take you to the page in that mode (i.e. with a back button and the border around the content)? Or would you prefer the experience currently in this PR (i.e. no back button and no border)?
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 wasn't sure whether we'd want that border around the content which is why I added the ability to conditionalise focusMode.
I think avoiding the border we get from focusMode was the right call, so I don't mind it being set to false here. The main question I have (and this is also my naivety as to how the Back button works) is whether we can also somehow get the Back button working while not in focusMode 🤔
Not necessarily a blocker to figure out, either, if folks are happy with how this PR is behaving — we could always look into how to add in the Back button behaviour separately if it proves tricky.
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.
The main question I have (and this is also my naivety as to how the Back button works) is whether we can also somehow get the Back button working while not in
focusMode🤔
Here is where Back is set
gutenberg/packages/edit-site/src/components/block-editor/use-site-editor-settings.js
Lines 24 to 30 in 2826f74
| const isFocusMode = | |
| location.query.focusMode || | |
| ( location?.params?.postId && | |
| FOCUSABLE_ENTITIES.includes( location?.params?.postType ) ); | |
| const didComeFromEditorCanvas = | |
| previousLocation?.query.canvas === 'edit'; | |
| const showBackButton = isFocusMode && didComeFromEditorCanvas; |
The Back button appears in focus mode when you've navigated from the main editor canvas (canvas=edit) to a focused entity. It's controlled by URL parameters and browser history.
I think the Back button is pretty important so I'm going to remove the changes I made and allow it to land with the border.
However, we'll probably want to invent some new mechanic that allows for the Back button to appear in non-focus contexts.
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.
However, we'll probably want to invent some new mechanic that allows for the Back button to appear in non-focus contexts.
Agreed — currently if you're viewing a page within focus mode there's a couple of other peculiarities, for example if you select to "Show template" from within focus mode while iewing a page, then the Back button in the DocumentBar will overlap with the template icon. But that's something to fix in the DocumentBar component, I believe:
It'd be nice to have a param we can set that enables navigating back without the focus mode border, so we'll wind up needing to fix that DocumentBar issue anyway.
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.
For fixing up the DocumentBar and overlapping Back button, I've opened up: #71183
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.
Tiny update: I've merged #71183, so the DocumentBar will be fixed once this is either rebased or merged.
|
I want to not block this PR because the feature is good and valid. I do feel like there are still challenges with the navigation block which are not fundamentally mitigated by this one, i.e. it's still hard to select an individual menu item, and even then, the menu item can be either a "link" or a "page"—does the View button work on one, not the other? In that vein, I've some good hopes that a contentOnly representation of the navigation block can help make that clearer, so the feature likely needs to work regardless. There's also a great deal of overlap here with the much discussed "navigation mode", where any link on the page you should be able to follow. Like when you hold ⌘ in VS Code and click a link, you go to it. Similarly here, should a "View" functionality be present both on page links but also normal links? How about category links? What about just a Button block you inserted, should you want to follow that? And would holding ⌘ actually be a good way to do it? How about if we add the option to the ellipsis menu, but allow that ellipsis menu to take over the browser's right-click menu? Again, I don't know that any of those arguments should block this feature. I'm mostly worried about building something that's a little too specific for just one block type, when we know we'd actually like this feature to exist for almost any link in the interface, thus potentially benefitting from a more generic placement for the button (i.e. ellipsis menu available on right click and perhaps holding ⌘ to also navigate to a link). |
|
Very vaguely related: #45264 |
|
@jasmussen You make an interesting point and thank you for the feedback. As you've noted, adding this "View" option to Navigation (and yes it only applies to Menu Items that link to "entities") doesn't necessarily preclude the ability to augment other blocks with similar in the future. Indeed, it's simple enough to experiment with this and get real world feedback on its utility before undertaking a larger effort to do the same across the entire Editor experience. It would be simple enough to restrict this to when the Nav Item blcoks are I'll defer to yourself and perhaps @richtabor who has previously mentioned the need for this functionality. |
|
I'm happy to not block this, especially if we believe the implementation is clear and clean enough that without back compat concerns, we can re-work it and expand it at a later time, if that's the direction we choose to go. |
|
I think we should make this a content only feature. As the Nav block does not currently have a contentOnly representation this will allow us to merge an iterate. |
|
@scruffian With the additional of the
|
scruffian
left a comment
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.
Looks good. Let's bring this in to help us iterate on write mode.
andrewserong
left a comment
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.
Looks good. Let's bring this in to help us iterate on write mode.
Agreed — now that it's guarded behind the contentOnly check this feels very minimally invasive to land and then continue to iterate on, so I think this is a good place to start 👍
|
I added an issue to capture the discussion around navigating to links: #71187 |
|
Note: this feature is in WP 6.9 but gated to when the block (the individual Navigation Link) edit mode is |
What
Adds a "View" button to the block toolbar for navigation link blocks that represent Pages (only). When clicked, the button navigates to the corresponding page in edit mode within the current editor context.
Note this doesn't not support other post types but could be extended to do so in the future if necessary.
Why
This enhancement improves the user experience by allowing quick navigation to pages while maintaining the editing workflow. Users can now easily jump to different pages without losing their current editing context or unsaved changes.
How
onNavigateToEntityRecordto navigate within the editoronNavigateToEntityRecord Enhancement
The
onNavigateToEntityRecordfunction has been enhanced to support conditionalfocusModeparameter handling:focusMode: true)focusMode: falseis explicitly passed, the parameter is entirely removed from the URL query stringfocusMode: falseto show the full editor interface rather than just the focused content areaTesting
Screenshots