Navigation: Use the shared list view implementation, not a custom one#75694
Open
Navigation: Use the shared list view implementation, not a custom one#75694
Conversation
scruffian
added
[Type] Code Quality
github-actions
bot
added
[Package] Block library
|
Size Change: -1.02 kB (-0.01%) Total Size: 6.84 MB
ℹ️ View Unchanged
|
scruffian
requested review from
ajitbohra,
fabiankaegy,
juanmaguitar and
tellthemachines
as code owners
|
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. |
jeryj
reviewed
Feb 20, 2026
Contributor
jeryj
left a comment
jeryj
left a comment
There was a problem hiding this comment.
Drive by review. Glad we'll rely on the existing list view!
| * @param {Function} props.setInsertedBlock Callback to update inserted block state. | ||
| * @return {Element|null} The additional content or null. | ||
| */ | ||
| export function AdditionalBlockContent( { |
Contributor
There was a problem hiding this comment.
I think this would be better named as NavigationLinkUI. I think it's only named AdditionalBlockContent because that's the name of the prop it's being passed to?
| @@ -1,304 +0,0 @@ | |||
| /** | |||
Contributor
There was a problem hiding this comment.
What actually changed from this file? It would make it easier to review if we just exported the things we needed from this file first, then after we feel it's good, do the refactor to split it into different files?
mikachan
reviewed
Feb 20, 2026
| clientId={ insertedBlock?.clientId } | ||
| link={ insertedBlock?.attributes } | ||
| onBlockInsert={ handleSetInsertedBlock } | ||
| onClose={ () => { |
Member
There was a problem hiding this comment.
Do we need this arrow function here?
scruffian
requested review from
a team,
ObliviousHarmony,
adamsilverstein,
desrosj,
ellatrix,
gigitux,
gziolo,
nerrad,
ntsekouras,
ntwb,
oandregal,
peterwilsoncc,
spacedmonkey,
swissspidy,
t-hamano and
talldan
as code owners
github-actions
bot
added
[Package] DOM
5 tasks
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…stViewHeader Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
scruffian
commented
Feb 25, 2026
| * @param {string} props.clientId Block client ID. | ||
| * @return {Element} The header component. | ||
| */ | ||
| export default function NavigationListViewHeader( { clientId } ) { |
Contributor
Author
There was a problem hiding this comment.
It looks like there's a lot of changes here, but really its just that we are sourcing the data from hooks rather than passing it as props.
The NavigationMenuSelector used useEntityProp without an ID, relying on EntityProvider context to resolve the current menu title. After moving the menu selector from MenuInspectorControls (which was wrapped in EntityProvider) to the shared ListViewPanel (which is not), the title was always undefined, causing the "Test Menu" button to never appear. Get the title directly from the navigationMenu entity record already fetched by useNavigationMenu(currentMenuId) instead. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
The shared ListViewPanel uses "No items yet." instead of "This Navigation Menu is empty." for the empty state message. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
getdave
reviewed
Feb 26, 2026
| const showBlockTitle = isSelectionWithinCurrentSection; | ||
|
|
||
| // Get custom list view configuration from block type | ||
| const listViewConfig = blockType?.listView || {}; |
Contributor
There was a problem hiding this comment.
Could this also allow a function?
talldan
reviewed
Feb 26, 2026
| blockSettingsMenu: LeafMoreMenu, | ||
| additionalBlockContent: NavigationLinkUI, | ||
| panelHeader: NavigationListViewHeader, | ||
| }, |
Contributor
There was a problem hiding this comment.
I think it might be good to expand the reviewers with it being a new addition to the block API. I'm not sure if there are many settings like this that expand the function of block supports. I'm personally not against the API, but I'm also wondering about the alternatives.
When it comes to the props, blockSettingsMenu and panelHeader both seem fairly obvious and useful.
additionalBlockContent isn't very self-documenting. This is what shows the Link popover when adding a new block. Can it be used for adding other things other than a popover? That's the only one that I think might be worth making private, as it has the potential to break things badly if misused.
With it being a public API, some docs should be added. It might be worth considering stabilizing the same props of PrivateListView along with this.
Contributor
There was a problem hiding this comment.
One other thought. Comparing the Block Settings Menus, I wonder if it's worth trying to standardize:
Button
Nav Link
Move Up / Move Down could be permanent additions to the normal menu when in contentOnly mode.
Add submenu link could be added through the slotfill that the normal menu exposes (though not completely sure if it works in contentOnly mode).
Delete is already present for both, but labelled differently.
Duplicate / Add before / Add after seem useful for nav links, so could be kept for both.
Maybe with that the custom blockSettingsMenu isn't needed
This was referenced Feb 26, 2026
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
5 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?
Updates the ListView implementation in the Navigation block to use the same components as other blocks (List, Buttons, Social Icons) rather than having its own custom implementation.
Why?
It's generally better for us to have one implemention for features rather than duplicating, so the UX stays consistent and so we have less code to maintain.
How?
This PR refactors the list view system to be extensible, allowing any block to customize its list view presentation while using the same underlying infrastructure.
New listView block configuration:
Architecture:
- Reads blockType.listView configuration
- Renders custom components at specific extension points
- Removes hardcoded navigation special cases
- Wraps the list view header with navigation-specific state
- Uses the same hooks as the main edit component (useCreateNavigationMenu, useConvertClassicToBlockMenu)
- Provides menu selection, creation, and classic conversion callbacks
- Renders the menu selector dropdown in the list view panel header
- Consumes navigation context for current menu ID and callbacks
- Only shown when blockEditingMode === 'default'
- Provides inline link editing UI directly in the list view
- Handles entity binding for navigation links
- Auto-cleanup of empty navigation links
Testing Instructions
Basic Functionality
Pattern Context
Menu Selection
Creating Menus
Classic Menu Conversion (if classic menus are available)
Inline Link Editing
Block Editing Modes
Visual Regression
Screenshots or screencast
There should be no visual difference,