mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04:00
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.
This commit is contained in:
parent
851485ef1b
commit
16bdffbac7
5 changed files with 77 additions and 7 deletions
|
|
@ -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}
|
||||
|
|
|
|||
|
|
@ -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(<SearchResultsInput searchStr='axe' onSubmit={jest.fn()} results={results}
|
||||
handleResultSelect={handleResultSelect}/>);
|
||||
|
||||
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) => <span>rendered:{result.title}</span>;
|
||||
render(<SearchResultsInput searchStr='cook' onSubmit={jest.fn()} results={results}
|
||||
handleResultSelect={jest.fn()} resultRenderer={resultRenderer}/>);
|
||||
|
||||
await userEvent.click(screen.getByRole('combobox'));
|
||||
|
||||
expect(screen.getByText('rendered:Cooking')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -42,6 +42,12 @@ export interface SearchBoxProps {
|
|||
onResultSelect?: (result: SearchResult) => void;
|
||||
onBlur?: React.FocusEventHandler<HTMLInputElement>;
|
||||
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)}
|
||||
>
|
||||
<div className='wrolpi-searchbox-result-title'>{result.title}</div>
|
||||
{result.description && <div className='wrolpi-searchbox-result-description'>
|
||||
{result.description}
|
||||
</div>}
|
||||
{resultRenderer ? resultRenderer(result) : <>
|
||||
<div className='wrolpi-searchbox-result-title'>{result.title}</div>
|
||||
{result.description && <div className='wrolpi-searchbox-result-description'>
|
||||
{result.description}
|
||||
</div>}
|
||||
</>}
|
||||
</div>
|
||||
})}
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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(<SearchBox value='vid' onChange={jest.fn()} results={results}
|
||||
onResultSelect={jest.fn()}
|
||||
resultRenderer={result => <em>{`custom: ${result.title}`}</em>}/>);
|
||||
|
||||
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(<SearchBox value='vid' onChange={onChange} results={results}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue