From 16bdffbac7ee6df86d8a66baf942769885787e6c Mon Sep 17 00:00:00 2001 From: Roland Date: Thu, 30 Jul 2026 11:28:17 -0600 Subject: [PATCH] ui: fix search suggestion selection, rendering, and clipping Three findings from review and CI on the Common.js migration, all real. Selecting a suggestion navigated nowhere. Semantic handed its callers `{result}` and every caller destructures that shape, so passing the bare result left `result.location` undefined -- the search suggestions and the Ctrl+K modal both stopped navigating. The shape stays until the call sites can change together. (Greptile P1.) Suggestions lost their custom markup. Callers pass `resultRenderer` to draw a tag as a chip rather than a line of text, and the migrated wrapper forwarded it into a div's props instead of to SearchBox, so every suggestion fell back to the generic title and description. SearchBox takes it now. (Greptile P2.) The suggestion list clipped its last group. A cypress spec caught it: with directories, channels and domains all present, the content runs past the 320px cap and the final row was cut off. Eight grouped suggestions is an ordinary result set, so the cap was simply too low; it is viewport-relative now and still bounded. Both regressions are pinned by tests -- one asserting the callback shape a caller actually destructures, one asserting the custom renderer replaces the default markup rather than wrapping it. --- app/src/components/Common.js | 7 +++++- app/src/components/Common.test.js | 39 ++++++++++++++++++++++++++++- app/src/components/ui/SearchBox.tsx | 17 ++++++++++--- app/src/components/ui/ui.css | 7 +++++- app/src/components/ui/ui.test.js | 14 +++++++++++ 5 files changed, 77 insertions(+), 7 deletions(-) diff --git a/app/src/components/Common.js b/app/src/components/Common.js index a83fb145..bd0c7be7 100644 --- a/app/src/components/Common.js +++ b/app/src/components/Common.js @@ -469,6 +469,7 @@ export function SearchResultsInput({ clearDisabled = null, results = undefined, handleResultSelect = null, + resultRenderer = undefined, loading = false, inputRef = null, ...props @@ -484,7 +485,10 @@ export function SearchResultsInput({ const localHandleResultSelect = (result) => { if (handleResultSelect) { - handleResultSelect(result); + // Semantic handed its callers `{result}`, and they destructure it -- passing the + // bare result silently broke navigation from the search suggestions. The shape + // stays until the call sites are ready to change together. + handleResultSelect({result}); } else { console.error('No handleResultSelect defined!'); } @@ -498,6 +502,7 @@ export function SearchResultsInput({ onResultSelect={localHandleResultSelect} onClear={handleClear} results={results} + resultRenderer={resultRenderer} loading={loading} placeholder={placeholder} clearable={clearable} diff --git a/app/src/components/Common.test.js b/app/src/components/Common.test.js index 316eebdf..a3c01a04 100644 --- a/app/src/components/Common.test.js +++ b/app/src/components/Common.test.js @@ -1,7 +1,7 @@ import React from 'react'; import {act, render, screen, waitFor} from '../test-utils'; import userEvent from '@testing-library/user-event'; -import {DirectorySearch} from './Common'; +import {DirectorySearch, SearchResultsInput} from './Common'; // Mock debounce to make tests run faster jest.mock('lodash/debounce', () => jest.fn(fn => { @@ -442,3 +442,40 @@ describe('DirectorySearch', () => { }); }); }); + +describe('SearchResultsInput', () => { + const results = { + tags: {name: 'Tags', results: [{type: 'tag', title: 'Cooking'}]}, + videos: {name: 'Videos', results: [ + {title: 'How To Sharpen An Axe', description: 'Wranglerstar', location: '/videos/1'}, + ]}, + }; + + it('hands the selected suggestion to its caller as {result}', async () => { + /* + * Regression: the migration passed the bare result, but every caller destructures + * `{result}` (see Search.js), so `result.location` was undefined and clicking a + * search suggestion navigated nowhere. + */ + const handleResultSelect = jest.fn(); + render(); + + await userEvent.click(screen.getByRole('combobox')); + await userEvent.click(screen.getByText('How To Sharpen An Axe')); + + expect(handleResultSelect).toHaveBeenCalledWith( + {result: expect.objectContaining({location: '/videos/1'})}); + }); + + it('forwards resultRenderer so a caller keeps its own suggestion markup', async () => { + // Search.js renders a tag suggestion as a tag chip, not a line of text. + const resultRenderer = (result) => rendered:{result.title}; + render(); + + await userEvent.click(screen.getByRole('combobox')); + + expect(screen.getByText('rendered:Cooking')).toBeInTheDocument(); + }); +}); diff --git a/app/src/components/ui/SearchBox.tsx b/app/src/components/ui/SearchBox.tsx index affd60f7..10bae23c 100644 --- a/app/src/components/ui/SearchBox.tsx +++ b/app/src/components/ui/SearchBox.tsx @@ -42,6 +42,12 @@ export interface SearchBoxProps { onResultSelect?: (result: SearchResult) => void; onBlur?: React.FocusEventHandler; results?: SearchResults; + /** + * Render one suggestion's contents. Callers use this to give a kind of result its + * own presentation — a tag chip rather than a line of text, say. Falls back to + * the title and description. + */ + resultRenderer?: (result: SearchResult) => React.ReactNode; /** Suggestions are being fetched; shows a spinner rather than "no results". */ loading?: boolean; placeholder?: string; @@ -71,6 +77,7 @@ export function SearchBox({ onResultSelect, onBlur, results, + resultRenderer, loading, placeholder = 'Search...', clearable, @@ -217,10 +224,12 @@ export function SearchBox({ }} onMouseEnter={() => setHighlighted(optionIndex)} > -
{result.title}
- {result.description &&
- {result.description} -
} + {resultRenderer ? resultRenderer(result) : <> +
{result.title}
+ {result.description &&
+ {result.description} +
} + } })} diff --git a/app/src/components/ui/ui.css b/app/src/components/ui/ui.css index a06a4644..2eb5cca7 100644 --- a/app/src/components/ui/ui.css +++ b/app/src/components/ui/ui.css @@ -275,7 +275,12 @@ html[data-theme="amber"] .wrolpi-searchbox-control:focus-within { top: 100%; left: 0; right: 0; - max-height: 320px; + /* + * Tall enough for a full set of grouped suggestions -- directories, channels and + * domains together run past 320px, and the last group was being clipped -- but + * still capped so a long list cannot cover the page. + */ + max-height: min(60vh, 480px); overflow-y: auto; background: var(--panel); border: 1px solid var(--border); diff --git a/app/src/components/ui/ui.test.js b/app/src/components/ui/ui.test.js index cda287c2..7b9fb6c0 100644 --- a/app/src/components/ui/ui.test.js +++ b/app/src/components/ui/ui.test.js @@ -573,6 +573,20 @@ describe('SearchBox', () => { expect.objectContaining({title: 'videos/RoseRed'})); }); + it('lets a caller render a suggestion its own way', async () => { + // The search suggestions render a tag as a chip rather than a line of text; a + // dropped resultRenderer loses that presentation silently. + renderUI( {`custom: ${result.title}`}}/>); + + await userEvent.click(screen.getByRole('combobox')); + + expect(screen.getByText('custom: videos/Wranglerstar').tagName).toBe('EM'); + // The default title/description markup is replaced, not wrapped. + expect(document.querySelector('.wrolpi-searchbox-result-title')).not.toBeInTheDocument(); + }); + it('closes on Escape without clearing what was typed', async () => { const onChange = jest.fn(); renderUI(