Let navigation-search filter by a type attribute - #4144
Conversation
There was a problem hiding this comment.
Approved — no actionable issues found.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
6110f71 to
ef216c9
Compare
Docs preview (local build)Handbook preview: https://docs-v3-preview.elastic.dev/elastic/docs-builder/pull/4144/ |
There was a problem hiding this comment.
Requesting changes: there is one medium-severity correctness issue in the inline comment that should be addressed before merge.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
| const { setTypeFilter } = useSearchActions() | ||
| const typeFilter = parseTypeFilter(type) | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
[MEDIUM] type filter is applied after the first render, so the first query can be unfiltered
setTypeFilter(typeFilter) runs in useEffect, which executes after render. If the shared store still has a non-empty searchTerm from a previous mount, useNavigationSearchQuery can fire immediately with stale typeFilter='all' before this effect runs, returning mixed docs/API results even when type="api" is set.
Please initialize the filter before query evaluation (for example by deriving query filter directly from the prop for initial render, or by ensuring mount-time state is set synchronously before NavigationSearch runs its query hook).
There was a problem hiding this comment.
Fixed in 5a7a77f — the type filter is written during render, before NavigationSearch mounts, so a cached health check cannot query with the previous filter.
The API sidebar must send type=api without changing unfiltered callers. Clearing the box must keep that host filter. Co-Authored-By: Cursor Grok 4.6 <noreply@anthropic.com>
A cached health check mounts search on the first render. An effect applies the filter after that query, so a leftover search term can run unfiltered. Co-Authored-By: Grok 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
5a7a77f to
49d3ff7
Compare
There was a problem hiding this comment.
Requesting changes: the inline comment identifies a correctness issue where the new type behavior is shared globally across instances instead of being instance-scoped.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
The attribute is already known on this render. Reading it from the store lets a cached health check query before the filter is applied. Co-Authored-By: Grok 4.7 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Approved — no actionable issues found.
What is this? | From workflow: PR Review
Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
<navigation-search type="api">now requests/v1/navigation-searchwithtype=api. Callers that omit the attribute stay unfiltered.Affects: Site UI, Search
Prompt summary: Second stacked PR for the Jump to page API story. Implement elastic/docs-eng-team#858 on top of elastic/docs-builder#4143: add a
typeattribute, apply it to the store filter, and keep unfiltered callers unchanged.Stack: 2 of 5, on top of #4143.
Why
The API sidebar needs Jump to page to request only API results. Every other host of the element stays unfiltered.
Closes elastic/docs-eng-team#858
What
Type attribute
The custom element accepts a
typestring.docsorapiis passed into the search query on that render. Any other value, including an omitted attribute, keeps the request unfiltered.Placeholder
When
typeisapiand noplaceholderis set, the box showsJump to API.Verify
Out of scope: Mounting the box in the API sidebar. That is the next PR.