diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/constants.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/constants.ts index f369e93d363..af00af4b441 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/constants.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/constants.ts @@ -1,6 +1,15 @@ /** Tailwind class applied to selected rows / columns / cells. */ export const SELECTION_TINT_BG = 'bg-[rgba(37,99,235,0.06)]' +/** + * Fill marking every cell matching the active find query. Reuses the app's + * search-highlight token (the knowledge-base search highlight paints with the + * same one) rather than inventing a third match colour, so the two stay + * theme-tuned together. The ACTIVE match is told apart by the selection + * outline drawn over it, not by a different fill. + */ +export const FIND_MATCH_TINT_BG = 'bg-[var(--highlight-match-bg)]' + /** Default column width in pixels. Used as a fallback when a column hasn't * been measured yet and as the initial width for newly-added columns. */ export const COL_WIDTH = 160 @@ -23,5 +32,7 @@ export const CELL_HEADER_CHECKBOX = /** Fixed height (not min-) so a Badge-rendered status pill doesn't make the row grow vs a plain-text neighbor. */ export const CELL_CONTENT = 'relative flex h-[22px] min-w-0 items-center overflow-clip text-ellipsis whitespace-nowrap text-small' -export const SELECTION_OVERLAY = - 'pointer-events-none absolute -top-px -right-px -bottom-px z-[5] border-[2px] border-[var(--selection)]' +/** Inset shared by every full-cell overlay, so the tints and the selection + * outline can't drift apart on a border-geometry change. */ +export const CELL_OVERLAY_INSET = 'pointer-events-none absolute -top-px -right-px -bottom-px' +export const SELECTION_OVERLAY = `${CELL_OVERLAY_INSET} z-[5] border-[2px] border-[var(--selection)]` diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx index acf192e3002..077f73fb2e5 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/data-row.tsx @@ -12,6 +12,8 @@ import { CELL, CELL_CHECKBOX, CELL_CONTENT, + CELL_OVERLAY_INSET, + FIND_MATCH_TINT_BG, SELECTION_OVERLAY, SELECTION_TINT_BG, } from './constants' @@ -67,6 +69,13 @@ export interface DataRowProps { pinnedOffsets?: Map /** Key of the rightmost pinned column, used to render a separator shadow. */ lastPinnedColKey?: string | null + /** + * Column keys in this row matching the active find query, tinted so every hit + * is visible at once rather than only the one being navigated to. Absent when + * the row has no match, which is the common case and keeps this row's memo + * from re-running for a search elsewhere in the table. + */ + findMatchColumns?: ReadonlySet } function cellRangeRowChanged( @@ -128,7 +137,8 @@ function dataRowPropsAreEqual(prev: DataRowProps, next: DataRowProps): boolean { prev.workflowGroups !== next.workflowGroups || prev.activeDispatches !== next.activeDispatches || prev.pinnedOffsets !== next.pinnedOffsets || - prev.lastPinnedColKey !== next.lastPinnedColKey + prev.lastPinnedColKey !== next.lastPinnedColKey || + prev.findMatchColumns !== next.findMatchColumns ) { return false } @@ -177,6 +187,7 @@ export const DataRow = React.memo(function DataRow({ activeDispatches, pinnedOffsets, lastPinnedColKey, + findMatchColumns, }: DataRowProps) { const sel = normalizedSelection /** @@ -299,6 +310,7 @@ export const DataRow = React.memo(function DataRow({ const isAnchor = sel !== null && rowIndex === sel.anchorRow && colIndex === sel.anchorCol const isEditing = editingColumnName === column.key const isHighlighted = inRange || isRowChecked + const isFindMatch = findMatchColumns?.has(column.key) const isTopEdge = inRange ? rowIndex === sel!.startRow : isRowChecked const isBottomEdge = inRange ? rowIndex === sel!.endRow : isRowChecked @@ -323,7 +335,7 @@ export const DataRow = React.memo(function DataRow({ data-pinned={isPinnedCell ? '' : undefined} className={cn( CELL, - (isHighlighted || isAnchor || isEditing) && 'relative', + (isHighlighted || isAnchor || isEditing || isFindMatch) && 'relative', isPinnedCell && 'z-[6] bg-[var(--bg)]', isPinnedSeparator && '[box-shadow:2px_0_0_0_var(--border)]' )} @@ -342,10 +354,26 @@ export const DataRow = React.memo(function DataRow({ } onDoubleClick={() => onDoubleClick(row.id, column.key, column.key)} > + {/* No z-index on purpose: with `auto` it paints in DOM order, so it + sits above the cell background but BELOW the cell text, the + selection tint (z-4) and the anchor outline (z-5). The active + match therefore still reads as the selected cell, and the wash + never dims the value it is pointing at. */} + {isFindMatch && ( +
+ )} {isHighlighted && (isMultiCell || isRowChecked) && (
({ + Button: ({ children, ...props }: { children: ReactNode } & Record) => ( + + ), + ChipInput: ({ + endAdornment, + icon: _icon, + ...props + }: { endAdornment?: ReactNode } & Record) => ( + <> + + {endAdornment} + + ), +})) + +vi.mock('@sim/emcn/icons', () => ({ + ChevronDown: () => , + ChevronUp: () => , + Loader: () => , + Search: () => , + X: () => , +})) + +import { TableFind, type TableFindProps } from './table-find' + +let container: HTMLDivElement +let root: Root + +beforeEach(() => { + container = document.createElement('div') + document.body.appendChild(container) + act(() => { + root = createRoot(container) + }) +}) + +afterEach(() => { + act(() => root.unmount()) + container.remove() +}) + +function render(overrides: Partial = {}) { + const props: TableFindProps = { + query: '', + onQueryChange: vi.fn(), + onNext: vi.fn(), + onPrev: vi.fn(), + onSubmit: vi.fn(), + onClose: vi.fn(), + isStale: false, + canNavigate: true, + count: 0, + currentIndex: 0, + truncated: false, + isLoading: false, + inputRef: createRef(), + ...overrides, + } + act(() => root.render()) + return props +} + +function input(): HTMLInputElement { + const el = container.querySelector('input') + if (!el) throw new Error('find input not rendered') + return el +} + +function counterText(): string | null { + return container.querySelector('[aria-live="polite"]')?.textContent ?? null +} + +function buttonByLabel(label: string): HTMLButtonElement { + const el = container.querySelector(`button[aria-label="${label}"]`) + if (!el) throw new Error(`no button labelled ${label}`) + return el as HTMLButtonElement +} + +function press(key: string, init: KeyboardEventInit = {}) { + act(() => { + input().dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, ...init })) + }) +} + +describe('TableFind counter', () => { + it('shows nothing before the user has typed', () => { + render({ query: '' }) + expect(counterText()).toBe('') + }) + + it('counts matches as 1-based', () => { + render({ query: 'a', count: 12, currentIndex: 0 }) + expect(counterText()).toBe('1 of 12') + render({ query: 'a', count: 12, currentIndex: 11 }) + expect(counterText()).toBe('12 of 12') + }) + + it('marks a server-capped result set', () => { + render({ query: 'a', count: 1000, currentIndex: 0, truncated: true }) + expect(counterText()).toBe('1 of 1000+') + }) + + it('says No results only once the search has settled', () => { + render({ query: 'zzz', count: 0, isLoading: true }) + expect(counterText()).toBe('') + expect(container.querySelector('[data-icon="loader"]')).not.toBeNull() + + render({ query: 'zzz', count: 0, isLoading: false }) + expect(counterText()).toBe('No results') + }) + + // Blanking the tally on each keystroke reads as the search breaking; the + // previous term's count holds until the new one lands. + it('keeps the previous count visible while the next result set loads', () => { + render({ query: 'ab', count: 3, currentIndex: 1, isLoading: true }) + expect(counterText()).toBe('2 of 3') + }) + + it('keeps the counter mounted and width-reserved before the user types', () => { + render({ query: '' }) + const region = container.querySelector('[aria-live="polite"]') + expect(region).not.toBeNull() + expect(region?.className).toContain('min-w-[64px]') + }) +}) + +describe('TableFind keyboard', () => { + it('navigates on Enter rather than submitting a search', () => { + const props = render({ query: 'a', count: 3 }) + press('Enter') + expect(props.onNext).toHaveBeenCalledTimes(1) + expect(props.onPrev).not.toHaveBeenCalled() + }) + + it('steps backwards on Shift+Enter', () => { + const props = render({ query: 'a', count: 3 }) + press('Enter', { shiftKey: true }) + expect(props.onPrev).toHaveBeenCalledTimes(1) + expect(props.onNext).not.toHaveBeenCalled() + }) + + // Committing makes the typed and submitted terms agree instantly, but the + // matches on screen still belong to the previous term until the request + // lands — stepping there would select a cell the box no longer names. + it('does not step while the committed term is still loading', () => { + const props = render({ query: 'abcd', count: 3, isStale: false, canNavigate: false }) + press('Enter') + expect(props.onNext).not.toHaveBeenCalled() + expect(props.onSubmit).not.toHaveBeenCalled() + + press('Enter', { shiftKey: true }) + expect(props.onPrev).not.toHaveBeenCalled() + }) + + it('disables the arrows until the results describe the term', () => { + render({ query: 'abcd', count: 3, canNavigate: false }) + expect(buttonByLabel('Next match').disabled).toBe(true) + expect(buttonByLabel('Previous match').disabled).toBe(true) + }) + + it('closes on Escape', () => { + const props = render({ query: 'a', count: 3 }) + press('Escape') + expect(props.onClose).toHaveBeenCalledTimes(1) + }) + + // Mid-debounce the visible matches still belong to the previous term, so + // stepping through them would land on a cell the box no longer describes. + it('commits instead of stepping while the results are stale', () => { + const props = render({ query: 'abcd', count: 3, isStale: true }) + press('Enter') + expect(props.onSubmit).toHaveBeenCalledTimes(1) + expect(props.onNext).not.toHaveBeenCalled() + + press('Enter', { shiftKey: true }) + expect(props.onSubmit).toHaveBeenCalledTimes(2) + expect(props.onPrev).not.toHaveBeenCalled() + }) +}) + +describe('TableFind controls', () => { + it('offers a clear button only once there is text', () => { + render({ query: '' }) + expect(container.querySelector('button[aria-label="Clear search"]')).toBeNull() + + const props = render({ query: 'abc' }) + act(() => buttonByLabel('Clear search').click()) + expect(props.onQueryChange).toHaveBeenCalledWith('') + }) + + it('disables navigation while there is nothing to navigate', () => { + render({ query: 'zzz', count: 0 }) + expect(buttonByLabel('Next match').disabled).toBe(true) + expect(buttonByLabel('Previous match').disabled).toBe(true) + + render({ query: 'a', count: 2 }) + expect(buttonByLabel('Next match').disabled).toBe(false) + expect(buttonByLabel('Previous match').disabled).toBe(false) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-find.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-find.tsx index 58b9220bdcb..a48a7e0122e 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-find.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-find.tsx @@ -1,53 +1,68 @@ 'use client' import type React from 'react' +import { memo } from 'react' import { Button, ChipInput } from '@sim/emcn' -import { ChevronDown, ChevronUp, Loader, X } from '@sim/emcn/icons' +import { ChevronDown, ChevronUp, Loader, Search, X } from '@sim/emcn/icons' export interface TableFindProps { query: string onQueryChange: (query: string) => void - /** Run the search (dirty Enter / search button). */ - onSubmit: () => void onNext: () => void onPrev: () => void + /** Adopts the typed term immediately, skipping the debounce. */ + onSubmit: () => void onClose: () => void + /** Whether the typed term has yet to be searched, so Enter should commit it. */ + isStale: boolean + /** + * Whether the matches on screen belong to the term that was searched. False + * while a term's own results are in flight, when the count still describes + * the previous term and stepping through it would land on a cell the box no + * longer names. + */ + canNavigate: boolean /** Number of matches after dropping columns not in the current view. */ count: number - /** 0-based index of the active match, or -1 when there are none. */ + /** 0-based index of the active match. Ignored when `count` is 0. */ currentIndex: number /** Whether the server capped the match set. */ truncated: boolean isLoading: boolean - /** Whether the input differs from the last submitted term. */ - isDirty: boolean inputRef: React.RefObject } -export function TableFind({ +/** + * Memoized: while the bar is open it is a child of the grid, which re-renders + * on scroll, hover and selection. Every prop is a primitive or a stable + * identity, so this collapses to renders where a find value actually changed. + */ +export const TableFind = memo(function TableFind({ query, onQueryChange, - onSubmit, onNext, onPrev, + onSubmit, onClose, + isStale, + canNavigate, count, currentIndex, truncated, isLoading, - isDirty, inputRef, }: TableFindProps) { const handleKeyDown = (e: React.KeyboardEvent) => { if (e.key === 'Enter') { e.preventDefault() - if (e.shiftKey) { - onPrev() - } else if (isDirty) { - onSubmit() - } else { - onNext() - } + // Commit an unsearched term; otherwise step — but only once the results + // describe it. In between (committed, still loading) Enter does nothing + // rather than walk the previous term's matches; the auto-reveal lands on + // the first hit as soon as they arrive. + if (isStale) onSubmit() + else if (!canNavigate) return + else if (e.shiftKey) onPrev() + else onNext() return } if (e.key === 'Escape') { @@ -56,9 +71,17 @@ export function TableFind({ } } + const hasQuery = query.trim().length > 0 const hasMatches = count > 0 - const label = - count === 0 ? 'No results' : `${currentIndex + 1} of ${count}${truncated ? '+' : ''}` + const navEnabled = hasMatches && canNavigate + + /** The tally holds its last value while the next result set loads — blanking + * it on every keystroke reads as the feature breaking rather than working. */ + function counterContent() { + if (!hasQuery) return null + if (hasMatches) return `${currentIndex + 1} of ${count}${truncated ? '+' : ''}` + return isLoading ? : 'No results' + } return (
@@ -66,42 +89,77 @@ export function TableFind({ ref={inputRef} value={query} placeholder='Search' + aria-label='Find in table' + spellCheck={false} + autoComplete='off' + icon={Search} className='w-[200px]' onChange={(e) => onQueryChange(e.target.value)} onKeyDown={handleKeyDown} + // Untrimmed on purpose: whitespace searches nothing, but it is still + // text the user may want cleared. + endAdornment={ + query.length > 0 ? ( + + ) : undefined + } /> - - {isLoading ? : label} + {/* Always mounted, reserving its width: rendering it only once there is a + query would resize the bar on the first keystroke, and a live region + inserted together with its text is announced unreliably. */} + + {counterContent()}
) -} +}) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx index 59222045098..bc0c289ac72 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx @@ -27,6 +27,7 @@ import { getColumnId } from '@/lib/table/column-keys' import { columnTypeOf } from '@/lib/table/column-types' import { TABLE_LIMITS } from '@/lib/table/constants' import { cellValueFilterConditions } from '@/lib/table/query-builder/cell-filter' +import { SEARCH_DEBOUNCE_MS } from '@/lib/url-state' import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' import type { RemoteTableSelection } from '@/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-room' import type { BlockedTableAction } from '@/app/workspace/[workspaceId]/tables/[tableId]/lock-copy' @@ -94,6 +95,7 @@ const logger = createLogger('TableView') const EMPTY_RUNNING_BY_ROW: Readonly> = Object.freeze({}) const EMPTY_FIND_MATCHES: readonly TableFindMatch[] = Object.freeze([]) +const EMPTY_FIND_MATCH_COLUMNS: ReadonlyMap> = Object.freeze(new Map()) const EMPTY_FILTER_CONDITIONS: readonly Predicate[] = Object.freeze([]) const COL_WIDTH_MIN = 80 @@ -481,11 +483,9 @@ export function TableGrid({ const [selectionFocus, setSelectionFocus] = useState(null) const [rowSelection, setRowSelection] = useState(ROW_SELECTION_NONE) const [isColumnSelection, setIsColumnSelection] = useState(false) - // Find (Cmd/Ctrl+F): `findQuery` is the live input, `submittedQuery` is the - // last Enter/search-triggered term the query hook runs on. + // Find (Cmd/Ctrl+F): `findQuery` is the live input. const [findOpen, setFindOpen] = useState(false) const [findQuery, setFindQuery] = useState('') - const [submittedQuery, setSubmittedQuery] = useState('') const [currentMatchIndex, setCurrentMatchIndex] = useState(0) const [isJumping, setIsJumping] = useState(false) // Bumped on every navigation so the reveal effect re-runs even when the target @@ -493,6 +493,18 @@ export function TableGrid({ const [pendingMatchTick, setPendingMatchTick] = useState(0) const findInputRef = useRef(null) const pendingMatchRef = useRef(null) + /** Monotonic id for the in-flight match jump; see `goToMatch`. */ + const goToMatchSeqRef = useRef(0) + /** The match the cursor is on, by identity rather than position, so a + * reordered result set can re-point at the same cell. */ + const activeMatchRef = useRef(null) + /** Term the auto-reveal has already run for, so a background refetch of the + * same term doesn't re-jump the viewport. */ + const autoRevealedTermRef = useRef('') + /** Whether the selection currently sits on the match at `currentMatchIndex`. + * False when the auto-reveal was skipped, so next/prev knows to land on that + * index rather than step past it. */ + const cursorIsOnMatchRef = useRef(false) const lastCheckboxRowRef = useRef(null) const isColumnSelectionRef = useRef(false) const [columnWidths, setColumnWidths] = useState>({}) @@ -1093,7 +1105,51 @@ export function TableGrid({ emitCellSelection({ anchor, focus, editing: editingCell !== null }) }, [selectionAnchor, selectionFocus, editingCell, rows, displayColumns, emitCellSelection]) - const { data: findData, isFetching: isFindFetching } = useFindTableRows({ + /** + * The term the search actually runs on: the live input, debounced so results + * follow typing without a request per keystroke. + * + * Owned here rather than via `useDebounce` because closing or clearing has to + * take effect IMMEDIATELY and cancel anything pending. `useDebounce` is + * trailing-edge and keeps serving its last value until the next timer fires, + * so after Esc it still holds the old term — and a guard on the *input* can't + * mask that, because the first keystroke of the next search makes the input + * non-empty again while the debounce is still holding the previous term. The + * result would be the old search replayed from cache (highlights, count and a + * viewport jump) under a box showing one fresh character. Cmd+F, Esc, Cmd+F + * is an ordinary correction, so that window gets hit. + */ + const trimmedFindQuery = findQuery.trim() + const [submittedQuery, setSubmittedQuery] = useState('') + useEffect(() => { + if (!findOpen || trimmedFindQuery.length === 0) { + setSubmittedQuery('') + return + } + const timer = setTimeout(() => setSubmittedQuery(trimmedFindQuery), SEARCH_DEBOUNCE_MS) + return () => clearTimeout(timer) + }, [findOpen, trimmedFindQuery]) + + const trimmedFindQueryRef = useRef(trimmedFindQuery) + trimmedFindQueryRef.current = trimmedFindQuery + + /** + * Adopt the typed term now instead of waiting out the debounce. Enter uses + * this while the two disagree: navigating there would step through the + * PREVIOUS term's matches — `keepPreviousData` still holds them — and land on + * a cell that doesn't match the box. Pressing Enter means "search this now", + * so it commits rather than navigates, and the auto-reveal takes it from + * there. The pending timer is harmless: it later sets the same string. + */ + const handleFindSubmit = useCallback(() => { + setSubmittedQuery(trimmedFindQueryRef.current) + }, []) + + const { + data: findData, + isFetching: isFindFetching, + isPlaceholderData: isFindPlaceholder, + } = useFindTableRows({ workspaceId, tableId, q: submittedQuery, @@ -1108,6 +1164,11 @@ export function TableGrid({ * to a cell that isn't rendered. */ const findMatches = useMemo(() => { + // `keepPreviousData` serves the previous term's matches while a new term + // loads, which is what keeps the counter steady mid-typing — but with an + // empty term the query is disabled, so that placeholder would otherwise + // linger as highlights over a cleared search box. + if (submittedQuery.length === 0) return EMPTY_FIND_MATCHES const raw = findData?.matches if (!raw || raw.length === 0) return EMPTY_FIND_MATCHES // `m.column` is the stable column id (the JSONB storage key); index display @@ -1120,10 +1181,44 @@ export function TableGrid({ a.ordinal - b.ordinal || (colIndexByKey.get(a.column) ?? 0) - (colIndexByKey.get(b.column) ?? 0) ) - }, [findData, displayColumns]) + }, [findData, displayColumns, submittedQuery]) + + /** + * Match column ids grouped by row id, so a row can mark its matching cells in + * O(1) without scanning the whole match list. Rebuilt only when the match set + * changes; `DataRow` is memoized on the per-row `Set`, so rows without a match + * keep the same `undefined` and never re-render for a search. + */ + const findMatchColumnsByRowId = useMemo>>(() => { + if (findMatches.length === 0) return EMPTY_FIND_MATCH_COLUMNS + const byRow = new Map>() + for (const match of findMatches) { + const existing = byRow.get(match.rowId) + if (existing) existing.add(match.column) + else byRow.set(match.rowId, new Set([match.column])) + } + return byRow + }, [findMatches]) + + /** + * Whether the matches on screen actually belong to the submitted term. + * + * False while a term's own results are still in flight — `keepPreviousData` + * keeps serving the PREVIOUS term's matches until they land, and the first + * search of a session has no data at all. Navigation is gated on this: + * committing with Enter makes the typed and submitted terms agree instantly, + * so without it a second Enter would step through the old term's matches. + * + * A background refetch of the SAME term keeps this true — its data is still + * for this key — so an SSE row update doesn't disable the arrows mid-search. + */ + const findResultsAreCurrent = + submittedQuery.length > 0 && findData !== undefined && !isFindPlaceholder const findMatchesRef = useRef(findMatches) findMatchesRef.current = findMatches + const findResultsAreCurrentRef = useRef(findResultsAreCurrent) + findResultsAreCurrentRef.current = findResultsAreCurrent const currentMatchIndexRef = useRef(currentMatchIndex) currentMatchIndexRef.current = currentMatchIndex const findOpenRef = useRef(findOpen) @@ -1136,11 +1231,33 @@ export function TableGrid({ const wrapped = ((index % matches.length) + matches.length) % matches.length const match = matches[wrapped] setCurrentMatchIndex(wrapped) + // Claim the target NOW, not when the reveal lands. Paging is awaited below, + // and a same-term refetch during that window would otherwise re-point the + // cursor at the cell we are navigating AWAY from. + activeMatchRef.current = match setIsJumping(true) + // Paging to a distant match can outlast the next keystroke now that the + // search runs as the user types. Stamp this jump and drop it on return if a + // newer one started, or the grid would land on a superseded term's match. + const seq = ++goToMatchSeqRef.current try { await ensureRowsLoadedUpToRef.current(match.ordinal + 1) } finally { - setIsJumping(false) + if (seq === goToMatchSeqRef.current) setIsJumping(false) + } + if (seq !== goToMatchSeqRef.current) return + // The match set can change while we page — find hangs off the rows cache, + // so any row write or SSE update refetches it. If the target is gone, + // revealing it would select a cell that no longer matches and mark the + // cursor as sitting on a result, which then makes the next step skip the + // match that replaced it. + const stillMatches = findMatchesRef.current.some( + (m) => m.rowId === match.rowId && m.column === match.column + ) + if (!stillMatches) { + activeMatchRef.current = null + cursorIsOnMatchRef.current = false + return } // Defer the anchor set to the reveal effect: it must run after the freshly // loaded rows have committed, else scrollToIndex clamps to the stale count. @@ -1148,6 +1265,31 @@ export function TableGrid({ setPendingMatchTick((t) => t + 1) }, []) + /** + * Editing the query strands a jump still paging toward the previous term's + * match: without this, that jump can finish, pass its own sequence check, and + * reveal a cell that no longer matches — most visibly when the new term's + * first hit isn't loaded, so nothing else moves the selection afterwards. + * + * Declared above BOTH the reveal and the auto-reveal effects so it runs + * first. Effects fire in declaration order, so if a queued reveal and a + * keystroke land in the same commit, a cancel declared later would clear + * `pendingMatchRef` only after the reveal had already applied the stale match. + * + * Keyed on the LIVE input, not the debounced term: during the debounce window + * the submitted term still names the old search, so keying on it would leave + * that jump valid for another `SEARCH_DEBOUNCE_MS` after the box already + * shows something else. Clearing and closing land here too — both blank the + * input. + */ + useEffect(() => { + goToMatchSeqRef.current++ + pendingMatchRef.current = null + cursorIsOnMatchRef.current = false + activeMatchRef.current = null + setIsJumping(false) + }, [trimmedFindQuery, findOpen]) + /** * Reveal the pending match's cell once its row is in the loaded window. Keyed * on `rows` (new pages) and `pendingMatchTick` (so it fires even when the row @@ -1166,35 +1308,150 @@ export function TableGrid({ setIsColumnSelection(false) setRowSelection((prev) => (prev.kind === 'none' ? prev : ROW_SELECTION_NONE)) setSelectionFocus(null) + cursorIsOnMatchRef.current = true setSelectionAnchor({ rowIndex, colIndex }) }, [rows, displayColumns, pendingMatchTick]) - /** New result set (new submitted term) → reset to and reveal the first match. */ + /** + * Re-point the cursor at the match it is actually on after the set changes. + * + * The cursor is stored as an index, but the list underneath it is mutable: a + * row insert or delete elsewhere in the table reorders matches for the SAME + * term, and index 1 can silently become a different cell. Stepping from it + * would then revisit the cell the user is on, or skip its neighbour. Matching + * on (rowId, column) — the match's identity — keeps the cursor attached to the + * cell rather than the position. + * + * When the active match is gone from the set — its row deleted, its cell + * edited so it no longer matches — the cursor is released instead: it is no + * longer sitting on a hit, so the next step must LAND on the clamped index + * rather than move past it. Without that, deleting the match under the cursor + * makes Next skip the one that took its place. + */ + useEffect(() => { + const active = activeMatchRef.current + if (!active || findMatches.length === 0) return + const index = findMatches.findIndex( + (m) => m.rowId === active.rowId && m.column === active.column + ) + if (index === -1) { + activeMatchRef.current = null + cursorIsOnMatchRef.current = false + setCurrentMatchIndex((i) => Math.min(i, findMatches.length - 1)) + return + } + if (index !== currentMatchIndexRef.current) setCurrentMatchIndex(index) + }, [findMatches]) + + /** + * A new TERM resets to its first match and reveals it. + * + * Keyed on the term, not on `findMatches` identity: the find query hangs off + * the rows cache, so any row write or SSE update refetches it, and keying on + * the result set would yank a user reading match 7 back to match 1 whenever + * a workflow cell landed. + * + * The reveal is skipped when the match is outside the loaded window. + * `ensureRowsLoadedUpTo` pages sequentially, so a selective term whose first + * hit is 50k rows down would fire ~50 serial round trips — per typing pause, + * now that the search is live. Highlights and the count still cover the whole + * table; only the viewport jump waits for a deliberate Enter or next-click. + * + * That deliberate path still runs the same unbounded, uncancellable paging it + * always has; this only stops typing from triggering it. Bounding it properly + * wants a fetch-at-offset on the rows endpoint, which is a server change. + */ useEffect(() => { + if (submittedQuery.length === 0) { + // Clearing the box has to un-latch, or retyping the same term — the + // ordinary "did I typo that?" correction — would match the stale latch + // and neither reset the cursor nor reveal anything. + autoRevealedTermRef.current = '' + return + } + // Wait for THIS term's own result set. `keepPreviousData` leaves + // `findMatches` describing the previous term while the new one loads, and + // on the session's first search there is no previous data at all — so + // `isPlaceholderData` is false while the query is still pending. Latching + // in either window would burn the one auto-reveal this term gets. + if (isFindPlaceholder || isFindFetching) return + if (autoRevealedTermRef.current === submittedQuery) return + autoRevealedTermRef.current = submittedQuery setCurrentMatchIndex(0) - if (findMatches.length > 0) goToMatch(0) - }, [findMatches, goToMatch]) + cursorIsOnMatchRef.current = false + activeMatchRef.current = null + const first = findMatches[0] + if (!first) return + if (!rowsRef.current.some((r) => r.id === first.rowId)) return + goToMatch(0) + }, [submittedQuery, findMatches, isFindPlaceholder, isFindFetching, goToMatch]) - const handleFindSubmit = useCallback(() => { - setSubmittedQuery(findQuery.trim()) - }, [findQuery]) + /** + * Step to the next/previous match — or, when the cursor is not on a match + * yet, to the current index itself. That second case is the term whose first + * hit the auto-reveal skipped because its row wasn't loaded: `+1` there would + * silently step over the very match the user pressed Enter to reach, and it + * would only come back around after wrapping the whole list. + */ + /** + * The index the next step counts from, clamped into the CURRENT match set. + * + * A row write or SSE update can shrink or reorder the matches for a term the + * user is still navigating; the term latch deliberately leaves the cursor + * alone in that case, so the stored index can now point past the end. Stepping + * from it would wrap off a stale base and land somewhere unrelated to the + * match on screen. Clamping here rather than in the two callers keeps the + * stepping base and the displayed index in agreement. + */ + const stepBaseIndex = () => + Math.min(currentMatchIndexRef.current, Math.max(0, findMatchesRef.current.length - 1)) const handleFindNext = useCallback(() => { - goToMatch(currentMatchIndexRef.current + 1) + if (!findResultsAreCurrentRef.current) return + const index = stepBaseIndex() + goToMatch(cursorIsOnMatchRef.current ? index + 1 : index) }, [goToMatch]) const handleFindPrev = useCallback(() => { - goToMatch(currentMatchIndexRef.current - 1) + if (!findResultsAreCurrentRef.current) return + const index = stepBaseIndex() + goToMatch(cursorIsOnMatchRef.current ? index - 1 : index) }, [goToMatch]) + /** + * Closes the bar and leaves no trace of the search: the term, the highlights + * (via the emptied term), and the match cursor all go. + * + * The cell selection is deliberately left where it is. Restoring the cell the + * user was on before opening find reads nicely, but deciding whether the + * current selection belongs to find or to the user is not answerable here — + * the grid has ~15 places that move the selection and no notion of who owns + * it, so every heuristic (compare the anchor, also check the focus, clear on + * click, clear on keydown) mis-fires on some ordinary gesture: extending a + * range from a match, clicking the match cell itself, arrowing away and back, + * Cmd+Z, or Cmd+F to refocus the bar. Leaving the cursor on the last match is + * what Sheets does and what this grid already did before find was reworked. + */ const handleFindClose = useCallback(() => { setFindOpen(false) setFindQuery('') - setSubmittedQuery('') + setCurrentMatchIndex(0) pendingMatchRef.current = null + // Strands any jump still paging toward a match, so it can't reveal a cell + // after the bar is gone. + goToMatchSeqRef.current++ + autoRevealedTermRef.current = '' + cursorIsOnMatchRef.current = false + activeMatchRef.current = null + setIsJumping(false) scrollRef.current?.focus({ preventScroll: true }) }, []) + /** The grid's own Escape handler is bound once and closes find through the + * same path as the bar's Escape, so the two can't drift. */ + const handleFindCloseRef = useRef(handleFindClose) + handleFindCloseRef.current = handleFindClose + const columnRename = useInlineRename({ // `columnName` is the column id; record the prior display name + id so undo // restores the label (not the id) and targets the right column. @@ -2476,10 +2733,7 @@ export function TableGrid({ if (e.key === 'Escape') { e.preventDefault() if (findOpenRef.current) { - setFindOpen(false) - setFindQuery('') - setSubmittedQuery('') - pendingMatchRef.current = null + handleFindCloseRef.current() return } if (dragColumnNameRef.current) { @@ -4228,15 +4482,19 @@ export function TableGrid({ )} @@ -4537,6 +4795,7 @@ export function TableGrid({ activeDispatches={activeDispatches} pinnedOffsets={pinnedOffsets.size > 0 ? pinnedOffsets : undefined} lastPinnedColKey={lastPinnedColKey} + findMatchColumns={findMatchColumnsByRowId.get(row.id)} /> ) })} diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/search-params.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/search-params.ts index 5b510dc38e3..dd6776ded92 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/search-params.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/search-params.ts @@ -17,6 +17,15 @@ export const DEFAULT_TABLE_DETAIL_SORT_DIRECTION = 'asc' * recursive, arbitrarily-nested object (`$or`/`$and` combinators, per-column * operator objects); serializing it would put a large structured blob in the * URL, which the URL-state doctrine forbids. It stays in local `useState`. + * + * The in-grid `find` (Cmd+F) is likewise absent, for a different reason: it is + * a viewport cursor, not a destination. Two things rule it out. It is not one + * value but a cluster — the term, the match cursor, and the cell the user was + * on before opening find — and only the term is serializable; closing restores + * that pre-find cell from an in-memory ref, so a term that survived a reload + * would arrive with no origin to return to. And the search runs on every + * debounced keystroke rather than on submit, which is the write frequency this + * doctrine keeps out of the URL. Same call the browser's own Cmd+F makes. */ export const tableDetailParsers = { sort: parseAsString, diff --git a/apps/sim/hooks/queries/tables.ts b/apps/sim/hooks/queries/tables.ts index 7423d6b09de..8610e82d923 100644 --- a/apps/sim/hooks/queries/tables.ts +++ b/apps/sim/hooks/queries/tables.ts @@ -129,6 +129,14 @@ const logger = createLogger('TableQueries') export const TABLE_DETAIL_STALE_TIME = 30 * 1000 export const TABLE_RUN_STATE_STALE_TIME = 30 * 1000 export const TABLE_FIND_STALE_TIME = 30 * 1000 +/** + * Shorter than the 5-minute default: the grid searches as the user types, so + * each typing pause mints its own cache entry holding up to + * `TABLE_LIMITS.MAX_FIND_MATCHES` matches. Long enough that backspacing to a + * recent term is still instant, short enough that a typed-through term set + * doesn't sit resident. + */ +export const TABLE_FIND_GC_TIME = 60 * 1000 export const TABLE_ROWS_STALE_TIME = 30 * 1000 export const TABLE_EXPORT_JOBS_STALE_TIME = 5 * 1000 @@ -469,9 +477,11 @@ async function fetchTableRowMatches({ } /** - * Server-side find across all cells. `q` is the *submitted* term (search is - * Enter-triggered), so React Query caches each submitted term and re-searching - * a prior one is instant. Disabled while `q` is empty. + * Server-side find across all cells. `q` is the term the caller has settled on + * — the grid debounces the live input before passing it — so React Query caches + * each settled term and backspacing to a prior one is instant. Disabled while + * `q` is empty; `keepPreviousData` holds the last result set so the match count + * doesn't blank between terms. */ export function useFindTableRows({ workspaceId, tableId, q, filter, sort }: FindTableRowsParams) { const paramsKey = JSON.stringify({ q, filter: filter ?? null, sort: sort ?? null }) @@ -481,6 +491,7 @@ export function useFindTableRows({ workspaceId, tableId, q, filter, sort }: Find fetchTableRowMatches({ workspaceId, tableId, q, filter, sort, signal }), enabled: Boolean(workspaceId && tableId) && q.trim().length > 0, staleTime: TABLE_FIND_STALE_TIME, + gcTime: TABLE_FIND_GC_TIME, placeholderData: keepPreviousData, }) }