Skip to content

Commit c584cde

Browse files
fix(tables): scope layout undo to the view that recorded it
Layout is view-owned, but UndoEntry carried no owner, so the persistence sink — rebound every render to the active view — decided the target at undo time rather than at record time. Reordering in view A then undoing from view B wrote A's column order into B and left A unchanged. Entries are now stamped with the active view id, and the three layout-bearing action types (create-column, delete-column, reorder-columns) are pruned from both stacks when the active view changes. Row and schema actions are table-scoped and survive the switch untouched. Inert when views are disabled: the id is always null, so nothing is ever pruned.
1 parent a75b290 commit c584cde

6 files changed

Lines changed: 136 additions & 4 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,7 @@ export function TableGrid({
730730
getPinnedColumns,
731731
getColumnWidths,
732732
onPersistLayout,
733+
activeViewId: viewLayoutKey,
733734
})
734735
const undoRef = useRef(undo)
735736
undoRef.current = undo

apps/sim/hooks/use-table-undo.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ const mockPush = vi.fn()
4141
const mockPatchRedoRowId = vi.fn()
4242
const mockPatchUndoRowId = vi.fn()
4343
const mockClear = vi.fn()
44+
const mockPruneLayoutActions = vi.fn()
4445

4546
const storeState = {
4647
stacks: {},
@@ -50,6 +51,7 @@ const storeState = {
5051
patchRedoRowId: mockPatchRedoRowId,
5152
patchUndoRowId: mockPatchUndoRowId,
5253
clear: mockClear,
54+
pruneLayoutActions: mockPruneLayoutActions,
5355
}
5456

5557
vi.mock('@/stores/table/store', () => ({

apps/sim/hooks/use-table-undo.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,13 @@ interface UseTableUndoProps {
110110
* reorder was stored rather than silently rewriting the "All" layout.
111111
*/
112112
onPersistLayout?: (patch: TableMetadata) => void
113+
/**
114+
* Active view id (`null` for "All" or when views are disabled). Layout is
115+
* view-owned, so recorded layout actions are stamped with it and dropped when
116+
* the user switches away — otherwise an undo would write the outgoing view's
117+
* column order into whichever view happens to be active at the time.
118+
*/
119+
activeViewId?: string | null
113120
}
114121

115122
export function useTableUndo({
@@ -123,13 +130,15 @@ export function useTableUndo({
123130
getPinnedColumns,
124131
getColumnWidths,
125132
onPersistLayout,
133+
activeViewId = null,
126134
}: UseTableUndoProps) {
127135
const push = useTableUndoStore((s) => s.push)
128136
const popUndo = useTableUndoStore((s) => s.popUndo)
129137
const popRedo = useTableUndoStore((s) => s.popRedo)
130138
const patchRedoRowId = useTableUndoStore((s) => s.patchRedoRowId)
131139
const patchUndoRowId = useTableUndoStore((s) => s.patchUndoRowId)
132140
const clear = useTableUndoStore((s) => s.clear)
141+
const pruneLayoutActions = useTableUndoStore((s) => s.pruneLayoutActions)
133142
const canUndo = useTableUndoStore((s) => (s.stacks[tableId]?.undo.length ?? 0) > 0)
134143
const canRedo = useTableUndoStore((s) => (s.stacks[tableId]?.redo.length ?? 0) > 0)
135144

@@ -161,14 +170,20 @@ export function useTableUndo({
161170
getColumnWidthsRef.current = getColumnWidths
162171
const getLocksRef = useRef(getLocks)
163172
getLocksRef.current = getLocks
173+
const activeViewIdRef = useRef(activeViewId)
174+
activeViewIdRef.current = activeViewId
164175

165176
useEffect(() => {
166177
return () => clear(tableId)
167178
}, [clear, tableId])
168179

180+
useEffect(() => {
181+
pruneLayoutActions(tableId, activeViewId)
182+
}, [pruneLayoutActions, tableId, activeViewId])
183+
169184
const pushUndo = useCallback(
170185
(action: TableUndoAction) => {
171-
push(tableId, action)
186+
push(tableId, action, activeViewIdRef.current)
172187
},
173188
[push, tableId]
174189
)
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it } from 'vitest'
5+
import { useTableUndoStore } from '@/stores/table/store'
6+
import type { TableUndoAction } from '@/stores/table/types'
7+
8+
const TABLE = 'tbl-1'
9+
10+
const reorder: TableUndoAction = {
11+
type: 'reorder-columns',
12+
previousOrder: ['a', 'b'],
13+
newOrder: ['b', 'a'],
14+
}
15+
const updateCell: TableUndoAction = {
16+
type: 'update-cell',
17+
rowId: 'r1',
18+
columnName: 'a',
19+
previousValue: 1,
20+
newValue: 2,
21+
}
22+
23+
describe('pruneLayoutActions', () => {
24+
beforeEach(() => {
25+
useTableUndoStore.getState().clear(TABLE)
26+
})
27+
28+
it('drops a layout action recorded under a different view', () => {
29+
const store = useTableUndoStore.getState()
30+
store.push(TABLE, reorder, 'view-a')
31+
32+
store.pruneLayoutActions(TABLE, 'view-b')
33+
34+
expect(useTableUndoStore.getState().stacks[TABLE]?.undo).toHaveLength(0)
35+
})
36+
37+
it('keeps a layout action when the owning view is still active', () => {
38+
const store = useTableUndoStore.getState()
39+
store.push(TABLE, reorder, 'view-a')
40+
41+
store.pruneLayoutActions(TABLE, 'view-a')
42+
43+
expect(useTableUndoStore.getState().stacks[TABLE]?.undo).toHaveLength(1)
44+
})
45+
46+
it('keeps row actions across a view switch — rows are table-scoped', () => {
47+
const store = useTableUndoStore.getState()
48+
store.push(TABLE, updateCell, 'view-a')
49+
50+
store.pruneLayoutActions(TABLE, 'view-b')
51+
52+
const undo = useTableUndoStore.getState().stacks[TABLE]?.undo
53+
expect(undo).toHaveLength(1)
54+
expect(undo?.[0].action.type).toBe('update-cell')
55+
})
56+
57+
it('prunes the redo stack too, so redo cannot replay into the wrong view', () => {
58+
const store = useTableUndoStore.getState()
59+
store.push(TABLE, reorder, 'view-a')
60+
store.popUndo(TABLE)
61+
expect(useTableUndoStore.getState().stacks[TABLE]?.redo).toHaveLength(1)
62+
63+
store.pruneLayoutActions(TABLE, 'view-b')
64+
65+
expect(useTableUndoStore.getState().stacks[TABLE]?.redo).toHaveLength(0)
66+
})
67+
68+
it('treats All (null) as its own owner', () => {
69+
const store = useTableUndoStore.getState()
70+
store.push(TABLE, reorder, null)
71+
72+
store.pruneLayoutActions(TABLE, 'view-a')
73+
expect(useTableUndoStore.getState().stacks[TABLE]?.undo).toHaveLength(0)
74+
})
75+
})

apps/sim/stores/table/store.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { generateShortId } from '@sim/utils/id'
77
import { create } from 'zustand'
88
import { devtools } from 'zustand/middleware'
99
import type { TableUndoAction, TableUndoStacks, TableUndoState, UndoEntry } from './types'
10+
import { LAYOUT_UNDO_ACTIONS } from './types'
1011

1112
const STACK_CAPACITY = 100
1213
const EMPTY_STACKS: TableUndoStacks = { undo: [], redo: [] }
@@ -99,10 +100,15 @@ export const useTableUndoStore = create<TableUndoState>()(
99100
(set, get) => ({
100101
stacks: {},
101102

102-
push: (tableId: string, action: TableUndoAction) => {
103+
push: (tableId: string, action: TableUndoAction, viewId: string | null) => {
103104
if (undoRedoInProgress) return
104105

105-
const entry: UndoEntry = { id: generateShortId(), action, timestamp: Date.now() }
106+
const entry: UndoEntry = {
107+
id: generateShortId(),
108+
action,
109+
timestamp: Date.now(),
110+
viewId,
111+
}
106112

107113
set((state) => {
108114
const current = state.stacks[tableId] ?? EMPTY_STACKS
@@ -182,6 +188,17 @@ export const useTableUndoStore = create<TableUndoState>()(
182188
})
183189
},
184190

191+
pruneLayoutActions: (tableId: string, viewId: string | null) => {
192+
const current = get().stacks[tableId]
193+
if (!current) return
194+
const owned = (entry: UndoEntry) =>
195+
!LAYOUT_UNDO_ACTIONS.has(entry.action.type) || entry.viewId === viewId
196+
const undo = current.undo.filter(owned)
197+
const redo = current.redo.filter(owned)
198+
if (undo.length === current.undo.length && redo.length === current.redo.length) return
199+
set((state) => ({ stacks: { ...state.stacks, [tableId]: { undo, redo } } }))
200+
},
201+
185202
clear: (tableId: string) => {
186203
set((state) => {
187204
const { [tableId]: _, ...rest } = state.stacks

apps/sim/stores/table/types.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,19 +87,41 @@ export interface UndoEntry {
8787
id: string
8888
action: TableUndoAction
8989
timestamp: number
90+
/**
91+
* Active view when the action was recorded — `null` for "All" or when views
92+
* are disabled. Layout is view-owned, so a layout action is only meaningful
93+
* against the view that owned it; see {@link LAYOUT_UNDO_ACTIONS}.
94+
*/
95+
viewId: string | null
9096
}
9197

98+
/**
99+
* Action types whose undo writes column layout (order, widths, pinning). These
100+
* are scoped to the view that was active when they were recorded; every other
101+
* action type operates on rows or the schema and is table-scoped.
102+
*/
103+
export const LAYOUT_UNDO_ACTIONS = new Set<TableUndoAction['type']>([
104+
'create-column',
105+
'delete-column',
106+
'reorder-columns',
107+
])
108+
92109
export interface TableUndoStacks {
93110
undo: UndoEntry[]
94111
redo: UndoEntry[]
95112
}
96113

97114
export interface TableUndoState {
98115
stacks: Record<string, TableUndoStacks>
99-
push: (tableId: string, action: TableUndoAction) => void
116+
push: (tableId: string, action: TableUndoAction, viewId: string | null) => void
100117
popUndo: (tableId: string) => UndoEntry | null
101118
popRedo: (tableId: string) => UndoEntry | null
102119
patchRedoRowId: (tableId: string, oldRowId: string, newRowId: string) => void
103120
patchUndoRowId: (tableId: string, oldRowId: string, newRowId: string) => void
104121
clear: (tableId: string) => void
122+
/**
123+
* Drops layout actions recorded under a different view. Called on every view
124+
* switch so undo can never write one view's layout into another.
125+
*/
126+
pruneLayoutActions: (tableId: string, viewId: string | null) => void
105127
}

0 commit comments

Comments
 (0)