Navigation: select list view tab on contentOnly#75024
Closed
MaggieCabrera wants to merge 7 commits intotrunkfrom
Closed
Navigation: select list view tab on contentOnly#75024MaggieCabrera wants to merge 7 commits intotrunkfrom
MaggieCabrera wants to merge 7 commits intotrunkfrom
Conversation
|
Size Change: +170 B (0%) Total Size: 6.84 MB
ℹ️ View Unchanged
|
MaggieCabrera
added
[Block] Navigation
|
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. |
mikachan
previously approved these changes
Jan 28, 2026
Member
mikachan
left a comment
mikachan
left a comment
There was a problem hiding this comment.
This works well for me, and the code changes seem simple enough 🙌
mikachan
reviewed
Jan 28, 2026
packages/block-editor/src/components/inspector-controls-tabs/index.js
Outdated
Show resolved
Hide resolved
Member
|
I get the idea of this PR but I can't reproduce the problem in a fresh install Can you provide some more precise steps to actually see the problem in action before the PR? gb75024.mp4 |
SirLouen
added
the
[Status] Needs More Info
Contributor
Author
Is that trunk? This is what I see when I do the same using the adventurer theme and one of the header patterns. It needs to be a pattern that isn't synced so we are in contentOnly mode. Check the right sidebar how it says pattern. This is trunk after clicking on the nav block: 
My test theme is adventurer |
getdave
previously requested changes
Jan 28, 2026
Contributor
There was a problem hiding this comment.
Thank you for the PR 🙇
I haven't managed to test this yet, but based on the description I;m concerned that we might be making too large of a change here.
This means that when you select a template part it shows List View immediately which I believe isn't what the team working on this were aiming for.
Can we take a pause here whilst I review?
If we haven't already please can we consider what is outlined in Proposed solution on the related Issue?
Contributor
|
I've requested a review from @talldan who I know has been instrumental in the wider contentOnly work. Perhaps it would be possible to come up with a solution that solves for Navigation whilst also remaining faithful to the goals of the wider contentOnly work. |
Contributor
|
I'll add some comments ot the Issue with clearer intent and rationale. |
Member
Ok, I've been able to reproduce it locally I wondered now: by default, it's selecting the leftmost element in the Inspector Why not default to List View instead, on the left in all blocks like a canonical thing? It's intuitive (at least in LTR mindset), to have by default selected the leftmost tab. |
Contributor
|
I'm not sure about having the second tab as the default for patterns. Agree with @SirLouen that it feels expected for the first tab to be selected. I don't mind the idea of moving the List View tab to first. The current order matches what was proposed in Block Attribute Groups, so that might be a good place to discuss it. In terms of other ideas, the suggestion from @andrewserong (#74843 (review)) seems like a good one (switch to the List View tab whenever 'Edit Navigation' is clicked), and that's something I started looking into, but there's no API for selecting an inspector tab, so it would need something new. My other idea is that perhaps whenever a block that has the gutenberg/packages/block-editor/src/components/block-card/index.js Lines 122 to 125 in fdb5877 |
Contributor
Yeah, if possible, I feel like defaulting to the list view tab should happen based on clicking "Edit Navigation" as it seems to be a clearer user intent than always switching to the list view. Otherwise, the current state of this PR feels a bit stuck to me as it buries the overview of the pattern/section. In the following video, note that when I select the CTA pattern that contains the buttons, you stay on that view even when selecting other blocks within the pattern. Then when selecting a template part, because we're on the list view, we don't see the Site Title or other blocks, because we're on the list view: 2026-01-29.14.31.24.mp4Put another way: I think the content tab is more suited as an overview / first place to land, whereas the list view feels a bit more like something you go to for making edits and changes. |
Contributor
|
Thanks for all the comments here. I've updated the Issue description to provide a better overview of user requirements and various criteria. Hopefully this helps to inform the direction a bit more.
This is a good instinct. It signals clear intent to edit Navigation so it's clearly a good pattern to adopt. |
getdave
dismissed
their stale review
Now outdated in light of recent changes.
Contributor
For me this suggests that the original acceptance criteria is correct. If the user signals clear intent to "Edit Navigation" then we should show the interface of the block that is best suited to editing. I hope that makes sense?
|
Member
rebases are loaded with danger 🤣 |
MaggieCabrera
force-pushed
the
navigation-show-list-contentonly
branch
from
ea4cc8f to
1daf4d9
Compare
MaggieCabrera
force-pushed
the
navigation-show-list-contentonly
branch
from
1daf4d9 to
ba3a001
Compare
getdave
reviewed
Feb 10, 2026
Contributor
getdave
left a comment
getdave
left a comment
There was a problem hiding this comment.
Whoops! I just realised that we aren't targeting Navigation here.
The condition of the related Issue is that only when someone clicks the Edit Navigation button we select the List View.
I don't think we should merge if it's going to select the List View for all container blocks in contentOnly.
MaggieCabrera
force-pushed
the
navigation-show-list-contentonly
branch
from
04b94cd to
7f5ea5c
Compare
mikachan
dismissed
their stale review
Approved too early, before we'd had wider discussions
Contributor
|
I'm seeing some strange behaviour when I have two navigation blocks in a section: Screen.Recording.2026-02-12.at.16.59.05.movThe main problem is that it's not consistent. I think what should happen is:
|
Contributor
Author
What I see in your video is not caused by this PR, but this PR makes it more apparent. I don't know if I agree with 1, but we can discuss it. I think the behaviour I would expect is that if I'm in the content tab and click on another nav block, it stays in the content tab and focuses it. And if I'm list view and click in another nav block, it opens its accordion on the list view, instead of navigating you out to the content tab. |
Contributor
Author
|
Updated the PR's description |
getdave
reviewed
Feb 13, 2026
getdave
reviewed
Feb 13, 2026
getdave
reviewed
Feb 13, 2026
Contributor
getdave
left a comment
getdave
left a comment
There was a problem hiding this comment.
Thanks for tackling. I'm yet to manually test this but I can see how it would work.
Thinking about this some more...
I'm wondering if longer term we should update the design of selectBlock to accept metadata about UI
selectBlock(clientId, initialPosition, {
ui: {
inspectorTab: "list-view",
},
});
Alternatively separate them like this:
dispatch(blockEditorStore).selectBlock(navigationBlockId);
dispatch(blockEditorStore).__unstableRequestInspectorTab("list-view");
That would be an explicit signal of intent that could be triggered rather than reactively inferred like we do in this PR.
Perhaps that's a longer term goal but we should consider it. There are other occasions when we want to select a block with a given portion of UI open.
The real question for now is: can we proceed with a navigation-specific approach for 7.0 whilst still leaving the code open to extension in further releases without any backwards compatibility concerns?
MaggieCabrera
force-pushed
the
navigation-show-list-contentonly
branch
from
7f5ea5c to
1e9c1a3
Compare
|
Flaky tests detected in 1e9c1a3. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22060734478
|
Contributor
Author
@getdave I think we shouldn't have back compat concerns if we go with this for 7.0 since everything we are doing is (now, thanks for the spot) in private selectors. I prefer your separate action approach over extending selectBlock, it keeps APIs focused and it's not tied to the block selection. Maybe we can explore this solution on a different issue/PR that is not timeboxed for 7.0, what do you think? |
Contributor
Author
getdave
reviewed
Feb 17, 2026
Contributor
getdave
left a comment
getdave
left a comment
There was a problem hiding this comment.
Thanks for all the work on this @MaggieCabrera.
I've left some inline comments on performance optimizations we could make. Beyond those, I wanted to surface a broader question about approach.
As previously mentioned, this implementation uses reactive detection - checking if the selected block is a navigation with contentOnly children. This works well and solves the UX problem. The trade-off is it opens List View in scenarios beyond the "Edit Navigation" button click (like undo/redo, programmatic selection, or clicking the nav directly in canvas).
I think your PR #75578 with the explicit action API might be the stronger path forward. The request inspector tab approach is:
- Very explicit about user intent (only on button click)
- Reusable for other blocks in future
- Follows Redux action-dispatch patterns
- Has simpler performance characteristics
The downside is it's a bit more infrastructure, but it feels like a cleaner foundation. Since everything stays behind private APIs it could be safe to land for 7.0.
Both approaches work! Curious what others think about the trade-offs? Happy to discuss further.
| const shouldShowListViewForNavigation = useMemo( () => { | ||
| return ( | ||
| selectedBlockName === 'core/navigation' && | ||
| contentClientIds?.length > 0 && |
Contributor
There was a problem hiding this comment.
contentClientIds memo is invalidated on each change to the clientIds. We can compute the length outside of the memo to make it a Boolean transition.
Comment on lines
+93
to
+95
| if ( ! tabs?.length ) { | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
We can compute as a Boolean outside to reduce re-runs of the effect.
| // Check if we should show List View tab for navigation block (Edit Navigation context) | ||
| const shouldShowListViewForNavigation = useMemo( () => { | ||
| return ( | ||
| selectedBlockName === 'core/navigation' && |
Contributor
There was a problem hiding this comment.
We could compute as a Boolean outside and then the change becomes a Boolean transition from :"is navigation" to "is not navigation".
| select( blockEditorStore ); | ||
| const selectedClientId = getSelectedBlockClientId(); | ||
| return { | ||
| selectedBlockName: selectedClientId |
Contributor
There was a problem hiding this comment.
We could instead return whether or not the block is navigation directly.
The issue is that the selectedBlocklClientId will change anyway on each update so it invalidates the perf benefits.
| }, [] ); | ||
|
|
||
| // Get the actual selected block name and clientId to detect when navigation block is selected via "Edit Navigation" button | ||
| const { selectedBlockName, selectedBlockClientId } = useSelect( |
Contributor
There was a problem hiding this comment.
Question: does clientId already equal the selectedBlockClientId?
Comment on lines
+97
to
+101
| if ( | ||
| shouldShowListViewForNavigation && | ||
| selectedTabId !== TAB_LIST_VIEW.name && | ||
| ! hasUserSelectionRef.current | ||
| ) { |
Contributor
There was a problem hiding this comment.
// Derive boolean values to be used as effect deps
const hasTabs = tabs?.length > 0;
const isNotListViewTab = selectedTabId !== TAB_LIST_VIEW.name;
const hasSelectedNavBlock = !! selectedBlockClientId;
Contributor
Author
|
Closing in favor of #75578 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
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?
Closes #74894
Makes the List View tab the default active tab when selecting a Navigation block via "Edit Navigation" button, instead of the Content tab.
Why?
When selecting a Navigation block via "Edit Navigation", users should immediately see the List View tab which provides essential editing functionality. Currently, the Content tab is shown by default, requiring an extra click to access List View.
How?
InspectorControlsTabscomponent to detect when Navigation block is selected with contentOnly childrenuseLayoutEffectto switch to List View tab synchronously before paint to prevent flash__unstableIsListViewPanelOpenedand__unstableGetListViewExpandRevisionto manage List View panel stateTesting Instructions
Screen.Recording.2026-02-13.at.10.57.32.mov