Skip to content

Commit a94d7de

Browse files
committed
fix(tables): isolate flagged view interactions
1 parent 58cd6c2 commit a94d7de

6 files changed

Lines changed: 147 additions & 21 deletions

File tree

apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-options/resource-options.test.tsx

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ describe('SortDropdown', () => {
8282
active: { column: 'name', direction: 'asc' },
8383
onSort,
8484
onClear,
85+
keepOpenOnSelect: true,
8586
}}
8687
/>
8788
)
@@ -98,4 +99,28 @@ describe('SortDropdown', () => {
9899
expect(onOpenChange).not.toHaveBeenCalledWith(false)
99100
expect(document.body.querySelectorAll('[role="menuitem"]')).toHaveLength(2)
100101
})
102+
103+
it('keeps the legacy close-on-select behavior by default', () => {
104+
const onOpenChange = vi.fn()
105+
const onSort = vi.fn()
106+
act(() => {
107+
root.render(
108+
<SortDropdown
109+
open
110+
onOpenChange={onOpenChange}
111+
config={{
112+
options: [{ id: 'name', label: 'Name', icon: ColumnIcon }],
113+
active: null,
114+
onSort,
115+
}}
116+
/>
117+
)
118+
})
119+
120+
const item = document.body.querySelector<HTMLElement>('[role="menuitem"]')
121+
act(() => item?.click())
122+
123+
expect(onSort).toHaveBeenCalledWith('name', 'desc')
124+
expect(onOpenChange).toHaveBeenCalledWith(false)
125+
})
101126
})

apps/sim/app/workspace/[workspaceId]/components/resource/components/resource-options/resource-options.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export interface SortConfig {
4848
active: { column: string; direction: SortDirection } | null
4949
onSort: (column: string, direction: SortDirection) => void
5050
onClear?: () => void
51+
keepOpenOnSelect?: boolean
5152
}
5253

5354
export interface FilterTag {
@@ -283,7 +284,7 @@ export const SortDropdown = memo(function SortDropdown({
283284
open,
284285
onOpenChange,
285286
}: SortDropdownProps) {
286-
const { options, active, onSort, onClear } = config
287+
const { options, active, onSort, onClear, keepOpenOnSelect = false } = config
287288

288289
return (
289290
<DropdownMenu modal={false} open={open} onOpenChange={onOpenChange}>
@@ -301,7 +302,7 @@ export const SortDropdown = memo(function SortDropdown({
301302
<>
302303
<DropdownMenuItem
303304
onSelect={(event) => {
304-
event.preventDefault()
305+
if (keepOpenOnSelect) event.preventDefault()
305306
onClear()
306307
}}
307308
>
@@ -320,7 +321,7 @@ export const SortDropdown = memo(function SortDropdown({
320321
<DropdownMenuItem
321322
key={option.id}
322323
onSelect={(event) => {
323-
event.preventDefault()
324+
if (keepOpenOnSelect) event.preventDefault()
324325
if (isActive) {
325326
onSort(option.id, active.direction === 'asc' ? 'desc' : 'asc')
326327
} else {

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

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,20 @@ afterEach(() => {
2626

2727
function renderFilter(
2828
onChange: (filter: TablePredicate | null) => void,
29-
filter: TablePredicate | null = null
29+
filter: TablePredicate | null = null,
30+
autoApply = true,
31+
onClose: () => void = vi.fn()
3032
) {
3133
act(() => {
32-
root.render(<TableFilter columns={COLUMNS} filter={filter} onChange={onChange} />)
34+
root.render(
35+
<TableFilter
36+
columns={COLUMNS}
37+
filter={filter}
38+
autoApply={autoApply}
39+
onChange={onChange}
40+
onClose={onClose}
41+
/>
42+
)
3343
})
3444
}
3545

@@ -106,6 +116,25 @@ describe('TableFilter', () => {
106116
expect(container.textContent).not.toContain('Clear filters')
107117
})
108118

119+
it('keeps the legacy Apply flow while automatic view saves are disabled', () => {
120+
const onChange = vi.fn()
121+
renderFilter(onChange, null, false)
122+
const input = valueInput()
123+
124+
act(() => typeInto(input, 'Ada'))
125+
act(() => input?.dispatchEvent(new FocusEvent('focusout', { bubbles: true })))
126+
127+
expect(onChange).not.toHaveBeenCalled()
128+
const applyButton = Array.from(container.querySelectorAll('button')).find(
129+
(button) => button.textContent?.trim() === 'Apply filter'
130+
)
131+
act(() => applyButton?.click())
132+
133+
expect(onChange).toHaveBeenCalledWith({
134+
all: [{ field: 'col-name', op: 'eq', value: 'Ada' }],
135+
})
136+
})
137+
109138
it('clears the active filter as soon as its last rule is removed', () => {
110139
const onChange = vi.fn()
111140
renderFilter(onChange, {
@@ -174,10 +203,12 @@ describe('TableFilter', () => {
174203
it('does not autosave when columns refresh without a user edit', () => {
175204
const onChange = vi.fn()
176205
act(() => {
177-
root.render(<TableFilter columns={COLUMNS} filter={null} onChange={onChange} />)
206+
root.render(<TableFilter columns={COLUMNS} filter={null} autoApply onChange={onChange} />)
178207
})
179208
act(() => {
180-
root.render(<TableFilter columns={[...COLUMNS]} filter={null} onChange={onChange} />)
209+
root.render(
210+
<TableFilter columns={[...COLUMNS]} filter={null} autoApply onChange={onChange} />
211+
)
181212
})
182213

183214
expect(onChange).not.toHaveBeenCalled()

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

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

33
import { memo, useCallback, useMemo, useRef, useState } from 'react'
4-
import { Button, ChipDropdown, ChipInput } from '@sim/emcn'
4+
import { Button, ChipDropdown, ChipInput, cn } from '@sim/emcn'
55
import { Plus, X } from '@sim/emcn/icons'
66
import { generateShortId } from '@sim/utils/id'
77
import type { ColumnDefinition, FilterRule, TablePredicate } from '@/lib/table'
@@ -43,10 +43,18 @@ function isCompleteRule(rule: FilterRule): boolean {
4343
interface TableFilterProps {
4444
columns: ColumnDefinition[]
4545
filter: TablePredicate | null
46+
autoApply?: boolean
4647
onChange: (filter: TablePredicate | null) => void
48+
onClose?: () => void
4749
}
4850

49-
export function TableFilter({ columns, filter, onChange }: TableFilterProps) {
51+
export function TableFilter({
52+
columns,
53+
filter,
54+
autoApply = false,
55+
onChange,
56+
onClose,
57+
}: TableFilterProps) {
5058
const lastAppliedFilterRef = useRef<string | undefined>(undefined)
5159
const [rules, setRules] = useState<FilterRule[]>(() => {
5260
const fromFilter = predicateToFilterRules(filter)
@@ -67,6 +75,7 @@ export function TableFilter({ columns, filter, onChange }: TableFilterProps) {
6775
const nextRules = update(rulesRef.current)
6876
rulesRef.current = nextRules
6977
setRules(nextRules)
78+
if (!autoApply) return
7079

7180
const deferredRule = nextRules.find((rule) => rule.id === deferIncompleteRuleId)
7281
if (deferredRule && !isCompleteRule(deferredRule)) return
@@ -77,7 +86,7 @@ export function TableFilter({ columns, filter, onChange }: TableFilterProps) {
7786
lastAppliedFilterRef.current = signature
7887
onChange(nextFilter)
7988
},
80-
[columns, onChange]
89+
[autoApply, columns, onChange]
8190
)
8291

8392
// `value` is the filter field key (column id); `label` is what the user sees.
@@ -97,12 +106,26 @@ export function TableFilter({ columns, filter, onChange }: TableFilterProps) {
97106

98107
const handleRemove = useCallback(
99108
(id: string) => {
109+
if (!autoApply) {
110+
const nextRules = rulesRef.current.filter((rule) => rule.id !== id)
111+
if (nextRules.length > 0) {
112+
rulesRef.current = nextRules
113+
setRules(nextRules)
114+
return
115+
}
116+
const resetRules = [createRule(columns)]
117+
rulesRef.current = resetRules
118+
setRules(resetRules)
119+
onChange(null)
120+
onClose?.()
121+
return
122+
}
100123
applyRules((current) => {
101124
const next = current.filter((rule) => rule.id !== id)
102125
return next.length > 0 ? next : [createRule(columns)]
103126
})
104127
},
105-
[applyRules, columns]
128+
[applyRules, autoApply, columns, onChange, onClose]
106129
)
107130

108131
const handleUpdate = useCallback(
@@ -156,6 +179,17 @@ export function TableFilter({ columns, filter, onChange }: TableFilterProps) {
156179
[applyRules, columnById]
157180
)
158181

182+
const handleApply = useCallback(() => {
183+
onChange(toAppliedPredicate(rulesRef.current, columns))
184+
}, [columns, onChange])
185+
186+
const handleClear = () => {
187+
const resetRules = [createRule(columns)]
188+
rulesRef.current = resetRules
189+
setRules(resetRules)
190+
onChange(null)
191+
}
192+
159193
return (
160194
<div className='border-[var(--border)] border-b bg-[var(--bg)] px-4 py-2'>
161195
<div className='flex flex-col gap-1'>
@@ -169,11 +203,13 @@ export function TableFilter({ columns, filter, onChange }: TableFilterProps) {
169203
onUpdate={handleUpdate}
170204
onColumnChange={handleColumnChange}
171205
onRemove={handleRemove}
206+
autoApply={autoApply}
207+
onApply={handleApply}
172208
onToggleLogical={handleToggleLogical}
173209
/>
174210
))}
175211

176-
<div className='mt-1 flex items-center'>
212+
<div className={cn('mt-1 flex items-center', !autoApply && 'justify-between')}>
177213
<Button
178214
variant='ghost'
179215
size='sm'
@@ -183,6 +219,23 @@ export function TableFilter({ columns, filter, onChange }: TableFilterProps) {
183219
<Plus className='mr-1 size-[10px]' />
184220
Add filter
185221
</Button>
222+
{!autoApply && (
223+
<div className='flex items-center gap-1.5'>
224+
{filter !== null && (
225+
<Button
226+
variant='ghost'
227+
size='sm'
228+
onClick={handleClear}
229+
className='px-2 py-1 text-[var(--text-secondary)] text-xs'
230+
>
231+
Clear filters
232+
</Button>
233+
)}
234+
<Button variant='default' size='sm' onClick={handleApply} className='text-xs'>
235+
Apply filter
236+
</Button>
237+
</div>
238+
)}
186239
</div>
187240
</div>
188241
</div>
@@ -197,6 +250,8 @@ interface FilterRuleRowProps {
197250
onUpdate: (id: string, field: keyof FilterRule, value: string) => void
198251
onColumnChange: (id: string, columnId: string) => void
199252
onRemove: (id: string) => void
253+
autoApply: boolean
254+
onApply: () => void
200255
onToggleLogical: (id: string) => void
201256
}
202257

@@ -208,6 +263,8 @@ const FilterRuleRow = memo(function FilterRuleRow({
208263
onUpdate,
209264
onColumnChange,
210265
onRemove,
266+
autoApply,
267+
onApply,
211268
onToggleLogical,
212269
}: FilterRuleRowProps) {
213270
// Keep a stale column id selectable/visible (e.g. after the column was
@@ -281,11 +338,21 @@ const FilterRuleRow = memo(function FilterRuleRow({
281338
matchTriggerWidth={false}
282339
className='min-w-[100px] flex-1'
283340
/>
284-
) : (
341+
) : autoApply ? (
285342
<FilterValueInput
286343
value={rule.value}
287344
onCommit={(value) => onUpdate(rule.id, 'value', value)}
288345
/>
346+
) : (
347+
<ChipInput
348+
value={rule.value}
349+
onChange={(event) => onUpdate(rule.id, 'value', event.target.value)}
350+
onKeyDown={(event) => {
351+
if (event.key === 'Enter') onApply()
352+
}}
353+
placeholder='Enter a value'
354+
className='flex-1'
355+
/>
289356
)}
290357

291358
<Button

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,8 @@ interface TableProps {
119119
tableLocksEnabled?: boolean
120120
/**
121121
* Resolved `table-views` flag. Server-only to resolve for the same reason.
122-
* Defaults to `false` so the embedded mothership table — which has no server
123-
* context to resolve it — stays on today's Filter/Sort bar.
122+
* Defaults to `false` so any caller that has not resolved the flag stays on
123+
* today's Filter/Sort behavior.
124124
*/
125125
viewsEnabled?: boolean
126126
}
@@ -1177,8 +1177,9 @@ export function Table({
11771177
active: sortColumn ? { column: sortColumn, direction: sortDirection } : null,
11781178
onSort: handleSortColumn,
11791179
onClear: handleClearSort,
1180+
keepOpenOnSelect: viewsEnabled,
11801181
}),
1181-
[columnOptions, sortColumn, sortDirection, handleSortColumn, handleClearSort]
1182+
[columnOptions, sortColumn, sortDirection, handleSortColumn, handleClearSort, viewsEnabled]
11821183
)
11831184

11841185
const handleFilterChange = useCallback(
@@ -1530,7 +1531,9 @@ export function Table({
15301531
key={filterSeed}
15311532
columns={columns}
15321533
filter={effectiveFilter}
1534+
autoApply={viewsEnabled}
15331535
onChange={handleFilterChange}
1536+
onClose={() => setFilterOpen(false)}
15341537
/>
15351538
)}
15361539
<SaveViewModal

apps/sim/lib/core/config/feature-flags.ts

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,10 @@ const FEATURE_FLAGS = {
136136
'table-views': {
137137
description:
138138
'Saved table views (named filter/sort/column-visibility presets) plus the column show/hide ' +
139-
'menu, in the table-detail options bar. UI-only gate: resolved in the table page (server) ' +
140-
"and passed down, so the table falls back to today's Filter/Sort bar when off. The routes " +
141-
'and the table_views table ship ungated — they are inert with no UI to call them, and a view ' +
142-
'saved during a rollout must survive the flag being toggled back off. Embedded (mothership) ' +
143-
'tables render without views regardless, since no server context resolves the flag there. ' +
139+
'menu. UI-only gate: resolved server-side for table-detail and embedded tables, then passed ' +
140+
"down so both surfaces fall back to today's Filter/Sort behavior when off. The routes and " +
141+
'the table_views table ship ungated, and new or forked tables still seed their view data, so ' +
142+
'a saved view survives the flag being toggled off and can be restored when it is re-enabled. ' +
144143
'Off-AppConfig falls back to TABLE_VIEWS.',
145144
fallback: 'TABLE_VIEWS',
146145
},

0 commit comments

Comments
 (0)