Repository navigation
Conversation
github-actions
Bot
added
[Package] Block library
🤖 PR meta 🤖📦 Bundle sizeSize Change: +828 B (+0.01%) Total Size: 8.21 MB 📦 View Changed
⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
|
jeryj
added
[Feature] Link Editing
There was a problem hiding this comment.
🟡 Changes recommended
Unscoped results introduce incorrect entity labeling, relevance ordering, and mixed-version compatibility problems.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Enables Navigation link search across all supported entity types while prioritizing the current link type.
Changes:
- Adds suggestion transformation APIs and documentation.
- Filters media and prioritizes relevant entity types.
- Adds unit and end-to-end coverage.
File summaries
| File | Description |
|---|---|
test/e2e/specs/editor/blocks/navigation.spec.js |
Tests multi-entity Navigation searches. |
packages/block-library/src/navigation-link/shared/controls.jsx |
Applies transformations in inspector controls. |
packages/block-library/src/navigation-link/link-ui/transform-suggestions.js |
Filters and prioritizes suggestions. |
packages/block-library/src/navigation-link/link-ui/test/transform-suggestions.test.js |
Tests suggestion transformation. |
packages/block-library/src/navigation-link/link-ui/test/get-suggestions-query.test.js |
Tests unscoped queries and initial suggestions. |
packages/block-library/src/navigation-link/link-ui/index.jsx |
Integrates transformed suggestions. |
packages/block-library/src/navigation-link/link-ui/get-suggestions-query.js |
Builds unscoped search queries. |
packages/block-editor/src/components/link-picker/link-picker.jsx |
Forwards the transformation callback. |
packages/block-editor/src/components/link-control/use-search-handler.js |
Transforms fetched results. |
packages/block-editor/src/components/link-control/test/index.jsdom.test.jsx |
Tests the new callback contract. |
packages/block-editor/src/components/link-control/search-input.jsx |
Passes transformations to the search handler. |
packages/block-editor/src/components/link-control/README.md |
Documents the new prop. |
packages/block-editor/src/components/link-control/index.jsx |
Exposes the new LinkControl prop. |
packages/block-editor/CHANGELOG.md |
Records the enhancement. |
Review details
- Files reviewed: 14/14 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const { isInitialSuggestions } = suggestionsQuery; | ||
|
|
||
| const results = await fetchSearchSuggestions( val, suggestionsQuery ); | ||
| let results = await fetchSearchSuggestions( val, suggestionsQuery ); |
| normalizeType( suggestion.type ) === priority.type && | ||
| suggestion.kind === priority.kind; | ||
|
|
||
| ( isPriority ? prioritised : rest ).push( suggestion ); |
github-actions
Bot
added
[Package] Block library
|
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. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings affect custom entity links, package compatibility, and result completeness.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 3
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
| if ( kind === 'taxonomy' ) { | ||
| return { type: 'term', subtype: type, perPage }; | ||
| return { type: 'term', subtype: type }; | ||
| } | ||
| if ( kind === 'post-type' ) { | ||
| return { type: 'post', subtype: type, perPage }; | ||
| return { type: 'post', subtype: type }; |
| suggestionsQuery={ getSuggestionsQuery( type, kind ) } | ||
| transformSuggestions={ transformSuggestions } |
| if ( UNSUPPORTED_KINDS.includes( suggestion.kind ) ) { | ||
| continue; |
There was a problem hiding this comment.
This is a good point, but I can't see an easy fix. I think we would need to update Core Data to that fetchLinkSuggestions can take an array of types. Probably worth a followup, or maybe we decided to do that first?
The front page and blog home checks compared ids alone. Posts, terms and media are separate tables, so ids collide: on a fresh site "Hello world!" and "Uncategorized" are both id 1. Unscoped searches put both kinds in one result list, so a term could be labelled "Front page". Reported by Copilot on #82985. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv
|
|
||
| ### Internal | ||
|
|
||
| - `LinkControl`: Add an undocumented `transformSuggestions` prop, so a consumer can filter and order search suggestions before they are shown ([#82985](https://github.com/WordPress/gutenberg/pull/82985)). |
There was a problem hiding this comment.
I don't think simply not documenting the prop is sufficient for us to remove it in future as its still possible someone used it (especially now with AI). Either we should make it public or private.
One option is to private component that adds the prop and wraps LinkControl. That way the prop is only available in the private component. We did this for the ListView component. Happy to help if you think it's a good idea.
There was a problem hiding this comment.
I'm fine making it public as well. I think it's a useful feature to be able to filter results. I didn't want to assume to make it public from the start though.
|
|
||
| await expect( | ||
| searchResults.getByRole( 'option' ).first() | ||
| ).toContainText( DEFAULT_CATEGORY_NAME ); |
There was a problem hiding this comment.
This would also match the page fixture called ${ DEFAULT_CATEGORY_NAME } Notes, so I think we need more strict matching, or to use a different name in the fixture.
getdave
left a comment
getdave
left a comment
There was a problem hiding this comment.
Thanks for tackling this, @jeryj. The round trip through "Add block" to insert other types has been a genuine problem. #77072 lays it out well.
I want to flag a concern before this merges, because I seem to recall we've been here before.
#48740 made the Nav appender default to a Page link, and it got reverted within two weeks in #49126 because categories stopped appearing in search at all. This PR avoids that specific failure by widening the query instead of narrowing it, but I think it inherits the same root cause from the other direction.
fetchLinkSuggestions merges every entity kind into one list and truncates it to perPage before transformSuggestions ever sees it. If a search term happens to match a lot of posts or media, those can fill the shared 20-slot budget and get discarded by the media filter, or just crowd out a relevant page, before the reordering has anything left to work with.
I looks like @scruffian's review comment confirms this and that there is no easy fix here without Core Data supporting an array of types.
None of this is a reason to drop the discoverability fix. But "search everything, then reorder client-side over a shared, truncated pool" feels like the wrong move. What we probably need is a dedicate link search handler and REST API endpoint. In fact I think Classic WP had this for the Classic Editor?
Would it be worth tackling the Core Data change first, so Navigation can ask for results per type instead of reordering whatever's left after truncation? Keen to hear what you both think.
This doesn't need to block the discoverability win indefinitely. I just want to make sure we're not moving sideways/backwards here.
|
Thanks for the reviews @getdave and @scruffian! The 20 search results maybe not including the desired type is an issue I hadn't considered. Glad y'all caught this.
I think this would only help with solving search for media types, and One idea to solve this now: Parallel requests. I want to try two searches, one for our specific type and one global, then filter and append the global search. That should give us at least X results from our specific type, then allow other top matches from other types to still be available. |
Threading the callback down into useSearchHandler meant it only applied to the generic handler, so a consumer passing fetchSuggestions had its transform silently ignored. Wrap the handler in search-input instead: it applies to whichever handler is in use, and use-search-handler goes back to matching trunk exactly, which also removes the unused withURLSuggestion parameter that the old argument order forced. Hold the "CREATE" option back and re-append it, so it still always comes last and consumers never see a suggestion that is not a real entity. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv
Drop PrivateLinkControl. LinkControlSearchInput already carries fetchSuggestions and renderSuggestions as real props that are absent from the README and therefore unsupported, so transformSuggestions follows that instead of adding a new privateApis export. private-apis.js returns to matching trunk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv
It was only handed back out again. The helper answers which entity the block points at; pagination belongs to the caller. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv
normalizeEntity applied the page fallback to suggestions as well as to block attributes, which is a category error: "what should a link with no entity prioritise" is a question about the block. It also had a concrete cost. LinkControl's "Create page" row carries no kind, so it took the fallback, matched the priority of a freshly appended item, and sorted above every real page. Return null for anything that is not an entity and keep the fallback at the one call site that wants it. Non-entities are then never a match, so the create row stays where it arrived, and the hold-and-re-append in search-input goes away. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv
Per the repository's testing guidance, one behaviour per case and no adjacent coverage: - The LinkControl pair asserting the call arguments and asserting the rendered output were two halves of one pass-through. Neither stood alone, so they are now one case covering both. - "removes attachment results" and "keeps posts, pages and terms" were the same filtering behaviour asserted twice. - The two page-fallback cases differ only by input, so they are an it.each. - The alias case only covered `tag`. Covering both aliases closes the `post_format` gap: verified by deleting that alias, which fails only the new case. Also fixes indentation in search-input that Prettier flagged once the merged lockfile's dependencies were installed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv
WordPress always has "Uncategorized", so the specs no longer create a category and no longer hand-roll term cleanup. requestUtils has no deleteAllTerms, and createRecord rejects a duplicate name with term_exists, so an interrupted run used to break every later run at setup. Nothing to leave behind now. The inspector test seeds a page sharing that word, so one search still returns both a page and a term and their order can be asserted. Also strengthens the non-entity case. It asserted that the "Create page" option stayed last in a list where the other suggestion was itself the priority, so order was preserved either way and the test passed even with the guard removed. Against a category it fails as intended. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv
- Make transformSuggestions a documented public prop. Leaving it out of the README does not stop anyone depending on it, so the choice is public or genuinely private, and filtering results is useful on its own. - Match custom types whose slug contains a hyphen. updateAttributes stores the first hyphen as an underscore, so normalize the same way here. That also makes the post_format alias redundant, leaving `tag` as the only name the block and the API disagree on. - The category assertion in the inspector test matched the page fixture "Uncategorized Notes" too, so assert on the type instead. - The list view search assertions could pass against the initial suggestions, which are pages. Wait for a result that only the typed search can produce first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv
An unscoped search merges four requests and keeps the most relevant twenty of the merged list, so the link's own type can be crowded out before the results arrive. Searching "e" against a site with thirteen pages returns twenty suggestions with no page among them. Run a second request scoped to the link's own type alongside the first and merge the two, so that type is always represented. Deduplicate on kind, type and id together, since ids repeat across post types and taxonomies. Order the merged list as: 1. One result of another type, but only when it matches what was typed better than anything of the link's own type does. Searching "uncategorized" from a Page Link leads with that category; searching "contact" when a page is called "Contact" does not, because the page already answers it. Ignored below four characters, where a search is too vague to call any match obvious. 2. Up to three of the link's own type. 3. Everything else, by how well it matches, keeping the relevance order the results arrived in within each tier. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv
A whole-title match was treated as better than a start-of-title match, so searching "coffee" from a Page Link led with a category called "Coffee" even though a page called "Coffee Guide" matched every character typed. A shorter title is not a better answer. Another type now leads only when nothing of the link's own type matches at all. Tiers still order results within a group, so a whole-title match still sorts above a start-of-title one among pages. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv
Dropping every attachment took something away that trunk offers. A Page Link only ever queried pages, so media never reached it, but Custom Link has no top level type in its query and is already unscoped: searching "image" from one on a site with a media library returns attachments today. That is the one link that can point at a file, since it is an arbitrary URL rather than a bound entity. So the filter now applies to everything except a Custom Link, where `media` stays. Widening adds posts, categories, tags and formats to the typed links without also adding files to them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv
e3d7c7c to
712e860
Compare
|
Thanks for working on this. I tested it in Playground, and it feels like a much smoother UX. I’m not sure whether the 
Another thing that might become more relevant now is that we don’t have a “No results found” state, since this is neither a page nor a category, etc. 
I’m also curious whether it would be relatively easy to add anchor links to the Link UI. It’s currently a big PITA to link to an anchor. Related: #9660 |
|
Thanks for the review, @hanneslsm! The As for an empty state, that would be a good addition. It's the same as trunk though. If there's not already an open issue for that, could you create one? For Anchor links, are you hoping for the Anchor links to be searchable from the link control? |
Yes, done: #83324
Correct. I often have a button in the navigation ("Contact us now") which links to the #contact anchor on the front page. Currently, the easiest way is to use a custom link, but it would be great if I could select the anchor directly in the Link UI, just like a page or post. |
Oh, so it would be an anchor link on a separate page? Not an internal page anchor link? I'll be honest, I think that'd be a big lift to implement, and I see a lot of complexity in how to integrate this in a way that doesn't make the base experience more difficult. Feel free to explore it though if you feel strongly about it! Thanks for creating #83324! |
Even linking to an internal page would be a huge improvement for every one-pager websites. |
This might be more easily doable! Feel free to open an issue! |
Great to hear! I think this already is about the problem: #9660 |
Let a consumer filter, order or add to the search suggestions before they are shown. It runs inside the entity search, after the fetch and before the front page is labeled and the "create" option is added, so that option always stays last. URL input skips it. A suggestion the transform adds can bring its own icon and type label. Ported from the review work on #82985. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxqx12sRhNixE8aH4FAfm2
Let a consumer filter, order or add to the search suggestions before they are shown. It runs inside the entity search, after the fetch and before the front page is labeled and the "create" option is added, so that option always stays last. URL input skips it. A suggestion the transform adds can bring its own icon and type label. Ported from the review work on #82985. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxqx12sRhNixE8aH4FAfm2
Let a consumer filter, order or add to the search suggestions before they are shown. It runs inside the entity search, after the fetch and before the front page is labeled and the "create" option is added, so that option always stays last. URL input skips it. A suggestion the transform adds can bring its own icon and type label. Ported from the review work on #82985. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Lxqx12sRhNixE8aH4FAfm2
What?
Closes #77072
Makes the Navigation Link UI search every entity type, instead of only pages. Also, adds functionality that was previously impossible: convert a navigation link block from one type to another (category -> page, page -> category, etc).
Why?
Adding anything but a page to a Navigation means leaving the search box: click the appender, ignore the field that just took focus, click "Add block", pick "Category", land back in a search box, and type (or re-type) what you wanted. The UX is clunky for anything other than the main path.
How?
1.
getSuggestionsQuerywidens the search from one entity type to manyPreviously a navigation link set the query to ONLY search its own type. Now, we search all of them, but pass the navigation link's type as the initial search suggestion type. So, you are in a page link, it will default to showing you just pages when you open it.
Searching a string will now return all entity types that match.
2.
transform-suggestions.jsshapes what comes backIt receives the results and initial type from the link control, then reorders the results to put its matching type first. So, by default, pages return first, and everything else comes after:
It drops attachments (
kind: 'media') as those are not navigation link types AFAIK.3. Wiring
transformSuggestionsis passed from the Navigation Link Block into LinkControl and LinkPicker. There are barely any changes to LinkControl and LinkPicker.The prop is deliberately absent from the README to default it to Private, matching
fetchSuggestionsandrenderSuggestionsonLinkControlSearchInput.Notes for review
noURLSuggestionhas had no effect since Improve (and relax) search vs direct URL entry detection in Link Control #51011 removedwithURLSuggestionfromhandleEntitySearchwhile leaving the argument at the call site. Untouched here; worth its own fix.navigation-list-view.spec.jsclaimed to assert result types but read the result label, which passed only because the fixtures are titled "Test Page 1/2/3". Fixed in its own commit.Testing Instructions
+appender.Screenshots or screencast
Before
Screen.Recording.2026-09-16.at.1.16.51.PM.mov
After
Screen.Recording.2026-09-16.at.1.15.13.PM.mov
🤖 Generated with Claude Code
https://claude.ai/code/session_01BLn4hqAvfFyNGLLgsSkCcv