🧭 feat: Add a Fisheye Navigation Rail to Search Results#14353
🧭 feat: Add a Fisheye Navigation Rail to Search Results#14353danny-avila wants to merge 2 commits into
Conversation
Port the chat MessageNav rail into a data-driven, virtualization-native SearchNav for the virtualized search results view. - SearchNav.tsx: faithful port of the rail-local interaction layer (magnify/fisheye, shared hover tooltip, scrub-drag, prev/next chevrons, MessageIndicator ribs). The three chat-specific seams are replaced by props: entry source -> `entries`, DOM scroll -> `onJump(index, smooth)`, IntersectionObserver highlight -> `currentIndex` + `visibleIndices`. - Search.tsx: derive nav entries from `messages`, record the List's rendered range for current/visible state, and implement `onJump` via a measured-offset scrollToPosition animation that snaps with scrollToRow. - Tests for the pure helpers (ribDimsFor, magnifyFalloff) and a light render/interaction pass; extend Search.spec for the rail wiring.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: caff61fc4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!smooth || reducedMotionRef.current) { | ||
| list.scrollToRow(index); | ||
| return; |
There was a problem hiding this comment.
When a smooth rail jump is already running, an immediate jump from scrubbing calls this !smooth branch and returns without changing scrollTokenRef, so the previously queued requestAnimationFrame loop keeps calling scrollToPosition toward the old target for the rest of the 400 ms animation. This makes attempts to interrupt an animated click/chevron jump by dragging the search rail snap back toward the stale row until that old animation finishes; increment the token before returning from immediate jumps so they cancel any in-flight animation.
Useful? React with 👍 / 👎.
| list.push({ | ||
| id: 'search-nav-end', | ||
| index: list.length - 1, |
There was a problem hiding this comment.
When the final search result is taller than the viewport, the trailing search-nav-end entry is indistinguishable from the last result because it reuses the last row index. SearchNav can only pass that index to onJump, so clicking the “scroll to bottom” rib or pressing the down chevron from the last result targets the top of the last row again instead of the real bottom (and from the actual bottom it can even scroll upward); use a distinct bottom action/target for the end marker.
Useful? React with 👍 / 👎.
| return; | ||
| } | ||
| if (!smooth || reducedMotionRef.current) { | ||
| list.scrollToRow(index); |
There was a problem hiding this comment.
When prefers-reduced-motion is enabled, chevrons and indicator clicks take this scrollToRow path, but the List below does not set scrollToAlignment, so react-virtualized uses its default auto alignment and does nothing if the target row is already visible. In that setting, pressing Next from the top row stalls on the second row whenever it is already in the viewport because currentIndex never advances; use the same computed row offset with scrollToPosition or force start alignment for immediate jumps.
Useful? React with 👍 / 👎.
1 participant
Summary
Final piece of the search-improvement series (stacked on #14352, which is stacked on #14350). Gives the message-search results view its own navigation rail — the same fisheye/scrub rail the main chat has — now that search can return a large linear result set.
MessageNavis ~1,270 lines and almost entirely DOM-driven (it discovers entries by scanning.message-rendernodes, tracks position with an IntersectionObserver, and jumps by animatingscrollTopon real elements). A virtualized list unmounts off-screen rows, so that machinery had to be rebuilt as data-driven. The insight that made this tractable: the rich interaction layer is rail-local — magnify/fisheye, scrub-drag, the shared preview tooltip, and the chevrons all operate on the rail's own DOM, so they port verbatim. Only three seams are chat-specific:client/src/components/Chat/Messages/SearchNav.tsx— the ported rail.MessageIndicator,measureRibs/applyMagnify/ribDimsFor/magnifyFalloff, the tooltip stack, the scrub-drag (with thesuppressClickRefguard), reduced-motion handling, and prev/next chevrons are carried over faithfully; entry lookups switch from id→index. Driven by props:entries[],currentIndex,visibleIndices,onJump(index, smooth). Hides when fewer than 3 results.ribDimsFor/magnifyFalloffare exported for unit tests.client/src/routes/Search.tsxwires the three seams:entriesbuilt from the results (with a trailing scroll-to-bottom marker),currentIndex/visibleIndicesderived from theList's rendered range (equality-gated to avoid re-render storms), andonJump— which sums theCellMeasurerCacheheights to find a row's offset, rAF-animatesscrollToPosition(ease-out cubic, reduced-motion aware), then snaps withscrollToRowso the landing is exact even for a row that re-measures on mount.Notable choices: the trailing end-marker shares the last row's
indexbut is excluded fromaria-current, the< 3hide threshold, and the chevron targets (all keyed on array position, not index). Dropped the chat'sShift+Alt+Mglobal shortcut (message-specific). One new locale keycom_ui_search_navfor the landmark label; chevron/go-to labels reuse the existingcom_ui_message_nav_*keys.Change Type
Testing
npx jeston the search suites: 23/23 —SearchNav.spec.tsx(helpersribDimsFor/magnifyFalloff; renders one indicator per entry, hides below 3,aria-currenton the current row, click →onJump(index, true); jsdom-safe with 0-rect layout), plus the extendedSearch.spec.tsxand theSearchMessagecomparator.npx tsc --noEmitandnpx eslint: clean.scrollToPosition→scrollToRowhandoff are best confirmed live on a large result set (together with the two parent PRs); the logic and a11y are unit-covered here.Checklist