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(