Add Header Search Section for Improved User Experience#496
Open
Ansita20 wants to merge 8 commits into
Open
Conversation
❌ Deploy Preview for otel-ecosystem-explorer failed.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a header search affordance to the ecosystem-explorer frontend: a magnifier button in the navigation bar opens a modal overlay containing a debounced text input, filters a static list of page/section entries, and persists recent queries in localStorage.
Changes:
- New
src/lib/search.tswith a hard-codedsearchableContentarray and asearch()helper that filters/sorts by title-then-description matches. - New
src/hooks/useDebouncedValue.tsdebounce hook andsrc/components/ui/search-overlay.tsxmodal overlay (input, results list, recent-search history, clear actions). src/components/layout/header.tsxwraps its contents in a fragment, regroups nav into a flex container, and adds a search-icon button that toggles the overlay.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ecosystem-explorer/src/lib/search.ts | Static searchable-content list + filter/sort helper; several hard-coded paths do not match any registered route. |
| ecosystem-explorer/src/hooks/useDebouncedValue.ts | Generic debounce hook; uses camelCase filename and is missing the project license header. |
| ecosystem-explorer/src/components/ui/search-overlay.tsx | Modal search overlay; missing license header, no ARIA dialog semantics, no keyboard result navigation, fragile localStorage parsing, no tests. |
| ecosystem-explorer/src/components/layout/header.tsx | Adds the search trigger button and renders SearchOverlay conditionally; button is missing type="button". |
Comments suppressed due to low confidence (11)
ecosystem-explorer/src/lib/search.ts:61
- Several of the hard-coded
pathvalues do not match any route registered inLegacyApp.tsx/V1App.tsxand will navigate the user to the NotFoundPage:
/java-agent/instrumentationsshould be/java-agent/instrumentation/java-agent/configurationsshould be/java-agent/configuration/java-agent/release-comparisonshould be/java-agent/releases(and is only mounted when theJAVA_RELEASE_COMPARISONfeature flag is enabled, so it should probably not be exposed unconditionally either)/java-agent/config-buildershould be/java-agent/configuration/builder
Please align these paths with the actual <Route path=...> declarations.
{
title: 'Java Instrumentations',
description: 'Browse supported Java libraries and instrumentations',
path: '/java-agent/instrumentations',
type: 'section',
},
{
title: 'Java Configurations',
description: 'Configure OpenTelemetry Java Agent behavior',
path: '/java-agent/configurations',
type: 'section',
},
{
title: 'Java Release Comparison',
description: 'Compare features across Java Agent releases',
path: '/java-agent/release-comparison',
type: 'section',
},
{
title: 'Collector',
description: 'Explore OpenTelemetry Collector components',
path: '/collector',
type: 'page',
},
{
title: 'Configuration Builder',
description: 'Build custom OpenTelemetry configurations',
path: '/java-agent/config-builder',
type: 'section',
},
ecosystem-explorer/src/hooks/useDebouncedValue.ts:1
- Hook filenames in
ecosystem-explorer/src/hooks/use kebab-case (e.g.use-configuration-builder.ts,use-javaagent-data.ts,use-latest-java-agent-version.ts,data-state.ts). This new file is camelCase (useDebouncedValue.ts), which deviates from the established convention — please rename touse-debounced-value.tsand update the import insearch-overlay.tsx.
import { useState, useEffect } from "react";
ecosystem-explorer/src/components/ui/search-overlay.tsx:98
- The Escape key handler is attached only to the input's
onKeyDown. Once focus leaves the input (e.g. user tabs to a result, clicks "Clear recent searches", or clicks outside the panel without dismissing), pressing Escape no longer closes the overlay. Consider adding a window/document-levelkeydownlistener (or wrapping the panel with one) so Escape works regardless of focus, which is the standard behavior for a modal search overlay.
placeholder="Search instrumentations, collectors..."
value={query}
onChange={(e) => setQuery(e.target.value)}
onKeyDown={(e) => {
if (e.key === "Enter" && query.trim()) {
handleSelect(query);
} else if (e.key === "Escape") {
onClose();
}
}}
className="flex-1 bg-transparent outline-none text-foreground placeholder:text-muted-foreground text-sm"
aria-label="Search"
/>
ecosystem-explorer/src/components/ui/search-overlay.tsx:79
- This overlay is rendered as a modal dialog (full-screen backdrop, focus-trapping intent, Escape to close) but is not exposed as one to assistive tech. Consider adding
role="dialog"andaria-modal="true"(with anaria-labelsuch as "Search") on the panel, marking the results list with an appropriate role, and trapping focus inside the panel so keyboard/screen-reader users cannot tab into the page beneath. As-is, screen readers won't announce that a modal has opened and Tab will move focus into the obscured page content.
<>
{/* Backdrop */}
<div
className="fixed inset-0 z-40 bg-black/50 backdrop-blur-sm"
onClick={onClose}
aria-hidden="true"
/>
{/* Overlay panel */}
<div className="fixed inset-x-0 top-16 z-50 mx-auto max-w-2xl rounded-lg border border-border/20 bg-background shadow-lg">
ecosystem-explorer/src/components/ui/search-overlay.tsx:31
JSON.parseof localStorage content is not validated — any non-array value (e.g."5",{ "foo": 1 }, or an array containing non-strings) will be accepted and then crash later when the code calls.filter(...)on it or renderssearchQueryas a React key/child. Consider validating that the parsed value is an array of strings before callingsetRecentSearches, falling back to[]otherwise.
useEffect(() => {
const stored = localStorage.getItem(RECENT_SEARCHES_KEY);
if (stored) {
try {
setRecentSearches(JSON.parse(stored));
} catch {
setRecentSearches([]);
}
}
}, []);
ecosystem-explorer/src/components/ui/search-overlay.tsx:107
- This
<button>(and the Clear-recent button at line 159, the search-trigger button inheader.tsx, and the result buttons at lines 120/142) lacks an explicittype="button". Inside a<form>this defaults totype="submit"; while the overlay isn't currently in a form, it's the established pattern in this codebase (seesrc/components/ui/copy-button.tsx:77) to always specify it for safety. Please addtype="button"to the new buttons.
<button
onClick={() => setQuery("")}
className="text-muted-foreground hover:text-foreground"
aria-label="Clear search"
>
<X className="h-4 w-4" />
</button>
)}
ecosystem-explorer/src/components/ui/search-overlay.tsx:169
- There is a UX gap between typing and the debounced state: while the user is typing (
querynon-empty) butdebouncedQueryis still empty/stale,searchResultsmay be[]and the branch falls through toquery.trim() ? "No results for …". For the first 250 ms the user will see "No results for """ even though a search is in flight. Consider tracking a "pending" state (e.g.query !== debouncedQuery) and rendering a loading/idle placeholder instead of the "No results" message in that case.
{searchResults.length > 0 && !showRecent ? (
<>
<div className="px-4 py-2 text-xs font-semibold text-muted-foreground">
Results
</div>
<ul className="space-y-1 px-2 py-2">
{searchResults.map((result) => (
<li key={result.path}>
<button
onClick={() => handleSelect(result.title, result.path)}
className="w-full text-left rounded px-3 py-2 text-sm hover:bg-accent text-foreground transition-colors flex items-center justify-between"
>
<div>
<div className="font-medium">{result.title}</div>
<div className="text-xs text-muted-foreground">{result.description}</div>
</div>
<ChevronRight className="h-4 w-4 text-muted-foreground flex-shrink-0 ml-2" />
</button>
</li>
))}
</ul>
</>
) : recentSearches.length > 0 && showRecent ? (
<>
<div className="px-4 py-2 text-xs font-semibold text-muted-foreground">
Recent Searches
</div>
<ul className="space-y-1 px-2 py-2">
{recentSearches.map((searchQuery) => (
<li key={searchQuery}>
<button
onClick={() => {
const results = performSearch(searchQuery);
if (results.length > 0) {
handleSelect(searchQuery, results[0].path);
} else {
handleSelect(searchQuery);
}
}}
className="w-full text-left rounded px-3 py-2 text-sm hover:bg-accent text-foreground transition-colors"
>
<Search className="mr-2 inline h-4 w-4 text-muted-foreground" />
{searchQuery}
</button>
</li>
))}
</ul>
<button
onClick={handleClearRecent}
className="w-full px-4 py-2 text-xs text-muted-foreground hover:text-foreground border-t border-border/20"
>
Clear recent searches
</button>
</>
) : query.trim() ? (
<div className="px-4 py-8 text-center text-sm text-muted-foreground">
No results for "{debouncedQuery}"
</div>
ecosystem-explorer/src/components/ui/search-overlay.tsx:132
- There is no keyboard support for navigating results: ArrowUp/ArrowDown to move a highlighted item, Enter to select it. Currently Enter in the input always selects the raw query text via
handleSelect(query)(which only records a recent search and does not navigate anywhere even when a perfect match exists). For a header search overlay this is typically expected behavior; consider implementing arrow-key navigation with anaria-activedescendant/listbox pattern, and on Enter navigating to the highlighted (or first) result'spathinstead of just storing the raw text.
<input
ref={inputRef}
type="text"
placeholder="Search instrumentations, collectors..."
value={query}
onChange={(e) => setQuery(e.target.value)}
onKeyDown={(e) => {
if (e.key === "Enter" && query.trim()) {
handleSelect(query);
} else if (e.key === "Escape") {
onClose();
}
}}
className="flex-1 bg-transparent outline-none text-foreground placeholder:text-muted-foreground text-sm"
aria-label="Search"
/>
{query && (
<button
onClick={() => setQuery("")}
className="text-muted-foreground hover:text-foreground"
aria-label="Clear search"
>
<X className="h-4 w-4" />
</button>
)}
</div>
{/* Suggestions or recent searches */}
<div className="max-h-96 overflow-y-auto">
{searchResults.length > 0 && !showRecent ? (
<>
<div className="px-4 py-2 text-xs font-semibold text-muted-foreground">
Results
</div>
<ul className="space-y-1 px-2 py-2">
{searchResults.map((result) => (
<li key={result.path}>
<button
onClick={() => handleSelect(result.title, result.path)}
className="w-full text-left rounded px-3 py-2 text-sm hover:bg-accent text-foreground transition-colors flex items-center justify-between"
>
<div>
<div className="font-medium">{result.title}</div>
<div className="text-xs text-muted-foreground">{result.description}</div>
</div>
<ChevronRight className="h-4 w-4 text-muted-foreground flex-shrink-0 ml-2" />
</button>
</li>
))}
</ul>
ecosystem-explorer/src/lib/search.ts:68
- The "searchable content" is a hard-coded list of 7 page titles and one-line descriptions. The PR description ("Index documentation pages for keyword-based lookup", "search for CLI commands, documentation sections, keywords across the site") promises something substantially broader — searching across instrumentations, collector components, configuration fields, etc. Either the scope here should be reduced in the PR description, or the implementation should be extended (e.g. ingest the per-version instrumentation/collector indexes already loaded via
src/lib/api/*-data.tsand the configuration schema) before this lands. As written this search will not surface any of the actual content users are likely looking for.
const searchableContent: SearchResult[] = [
{
title: 'Java Agent',
description: 'Explore OpenTelemetry Java auto-instrumentation',
path: '/java-agent',
type: 'page',
},
{
title: 'Java Instrumentations',
description: 'Browse supported Java libraries and instrumentations',
path: '/java-agent/instrumentations',
type: 'section',
},
{
title: 'Java Configurations',
description: 'Configure OpenTelemetry Java Agent behavior',
path: '/java-agent/configurations',
type: 'section',
},
{
title: 'Java Release Comparison',
description: 'Compare features across Java Agent releases',
path: '/java-agent/release-comparison',
type: 'section',
},
{
title: 'Collector',
description: 'Explore OpenTelemetry Collector components',
path: '/collector',
type: 'page',
},
{
title: 'Configuration Builder',
description: 'Build custom OpenTelemetry configurations',
path: '/java-agent/config-builder',
type: 'section',
},
{
title: 'About',
description: 'Learn about OpenTelemetry Ecosystem Explorer',
path: '/about',
type: 'page',
},
];
ecosystem-explorer/src/components/ui/search-overlay.tsx:19
- No tests are added for
SearchOverlay,search(), oruseDebouncedValue, despite this being the codebase's convention (every other component insrc/components/ui/has a.test.tsxsibling — seecopy-button.test.tsx,info-tooltip.test.tsx,navigation-card.test.tsx,searchable-multi-select.test.tsx, and every hook insrc/hooks/has a.test.tssibling). Please add unit tests covering at least: empty/non-matching queries, title-vs-description matching/sort priority, recent-search persistence and clearing, Escape/clear button behavior, and navigation on selection.
export function SearchOverlay({ onClose, onSelect }: SearchOverlayProps) {
const navigate = useNavigate();
const [query, setQuery] = useState("");
const [recentSearches, setRecentSearches] = useState<string[]>([]);
const debouncedQuery = useDebouncedValue(query, 250);
const inputRef = useRef<HTMLInputElement>(null);
ecosystem-explorer/src/components/ui/search-overlay.tsx:7
- Other localStorage keys in this codebase follow the
otel-…-v<schema>kebab-case pattern (e.g.otel-config-builder-state-v3insrc/hooks/use-configuration-builder.ts:56). This key uses snake_case (otel_recent_searches) and has no schema version suffix — considerotel-recent-searches-v1for consistency and future migration safety.
const RECENT_SEARCHES_KEY = "otel_recent_searches";
Comment on lines
+21
to
+66
| type: 'page' | 'section'; | ||
| } | ||
|
|
||
| // Define searchable content in the app | ||
| const searchableContent: SearchResult[] = [ | ||
| { | ||
| title: 'Java Agent', | ||
| description: 'Explore OpenTelemetry Java auto-instrumentation', | ||
| path: '/java-agent', | ||
| type: 'page', | ||
| }, | ||
| { | ||
| title: 'Java Instrumentations', | ||
| description: 'Browse supported Java libraries and instrumentations', | ||
| path: '/java-agent/instrumentations', | ||
| type: 'section', | ||
| }, | ||
| { | ||
| title: 'Java Configurations', | ||
| description: 'Configure OpenTelemetry Java Agent behavior', | ||
| path: '/java-agent/configurations', | ||
| type: 'section', | ||
| }, | ||
| { | ||
| title: 'Java Release Comparison', | ||
| description: 'Compare features across Java Agent releases', | ||
| path: '/java-agent/release-comparison', | ||
| type: 'section', | ||
| }, | ||
| { | ||
| title: 'Collector', | ||
| description: 'Explore OpenTelemetry Collector components', | ||
| path: '/collector', | ||
| type: 'page', | ||
| }, | ||
| { | ||
| title: 'Configuration Builder', | ||
| description: 'Build custom OpenTelemetry configurations', | ||
| path: '/java-agent/config-builder', | ||
| type: 'section', | ||
| }, | ||
| { | ||
| title: 'About', | ||
| description: 'Learn about OpenTelemetry Ecosystem Explorer', | ||
| path: '/about', | ||
| type: 'page', |
Contributor
Author
There was a problem hiding this comment.
I did this because the file still had a few string literals written with single quotes, while the repo’s formatter/lint rules expect double quotes everywhere.
vitorvasc
reviewed
May 18, 2026
| } | ||
|
|
||
| // Define searchable content in the app | ||
| const searchableContent: SearchResult[] = [ |
Member
There was a problem hiding this comment.
I'm not sure if I like having this searchableContent, we'd need to update it anytime we add a new page, it doesn't seem to scale well.
| <input | ||
| ref={inputRef} | ||
| type="text" | ||
| placeholder="Search instrumentations, collectors..." |
Member
There was a problem hiding this comment.
Although the placeholder indicates that users can "search instrumentations", this isn't true for the current implementation. Looking at the search method, it currently only looks for pages declared in searchableContent.
A quick example from the preview:
The Java Agent page already has a search component that actually returns results for "kafka":
We could perhaps consider unifying the search engine to avoid duplicating code. WDYT?
Contributor
Author
There was a problem hiding this comment.
I’d rather unify this behind a shared search index/provider so we reuse the same source of truth across pages, while keeping page specific filters and UI local. That should make it scale better and avoid duplicating logic.
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
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.
Add Header Search Section for Improved User Experience
Description
This pull request proposes adding a search section in the header to improve navigation and usability across the documentation/website.
Currently, users need to manually browse through pages to find specific commands or documentation sections. Introducing a search option in the header would allow users to quickly locate relevant content.
Changes Proposed
Benefits
Possible Implementation
Testing
Related Issue
Closes #495
Additional Notes
This feature is intended to enhance usability and navigation.
Feedback from maintainers on the proposed implementation approach would be appreciated, and I’m happy to update the PR based on suggestions.