Skip to content

Commit e4f0403

Browse files
j15zclaude
andcommitted
feat(tables): apply filter text on enter or blur instead of a debounce
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0d65580 commit e4f0403

4 files changed

Lines changed: 109 additions & 157 deletions

File tree

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
export { TableFilter, type TableFilterHandle } from './table-filter'
1+
export { TableFilter } from './table-filter'

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

Lines changed: 52 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,11 @@
11
/**
22
* @vitest-environment jsdom
33
*/
4-
import { act, createRef, type Ref } from 'react'
4+
import { act } from 'react'
55
import { createRoot, type Root } from 'react-dom/client'
66
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
77
import type { ColumnDefinition, TablePredicate } from '@/lib/table'
8-
import {
9-
FILTER_DEBOUNCE_MS,
10-
TableFilter,
11-
type TableFilterHandle,
12-
} from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-filter/table-filter'
8+
import { TableFilter } from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-filter/table-filter'
139

1410
const COLUMNS: ColumnDefinition[] = [{ id: 'col-name', name: 'Name', type: 'string' }]
1511

@@ -18,7 +14,6 @@ let root: Root
1814

1915
beforeEach(() => {
2016
globalThis.IS_REACT_ACT_ENVIRONMENT = true
21-
vi.useFakeTimers()
2217
container = document.createElement('div')
2318
document.body.appendChild(container)
2419
root = createRoot(container)
@@ -27,41 +22,70 @@ beforeEach(() => {
2722
afterEach(() => {
2823
act(() => root.unmount())
2924
container.remove()
30-
vi.useRealTimers()
3125
})
3226

3327
function renderFilter(
3428
onChange: (filter: TablePredicate | null) => void,
35-
filter: TablePredicate | null = null,
36-
ref?: Ref<TableFilterHandle>
29+
filter: TablePredicate | null = null
3730
) {
3831
act(() => {
39-
root.render(<TableFilter ref={ref} columns={COLUMNS} filter={filter} onChange={onChange} />)
32+
root.render(<TableFilter columns={COLUMNS} filter={filter} onChange={onChange} />)
4033
})
4134
}
4235

36+
function valueInput(): HTMLInputElement | null {
37+
return container.querySelector<HTMLInputElement>('input[placeholder="Enter a value"]')
38+
}
39+
40+
function typeInto(input: HTMLInputElement | null, value: string) {
41+
if (!input) return
42+
Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set?.call(input, value)
43+
input.dispatchEvent(new Event('input', { bubbles: true }))
44+
}
45+
4346
describe('TableFilter', () => {
44-
it('applies text filters after a short typing delay', () => {
45-
const onApply = vi.fn()
46-
renderFilter(onApply)
47-
const input = container.querySelector<HTMLInputElement>('input[placeholder="Enter a value"]')
47+
it('commits a typed value on blur, not per keystroke', () => {
48+
const onChange = vi.fn()
49+
renderFilter(onChange)
50+
const input = valueInput()
4851
expect(input).not.toBeNull()
4952

50-
act(() => {
51-
if (!input) return
52-
Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set?.call(input, 'Ada')
53-
input.dispatchEvent(new Event('input', { bubbles: true }))
54-
})
53+
act(() => typeInto(input, 'Ada'))
54+
expect(onChange).not.toHaveBeenCalled()
5555

56-
expect(onApply).not.toHaveBeenCalled()
57-
act(() => vi.advanceTimersByTime(FILTER_DEBOUNCE_MS - 1))
58-
expect(onApply).not.toHaveBeenCalled()
59-
act(() => vi.advanceTimersByTime(1))
60-
expect(onApply).toHaveBeenCalledWith({
56+
act(() => input?.dispatchEvent(new FocusEvent('focusout', { bubbles: true })))
57+
expect(onChange).toHaveBeenCalledTimes(1)
58+
expect(onChange).toHaveBeenCalledWith({
6159
all: [{ field: 'col-name', op: 'eq', value: 'Ada' }],
6260
})
6361
})
6462

63+
it('commits a typed value on Enter', () => {
64+
const onChange = vi.fn()
65+
renderFilter(onChange)
66+
const input = valueInput()
67+
68+
act(() => typeInto(input, 'Grace'))
69+
act(() => input?.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })))
70+
71+
expect(onChange).toHaveBeenCalledTimes(1)
72+
expect(onChange).toHaveBeenCalledWith({
73+
all: [{ field: 'col-name', op: 'eq', value: 'Grace' }],
74+
})
75+
})
76+
77+
it('does not re-commit an unchanged value on blur after Enter', () => {
78+
const onChange = vi.fn()
79+
renderFilter(onChange)
80+
const input = valueInput()
81+
82+
act(() => typeInto(input, 'Ada'))
83+
act(() => input?.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })))
84+
act(() => input?.dispatchEvent(new FocusEvent('focusout', { bubbles: true })))
85+
86+
expect(onChange).toHaveBeenCalledTimes(1)
87+
})
88+
6589
it('offers a toggleable conjunction without apply or clear actions', () => {
6690
renderFilter(vi.fn())
6791
const addFilter = Array.from(container.querySelectorAll('button')).find((button) =>
@@ -82,51 +106,7 @@ describe('TableFilter', () => {
82106
expect(container.textContent).not.toContain('Clear filters')
83107
})
84108

85-
it('flushes the pending filter when the panel closes before the delay', () => {
86-
const onChange = vi.fn()
87-
const filterRef = createRef<TableFilterHandle>()
88-
renderFilter(onChange, null, filterRef)
89-
const input = container.querySelector<HTMLInputElement>('input[placeholder="Enter a value"]')
90-
91-
act(() => {
92-
if (!input) return
93-
Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set?.call(input, 'Ada')
94-
input.dispatchEvent(new Event('input', { bubbles: true }))
95-
})
96-
act(() => {
97-
filterRef.current?.flush()
98-
})
99-
100-
expect(onChange).toHaveBeenCalledTimes(1)
101-
expect(onChange).toHaveBeenCalledWith({
102-
all: [{ field: 'col-name', op: 'eq', value: 'Ada' }],
103-
})
104-
act(() => vi.advanceTimersByTime(FILTER_DEBOUNCE_MS))
105-
expect(onChange).toHaveBeenCalledTimes(1)
106-
})
107-
108-
it('cancels the previous debounce when typing continues', () => {
109-
const onChange = vi.fn()
110-
renderFilter(onChange)
111-
const input = container.querySelector<HTMLInputElement>('input[placeholder="Enter a value"]')
112-
const setInput = (value: string) => {
113-
if (!input) return
114-
Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')?.set?.call(input, value)
115-
input.dispatchEvent(new Event('input', { bubbles: true }))
116-
}
117-
118-
act(() => setInput('Ada'))
119-
act(() => vi.advanceTimersByTime(FILTER_DEBOUNCE_MS - 1))
120-
act(() => setInput('Grace'))
121-
act(() => vi.advanceTimersByTime(FILTER_DEBOUNCE_MS))
122-
123-
expect(onChange).toHaveBeenCalledTimes(1)
124-
expect(onChange).toHaveBeenCalledWith({
125-
all: [{ field: 'col-name', op: 'eq', value: 'Grace' }],
126-
})
127-
})
128-
129-
it('clears the active filter when its last rule is removed', () => {
109+
it('clears the active filter as soon as its last rule is removed', () => {
130110
const onChange = vi.fn()
131111
renderFilter(onChange, {
132112
all: [{ field: 'col-name', op: 'eq', value: 'Ada' }],
@@ -136,20 +116,15 @@ describe('TableFilter', () => {
136116
'button[aria-label="Remove filter"]'
137117
)
138118
act(() => removeButton?.click())
139-
act(() => vi.advanceTimersByTime(FILTER_DEBOUNCE_MS))
140119

141120
expect(onChange).toHaveBeenCalledWith(null)
142-
expect(
143-
container.querySelector<HTMLInputElement>('input[placeholder="Enter a value"]')?.value
144-
).toBe('')
121+
expect(valueInput()?.value).toBe('')
145122
})
146123

147124
it('preserves saved isNull conditions instead of dropping them', () => {
148125
const onChange = vi.fn()
149126
renderFilter(onChange, { all: [{ field: 'col-name', op: 'isNull' }] })
150127

151-
act(() => vi.advanceTimersByTime(FILTER_DEBOUNCE_MS))
152-
153128
expect(onChange).not.toHaveBeenCalled()
154129
})
155130

@@ -166,12 +141,10 @@ describe('TableFilter', () => {
166141
(button) => button.textContent?.trim() === 'or'
167142
)
168143
expect(orToggle).toBeDefined()
169-
170-
act(() => vi.advanceTimersByTime(FILTER_DEBOUNCE_MS))
171144
expect(onChange).not.toHaveBeenCalled()
172145
})
173146

174-
it('merges the OR groups when the conjunction is toggled back to and', () => {
147+
it('merges the OR groups as soon as the conjunction is toggled back to and', () => {
175148
const onChange = vi.fn()
176149
renderFilter(onChange, {
177150
any: [
@@ -184,7 +157,6 @@ describe('TableFilter', () => {
184157
(button) => button.textContent?.trim() === 'or'
185158
)
186159
act(() => orToggle?.click())
187-
act(() => vi.advanceTimersByTime(FILTER_DEBOUNCE_MS))
188160

189161
expect(onChange).toHaveBeenCalledWith({
190162
all: [

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

Lines changed: 53 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,6 @@
11
'use client'
22

3-
import {
4-
forwardRef,
5-
memo,
6-
useCallback,
7-
useEffect,
8-
useImperativeHandle,
9-
useMemo,
10-
useRef,
11-
useState,
12-
} from 'react'
3+
import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'
134
import { Button, ChipDropdown, ChipInput } from '@sim/emcn'
145
import { Plus, X } from '@sim/emcn/icons'
156
import { generateShortId } from '@sim/utils/id'
@@ -33,8 +24,6 @@ const MULTI_SELECT_COMPARISON_OPERATORS = COMPARISON_OPERATORS.filter((o) =>
3324
MULTI_SELECT_FILTER_OPERATORS.has(o.value)
3425
)
3526

36-
export const FILTER_DEBOUNCE_MS = 250
37-
3827
function selectFilterOperators(column: ColumnDefinition | undefined): Set<string> {
3928
return column?.multiple ? MULTI_SELECT_FILTER_OPERATORS : SINGLE_SELECT_FILTER_OPERATORS
4029
}
@@ -56,31 +45,17 @@ interface TableFilterProps {
5645
onChange: (filter: TablePredicate | null) => void
5746
}
5847

59-
export interface TableFilterHandle {
60-
flush: () => void
61-
}
62-
63-
interface PendingFilter {
64-
filter: TablePredicate | null
65-
signature: string
66-
}
67-
68-
export const TableFilter = forwardRef<TableFilterHandle, TableFilterProps>(function TableFilter(
69-
{ columns, filter, onChange },
70-
ref
71-
) {
48+
export function TableFilter({ columns, filter, onChange }: TableFilterProps) {
7249
const lastAppliedFilterRef = useRef<string | undefined>(undefined)
7350
const onChangeRef = useRef(onChange)
74-
const pendingFilterRef = useRef<PendingFilter | null>(null)
75-
const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null)
7651
const [rules, setRules] = useState<FilterRule[]>(() => {
7752
const fromFilter = predicateToFilterRules(filter)
7853
return fromFilter.length > 0 ? fromFilter : [createRule(columns)]
7954
})
8055
// Seed the "already applied" signature from the rules the panel actually
8156
// renders, not the raw prop: a saved tree the flat builder cannot express
8257
// (deeply nested groups, wire key order) round-trips differently, and seeding
83-
// from the prop would schedule an unedited autosave of that lossy form the
58+
// from the prop would fire an unedited autosave of that lossy form the
8459
// moment the panel opens. The normalized form persists only once the user
8560
// really edits a rule.
8661
lastAppliedFilterRef.current ??= JSON.stringify(toAppliedPredicate(rules, columns))
@@ -149,41 +124,19 @@ export const TableFilter = forwardRef<TableFilterHandle, TableFilterProps>(funct
149124
[columnById]
150125
)
151126

152-
const flush = useCallback(() => {
153-
const pending = pendingFilterRef.current
154-
if (!pending) return
155-
156-
if (timeoutRef.current) clearTimeout(timeoutRef.current)
157-
timeoutRef.current = null
158-
pendingFilterRef.current = null
159-
lastAppliedFilterRef.current = pending.signature
160-
onChangeRef.current(pending.filter)
161-
}, [])
162-
163-
useImperativeHandle(ref, () => ({ flush }), [flush])
164-
127+
// Applies on every rules change. Rules only change on completed gestures —
128+
// dropdown picks, row add/remove, conjunction toggles, and the value field's
129+
// Enter/blur commit ({@link FilterValueInput} buffers keystrokes locally) —
130+
// so nothing is ever pending and there is nothing to lose on unmount. The
131+
// signature guard keeps no-op changes (a blank row added, an untouched
132+
// reseed) from writing.
165133
useEffect(() => {
166134
const nextFilter = toAppliedPredicate(rules, columns)
167135
const signature = JSON.stringify(nextFilter)
168-
if (signature === lastAppliedFilterRef.current) {
169-
pendingFilterRef.current = null
170-
return
171-
}
172-
173-
const pending = { filter: nextFilter, signature }
174-
pendingFilterRef.current = pending
175-
const timeout = setTimeout(() => {
176-
if (pendingFilterRef.current !== pending) return
177-
timeoutRef.current = null
178-
flush()
179-
}, FILTER_DEBOUNCE_MS)
180-
timeoutRef.current = timeout
181-
182-
return () => {
183-
clearTimeout(timeout)
184-
if (timeoutRef.current === timeout) timeoutRef.current = null
185-
}
186-
}, [rules, columns, flush])
136+
if (signature === lastAppliedFilterRef.current) return
137+
lastAppliedFilterRef.current = signature
138+
onChangeRef.current(nextFilter)
139+
}, [rules, columns])
187140

188141
return (
189142
<div className='border-[var(--border)] border-b bg-[var(--bg)] px-4 py-2'>
@@ -216,7 +169,7 @@ export const TableFilter = forwardRef<TableFilterHandle, TableFilterProps>(funct
216169
</div>
217170
</div>
218171
)
219-
})
172+
}
220173

221174
interface FilterRuleRowProps {
222175
rule: FilterRule
@@ -311,11 +264,9 @@ const FilterRuleRow = memo(function FilterRuleRow({
311264
className='min-w-[100px] flex-1'
312265
/>
313266
) : (
314-
<ChipInput
267+
<FilterValueInput
315268
value={rule.value}
316-
onChange={(e) => onUpdate(rule.id, 'value', e.target.value)}
317-
placeholder='Enter a value'
318-
className='flex-1'
269+
onCommit={(value) => onUpdate(rule.id, 'value', value)}
319270
/>
320271
)}
321272

@@ -332,6 +283,43 @@ const FilterRuleRow = memo(function FilterRuleRow({
332283
)
333284
})
334285

286+
interface FilterValueInputProps {
287+
value: string
288+
onCommit: (value: string) => void
289+
}
290+
291+
/**
292+
* Locally buffered value field: keystrokes stay in the field until Enter or
293+
* blur commits them — the spreadsheet-cell contract. Every click-driven exit
294+
* from the panel (closing it, switching views, navigating away) blurs the
295+
* field first, so finishing-by-leaving commits without any imperative
296+
* coordination. An external reseed (column switch clearing the value, a view
297+
* replacement remount) adopts the incoming value over the draft.
298+
*/
299+
function FilterValueInput({ value, onCommit }: FilterValueInputProps) {
300+
const [draft, setDraft] = useState(value)
301+
const [prevValue, setPrevValue] = useState(value)
302+
if (prevValue !== value) {
303+
setPrevValue(value)
304+
setDraft(value)
305+
}
306+
307+
return (
308+
<ChipInput
309+
value={draft}
310+
onChange={(e) => setDraft(e.target.value)}
311+
onBlur={() => {
312+
if (draft !== value) onCommit(draft)
313+
}}
314+
onKeyDown={(e) => {
315+
if (e.key === 'Enter' && draft !== value) onCommit(draft)
316+
}}
317+
placeholder='Enter a value'
318+
className='flex-1'
319+
/>
320+
)
321+
}
322+
335323
function createRule(columns: ColumnDefinition[]): FilterRule {
336324
const first = columns[0]
337325
return {

0 commit comments

Comments
 (0)