Skip to content

Commit 1dc75c4

Browse files
committed
fix(tables): apply filter edits from user events
1 parent bbd6956 commit 1dc75c4

4 files changed

Lines changed: 81 additions & 52 deletions

File tree

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,18 @@ describe('TableFilter', () => {
144144
expect(onChange).not.toHaveBeenCalled()
145145
})
146146

147+
it('does not autosave when columns refresh without a user edit', () => {
148+
const onChange = vi.fn()
149+
act(() => {
150+
root.render(<TableFilter columns={COLUMNS} filter={null} onChange={onChange} />)
151+
})
152+
act(() => {
153+
root.render(<TableFilter columns={[...COLUMNS]} filter={null} onChange={onChange} />)
154+
})
155+
156+
expect(onChange).not.toHaveBeenCalled()
157+
})
158+
147159
it('merges the OR groups as soon as the conjunction is toggled back to and', () => {
148160
const onChange = vi.fn()
149161
renderFilter(onChange, {

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

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

3-
import { memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'
3+
import { memo, useCallback, useMemo, useRef, useState } from 'react'
44
import { Button, ChipDropdown, ChipInput } from '@sim/emcn'
55
import { Plus, X } from '@sim/emcn/icons'
66
import { generateShortId } from '@sim/utils/id'
@@ -28,7 +28,6 @@ function selectFilterOperators(column: ColumnDefinition | undefined): Set<string
2828
return column?.multiple ? MULTI_SELECT_FILTER_OPERATORS : SINGLE_SELECT_FILTER_OPERATORS
2929
}
3030

31-
/** The predicate the panel's current rows amount to — blank rows contribute nothing. */
3231
function toAppliedPredicate(
3332
rules: FilterRule[],
3433
columns: ColumnDefinition[]
@@ -47,19 +46,34 @@ interface TableFilterProps {
4746

4847
export function TableFilter({ columns, filter, onChange }: TableFilterProps) {
4948
const lastAppliedFilterRef = useRef<string | undefined>(undefined)
50-
const onChangeRef = useRef(onChange)
5149
const [rules, setRules] = useState<FilterRule[]>(() => {
5250
const fromFilter = predicateToFilterRules(filter)
5351
return fromFilter.length > 0 ? fromFilter : [createRule(columns)]
5452
})
53+
const rulesRef = useRef(rules)
54+
rulesRef.current = rules
5555
// Seed the "already applied" signature from the rules the panel actually
5656
// renders, not the raw prop: a saved tree the flat builder cannot express
5757
// (deeply nested groups, wire key order) round-trips differently, and seeding
5858
// from the prop would fire an unedited autosave of that lossy form the
5959
// moment the panel opens. The normalized form persists only once the user
6060
// really edits a rule.
6161
lastAppliedFilterRef.current ??= JSON.stringify(toAppliedPredicate(rules, columns))
62-
onChangeRef.current = onChange
62+
63+
const applyRules = useCallback(
64+
(update: (current: FilterRule[]) => FilterRule[]) => {
65+
const nextRules = update(rulesRef.current)
66+
rulesRef.current = nextRules
67+
setRules(nextRules)
68+
69+
const nextFilter = toAppliedPredicate(nextRules, columns)
70+
const signature = JSON.stringify(nextFilter)
71+
if (signature === lastAppliedFilterRef.current) return
72+
lastAppliedFilterRef.current = signature
73+
onChange(nextFilter)
74+
},
75+
[columns, onChange]
76+
)
6377

6478
// `value` is the filter field key (column id); `label` is what the user sees.
6579
const columnOptions = useMemo(
@@ -73,71 +87,67 @@ export function TableFilter({ columns, filter, onChange }: TableFilterProps) {
7387
)
7488

7589
const handleAdd = useCallback(() => {
76-
setRules((prev) => [...prev, createRule(columns)])
77-
}, [columns])
90+
applyRules((current) => [...current, createRule(columns)])
91+
}, [applyRules, columns])
7892

7993
const handleRemove = useCallback(
8094
(id: string) => {
81-
setRules((prev) => {
82-
const next = prev.filter((rule) => rule.id !== id)
95+
applyRules((current) => {
96+
const next = current.filter((rule) => rule.id !== id)
8397
return next.length > 0 ? next : [createRule(columns)]
8498
})
8599
},
86-
[columns]
100+
[applyRules, columns]
87101
)
88102

89-
const handleUpdate = useCallback((id: string, field: keyof FilterRule, value: string) => {
90-
setRules((prev) => prev.map((r) => (r.id === id ? { ...r, [field]: value } : r)))
91-
}, [])
103+
const handleUpdate = useCallback(
104+
(id: string, field: keyof FilterRule, value: string) => {
105+
applyRules((current) =>
106+
current.map((rule) => (rule.id === id ? { ...rule, [field]: value } : rule))
107+
)
108+
},
109+
[applyRules]
110+
)
92111

93-
const handleToggleLogical = useCallback((id: string) => {
94-
setRules((prev) =>
95-
prev.map((r) =>
96-
r.id === id ? { ...r, logicalOperator: r.logicalOperator === 'and' ? 'or' : 'and' } : r
112+
const handleToggleLogical = useCallback(
113+
(id: string) => {
114+
applyRules((current) =>
115+
current.map((rule) =>
116+
rule.id === id
117+
? { ...rule, logicalOperator: rule.logicalOperator === 'and' ? 'or' : 'and' }
118+
: rule
119+
)
97120
)
98-
)
99-
}, [])
121+
},
122+
[applyRules]
123+
)
100124

101125
// Switching a rule's column across the select boundary changes what values and
102126
// operators are valid, so clear the value and coerce an unsupported operator
103127
// back to `eq` — otherwise a stale free-text value or a range operator would
104128
// apply against a select column and be rejected server-side.
105129
const handleColumnChange = useCallback(
106130
(id: string, columnId: string) => {
107-
setRules((prev) =>
108-
prev.map((r) => {
109-
if (r.id !== id) return r
110-
const previous = columnById.get(r.column)
131+
applyRules((current) =>
132+
current.map((rule) => {
133+
if (rule.id !== id) return rule
134+
const previous = columnById.get(rule.column)
111135
const next = columnById.get(columnId)
112136
const wasSelect = previous?.type === 'select'
113137
const isSelect = next?.type === 'select'
114-
if (!wasSelect && !isSelect) return { ...r, column: columnId }
138+
if (!wasSelect && !isSelect) return { ...rule, column: columnId }
115139
// Single- and multi-select take different operators, so a switch
116140
// between them has to fall back too, not just select ↔ non-select.
117141
const allowed = selectFilterOperators(next)
118142
const fallback = next?.multiple ? 'contains' : 'eq'
119-
const operator = isSelect && !allowed.has(r.operator) ? fallback : r.operator
120-
return { ...r, column: columnId, operator, value: '' }
143+
const operator = isSelect && !allowed.has(rule.operator) ? fallback : rule.operator
144+
return { ...rule, column: columnId, operator, value: '' }
121145
})
122146
)
123147
},
124-
[columnById]
148+
[applyRules, columnById]
125149
)
126150

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.
133-
useEffect(() => {
134-
const nextFilter = toAppliedPredicate(rules, columns)
135-
const signature = JSON.stringify(nextFilter)
136-
if (signature === lastAppliedFilterRef.current) return
137-
lastAppliedFilterRef.current = signature
138-
onChangeRef.current(nextFilter)
139-
}, [rules, columns])
140-
141151
return (
142152
<div className='border-[var(--border)] border-b bg-[var(--bg)] px-4 py-2'>
143153
<div className='flex flex-col gap-1'>

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/views-menu/views-menu.tsx

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import { memo, useEffect, useRef, useState } from 'react'
44
import {
5+
Button,
56
ChipChevronDown,
67
chipContentLabelClass,
78
chipVariants,
@@ -248,24 +249,28 @@ function ViewRow({ label, isActive, onSelect, defaultState, actions }: ViewRowPr
248249
{actionCount > 0 && (
249250
<div className='absolute right-1.5 flex items-center gap-0.5'>
250251
{actions?.map((action) => (
251-
<button
252+
<Button
252253
key={action.label}
253254
type='button'
255+
variant='quiet'
256+
size='icon'
254257
aria-label={action.label}
255258
title={action.label}
256259
onClick={(event) => {
257260
event.preventDefault()
258261
event.stopPropagation()
259262
action.onClick()
260263
}}
261-
className='pointer-events-none rounded-md p-1 text-[var(--text-icon)] opacity-0 transition-[background-color,color,opacity] hover-hover:bg-[var(--surface-active)] hover-hover:text-[var(--text-body)] group-focus-within/view:pointer-events-auto group-focus-within/view:opacity-100 group-hover/view:pointer-events-auto group-hover/view:opacity-100'
264+
className='pointer-events-none opacity-0 transition-[background-color,color,opacity] group-focus-within/view:pointer-events-auto group-focus-within/view:opacity-100 group-hover/view:pointer-events-auto group-hover/view:opacity-100'
262265
>
263266
<action.icon className='size-3' />
264-
</button>
267+
</Button>
265268
))}
266269
{defaultState && (
267-
<button
270+
<Button
268271
type='button'
272+
variant='quiet'
273+
size='icon'
269274
aria-label={defaultState.isDefault ? 'Current default view' : 'Set as default'}
270275
title={defaultState.isDefault ? 'Current default view' : 'Set as default'}
271276
disabled={!defaultState.onSetDefault}
@@ -274,10 +279,9 @@ function ViewRow({ label, isActive, onSelect, defaultState, actions }: ViewRowPr
274279
event.stopPropagation()
275280
defaultState.onSetDefault?.()
276281
}}
277-
className='rounded-md p-1 text-[var(--text-icon)] transition-colors hover-hover:bg-[var(--surface-active)] hover-hover:text-[var(--text-body)]'
278282
>
279283
<Pin className={cn('size-3', defaultState.isDefault && 'fill-current')} />
280-
</button>
284+
</Button>
281285
)}
282286
</div>
283287
)}

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,18 +1200,21 @@ export function Table({
12001200
[persistActiveViewConfig]
12011201
)
12021202

1203-
const handleHiddenColumnsChange = (next: string[]) => {
1204-
setHiddenColumns(next)
1205-
persistActiveViewConfig({ hiddenColumns: next })
1206-
}
1203+
const handleHiddenColumnsChange = useCallback(
1204+
(next: string[]) => {
1205+
setHiddenColumns(next)
1206+
persistActiveViewConfig({ hiddenColumns: next })
1207+
},
1208+
[persistActiveViewConfig]
1209+
)
12071210

12081211
/**
12091212
* "Filter by cell value" from the grid's cell context menu. Narrows the
12101213
* PRUNED filter, so a condition the current schema already invalidated is not
12111214
* resurrected, and opens the panel — a silently narrowed table would leave the
12121215
* user no way to see what was applied. Persists explicitly: the reseeded
1213-
* panel starts signature-matched to this filter, so its debounce alone would
1214-
* never save it.
1216+
* panel starts signature-matched to this filter, so its gesture handlers will
1217+
* not emit it again.
12151218
*/
12161219
const handleFilterByCellValue = (conditions: readonly Predicate[]) => {
12171220
const next = withCellValueFilter(effectiveFilter, conditions)

0 commit comments

Comments
 (0)