Skip to content

Commit 7599a53

Browse files
committed
fix(tables): block find navigation until the results describe the term
1 parent d131feb commit 7599a53

3 files changed

Lines changed: 57 additions & 5 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-find.test.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ function render(overrides: Partial<TableFindProps> = {}) {
6262
onSubmit: vi.fn(),
6363
onClose: vi.fn(),
6464
isStale: false,
65+
canNavigate: true,
6566
count: 0,
6667
currentIndex: 0,
6768
truncated: false,
@@ -152,6 +153,25 @@ describe('TableFind keyboard', () => {
152153
expect(props.onNext).not.toHaveBeenCalled()
153154
})
154155

156+
// Committing makes the typed and submitted terms agree instantly, but the
157+
// matches on screen still belong to the previous term until the request
158+
// lands — stepping there would select a cell the box no longer names.
159+
it('does not step while the committed term is still loading', () => {
160+
const props = render({ query: 'abcd', count: 3, isStale: false, canNavigate: false })
161+
press('Enter')
162+
expect(props.onNext).not.toHaveBeenCalled()
163+
expect(props.onSubmit).not.toHaveBeenCalled()
164+
165+
press('Enter', { shiftKey: true })
166+
expect(props.onPrev).not.toHaveBeenCalled()
167+
})
168+
169+
it('disables the arrows until the results describe the term', () => {
170+
render({ query: 'abcd', count: 3, canNavigate: false })
171+
expect(buttonByLabel('Next match').disabled).toBe(true)
172+
expect(buttonByLabel('Previous match').disabled).toBe(true)
173+
})
174+
155175
it('closes on Escape', () => {
156176
const props = render({ query: 'a', count: 3 })
157177
press('Escape')

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-find.tsx

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,15 @@ export interface TableFindProps {
1313
/** Adopts the typed term immediately, skipping the debounce. */
1414
onSubmit: () => void
1515
onClose: () => void
16-
/** Whether the results on screen still describe an older term. */
16+
/** Whether the typed term has yet to be searched, so Enter should commit it. */
1717
isStale: boolean
18+
/**
19+
* Whether the matches on screen belong to the term that was searched. False
20+
* while a term's own results are in flight, when the count still describes
21+
* the previous term and stepping through it would land on a cell the box no
22+
* longer names.
23+
*/
24+
canNavigate: boolean
1825
/** Number of matches after dropping columns not in the current view. */
1926
count: number
2027
/** 0-based index of the active match. Ignored when `count` is 0. */
@@ -38,6 +45,7 @@ export const TableFind = memo(function TableFind({
3845
onSubmit,
3946
onClose,
4047
isStale,
48+
canNavigate,
4149
count,
4250
currentIndex,
4351
truncated,
@@ -47,9 +55,12 @@ export const TableFind = memo(function TableFind({
4755
const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
4856
if (e.key === 'Enter') {
4957
e.preventDefault()
50-
// Committing beats stepping while the matches on screen belong to an
51-
// older term — stepping there navigates results the box no longer shows.
58+
// Commit an unsearched term; otherwise step — but only once the results
59+
// describe it. In between (committed, still loading) Enter does nothing
60+
// rather than walk the previous term's matches; the auto-reveal lands on
61+
// the first hit as soon as they arrive.
5262
if (isStale) onSubmit()
63+
else if (!canNavigate) return
5364
else if (e.shiftKey) onPrev()
5465
else onNext()
5566
return
@@ -62,6 +73,7 @@ export const TableFind = memo(function TableFind({
6273

6374
const hasQuery = query.trim().length > 0
6475
const hasMatches = count > 0
76+
const navEnabled = hasMatches && canNavigate
6577

6678
/** The tally holds its last value while the next result set loads — blanking
6779
* it on every keystroke reads as the feature breaking rather than working. */
@@ -120,7 +132,7 @@ export const TableFind = memo(function TableFind({
120132
className='size-6 shrink-0'
121133
aria-label='Previous match'
122134
title='Previous match (Shift+Enter)'
123-
disabled={!hasMatches}
135+
disabled={!navEnabled}
124136
onClick={onPrev}
125137
>
126138
<ChevronUp className='size-[13px]' />
@@ -132,7 +144,7 @@ export const TableFind = memo(function TableFind({
132144
className='size-6 shrink-0'
133145
aria-label='Next match'
134146
title='Next match (Enter)'
135-
disabled={!hasMatches}
147+
disabled={!navEnabled}
136148
onClick={onNext}
137149
>
138150
<ChevronDown className='size-[13px]' />

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1197,8 +1197,25 @@ export function TableGrid({
11971197
return byRow
11981198
}, [findMatches])
11991199

1200+
/**
1201+
* Whether the matches on screen actually belong to the submitted term.
1202+
*
1203+
* False while a term's own results are still in flight — `keepPreviousData`
1204+
* keeps serving the PREVIOUS term's matches until they land, and the first
1205+
* search of a session has no data at all. Navigation is gated on this:
1206+
* committing with Enter makes the typed and submitted terms agree instantly,
1207+
* so without it a second Enter would step through the old term's matches.
1208+
*
1209+
* A background refetch of the SAME term keeps this true — its data is still
1210+
* for this key — so an SSE row update doesn't disable the arrows mid-search.
1211+
*/
1212+
const findResultsAreCurrent =
1213+
submittedQuery.length > 0 && findData !== undefined && !isFindPlaceholder
1214+
12001215
const findMatchesRef = useRef(findMatches)
12011216
findMatchesRef.current = findMatches
1217+
const findResultsAreCurrentRef = useRef(findResultsAreCurrent)
1218+
findResultsAreCurrentRef.current = findResultsAreCurrent
12021219
const currentMatchIndexRef = useRef(currentMatchIndex)
12031220
currentMatchIndexRef.current = currentMatchIndex
12041221
const findOpenRef = useRef(findOpen)
@@ -1324,11 +1341,13 @@ export function TableGrid({
13241341
* would only come back around after wrapping the whole list.
13251342
*/
13261343
const handleFindNext = useCallback(() => {
1344+
if (!findResultsAreCurrentRef.current) return
13271345
const index = currentMatchIndexRef.current
13281346
goToMatch(cursorIsOnMatchRef.current ? index + 1 : index)
13291347
}, [goToMatch])
13301348

13311349
const handleFindPrev = useCallback(() => {
1350+
if (!findResultsAreCurrentRef.current) return
13321351
const index = currentMatchIndexRef.current
13331352
goToMatch(cursorIsOnMatchRef.current ? index - 1 : index)
13341353
}, [goToMatch])
@@ -4401,6 +4420,7 @@ export function TableGrid({
44014420
onSubmit={handleFindSubmit}
44024421
onClose={handleFindClose}
44034422
isStale={trimmedFindQuery !== submittedQuery}
4423+
canNavigate={findResultsAreCurrent}
44044424
count={findMatches.length}
44054425
// Clamped, not stored: a background refetch of the same term can
44064426
// shrink the match set under a cursor the user already paged, and

0 commit comments

Comments
 (0)