Skip to content

Commit daefc28

Browse files
committed
fix(tables): preserve legacy layout through view adoption
1 parent c07a07d commit daefc28

3 files changed

Lines changed: 76 additions & 44 deletions

File tree

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

Lines changed: 40 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { Chip, ChipConfirmModal, toast } from '@sim/emcn'
55
import { Download, Lock, Pencil, Trash, Upload } from '@sim/emcn/icons'
66
import { createLogger } from '@sim/logger'
77
import { getErrorMessage } from '@sim/utils/errors'
8+
import { isEqual } from 'es-toolkit'
89
import { useParams, useRouter } from 'next/navigation'
910
import { useQueryStates } from 'nuqs'
1011
import { usePostHog } from 'posthog-js/react'
@@ -41,6 +42,7 @@ import { useRegisterGlobalCommands } from '@/app/workspace/[workspaceId]/provide
4142
import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider'
4243
import {
4344
getTableViewRevision,
45+
resolveTableViewConfig,
4446
resolveTableViewSelection,
4547
shouldApplyTableViewRevision,
4648
type TableViewRevision,
@@ -379,6 +381,7 @@ export function Table({
379381
tableId,
380382
queryOptions,
381383
})
384+
const tableAvailable = tableData !== undefined
382385
const createViewMutation = useCreateTableView({ workspaceId, tableId })
383386
const updateViewMutation = useUpdateTableView({ workspaceId, tableId })
384387
const updateMetadataMutation = useUpdateTableMetadata({ workspaceId, tableId })
@@ -387,6 +390,10 @@ export function Table({
387390
/** Resolve the default synchronously so the grid, autosave owner, and menu all
388391
* agree before the URL effect records the adopted view id. */
389392
const { selectedView, defaultView, activeView } = resolveTableViewSelection(views, activeViewId)
393+
const activeViewConfig = useMemo(
394+
() => resolveTableViewConfig(tableData?.metadata, activeView?.config ?? null),
395+
[tableData?.metadata, activeView?.config]
396+
)
390397

391398
const [viewModal, setViewModal] = useState<ViewModalState>(null)
392399
/** Which persisted view revision last seeded the local filter/sort/hidden state.
@@ -424,7 +431,7 @@ export function Table({
424431
* open panel would wipe keystrokes typed since the flush and steal focus.
425432
*/
426433
const replaceFilter = useCallback((next: TablePredicate | null) => {
427-
if (JSON.stringify(next) === JSON.stringify(filterRef.current)) return
434+
if (isEqual(next, filterRef.current)) return
428435
setFilter(next)
429436
setFilterSeed((seed) => seed + 1)
430437
}, [])
@@ -458,12 +465,8 @@ export function Table({
458465
const layoutSnapshotRef = useRef<(() => TableMetadata) | null>(null)
459466
const readLayout = useCallback((): TableMetadata => layoutSnapshotRef.current?.() ?? {}, [])
460467

461-
/** Layout KEYS the user changed before the views query settled, when there was
462-
* no owner to write to. Values aren't recorded — the grid holds them live —
463-
* but the keys are, so a settle to All persists only what was touched. A full
464-
* snapshot would also carry keys the grid hasn't seeded yet (e.g. pins while
465-
* the slower detail query is still in flight) and wipe them in metadata. */
466-
const pendingLayoutKeysRef = useRef<Set<keyof TableMetadata> | null>(null)
468+
/** Layout patch the user committed before the views query identified its owner. */
469+
const pendingLayoutPatchRef = useRef<TableMetadata | null>(null)
467470

468471
/** Whether the resolve effect has decided the initial owner — including the
469472
* terminal-error fallback to All. Until then a write that reads "All" might
@@ -473,31 +476,24 @@ export function Table({
473476
/**
474477
* Resolves that pending layout once the resolve effect has picked an owner.
475478
*
476-
* Settling on All re-seeds nothing — `viewLayoutKey` never changed — so the
477-
* user's resize is still on screen and has to be persisted or it silently
478-
* disappears on refresh. Adopting a view instead re-seeds the grid from that
479-
* view's config, which already replaced the gesture on screen, so it is dropped.
480-
*
481479
* Called from the resolve effect rather than keyed on the URL selection:
482-
* default adoption is resolved synchronously before that URL catches up, and
483-
* an independent effect could flush to All in exactly the case that must drop.
480+
* default adoption is resolved synchronously before that URL catches up.
484481
*/
485482
const resolvePendingLayout = useCallback(
486-
(adoptedView: boolean) => {
487-
const keys = pendingLayoutKeysRef.current
488-
pendingLayoutKeysRef.current = null
489-
if (!keys || keys.size === 0) return
490-
if (adoptedView || !userPermissions.canEdit) return
491-
const live = readLayout()
492-
const patch: TableMetadata = {}
493-
if (keys.has('columnWidths') && live.columnWidths) patch.columnWidths = live.columnWidths
494-
if (keys.has('columnOrder') && live.columnOrder) patch.columnOrder = live.columnOrder
495-
if (keys.has('pinnedColumns') && live.pinnedColumns) {
496-
patch.pinnedColumns = live.pinnedColumns
483+
(viewId: string | null) => {
484+
const patch = pendingLayoutPatchRef.current
485+
pendingLayoutPatchRef.current = null
486+
if (!patch || !userPermissions.canEdit) return
487+
if (viewId) {
488+
updateViewMutation.mutate(
489+
{ viewId, configPatch: patch },
490+
{ onError: (error) => toast.error(getErrorMessage(error, 'Failed to save layout')) }
491+
)
492+
return
497493
}
498-
if (Object.keys(patch).length > 0) updateMetadataMutation.mutate(patch)
494+
updateMetadataMutation.mutate(patch)
499495
},
500-
[userPermissions.canEdit, readLayout]
496+
[userPermissions.canEdit]
501497
)
502498

503499
/** What the user has already set by hand when the first view resolves. */
@@ -568,10 +564,10 @@ export function Table({
568564
// through — the list is still resolvable.
569565
if (viewsErrored && !viewsAvailable) {
570566
ownerResolvedRef.current = true
571-
resolvePendingLayout(false)
567+
resolvePendingLayout(null)
572568
return
573569
}
574-
if (!viewsAvailable) return
570+
if (!viewsAvailable || !tableAvailable) return
575571
ownerResolvedRef.current = true
576572
if (appliedViewRevisionRef.current === undefined) {
577573
// Embedded tables bind these parsers to the HOST page's URL, which the
@@ -597,8 +593,8 @@ export function Table({
597593
appliedViewRevisionRef.current = getTableViewRevision(defaultView)
598594
setTableParams({ view: defaultView.id })
599595
preserveViewState(defaultView.id, keep)
600-
applyViewConfig(defaultView.config, keep)
601-
resolvePendingLayout(true)
596+
applyViewConfig(resolveTableViewConfig(tableData?.metadata, defaultView.config), keep)
597+
resolvePendingLayout(defaultView.id)
602598
flushPendingViewConfig(defaultView.id)
603599
return
604600
}
@@ -607,28 +603,28 @@ export function Table({
607603
// exception: nothing about them refers to this table, so they're cleared.
608604
appliedViewRevisionRef.current = getTableViewRevision(null)
609605
if (inheritedParams) setTableParams({ view: ALL_VIEW_PARAM, sort: null, dir: null })
610-
resolvePendingLayout(false)
606+
resolvePendingLayout(null)
611607
return
612608
}
613609
if (activeViewId === ALL_VIEW_PARAM) {
614610
appliedViewRevisionRef.current = getTableViewRevision(null)
615-
resolvePendingLayout(false)
611+
resolvePendingLayout(null)
616612
return
617613
}
618614
// A `?view=` that resolves to nothing adopts the persisted default when
619615
// one exists; tables awaiting backfill retain the legacy All fallback.
620616
const viewToAdopt = selectedView ?? defaultView
621617
const keep = localWork()
622618
appliedViewRevisionRef.current = getTableViewRevision(viewToAdopt)
623-
resolvePendingLayout(viewToAdopt !== null)
619+
resolvePendingLayout(viewToAdopt?.id ?? null)
624620
if (selectedView) {
625621
preserveViewState(selectedView.id, keep)
626-
applyViewConfig(selectedView.config, keep)
622+
applyViewConfig(resolveTableViewConfig(tableData?.metadata, selectedView.config), keep)
627623
flushPendingViewConfig(selectedView.id)
628624
} else if (defaultView) {
629625
setTableParams({ view: defaultView.id })
630626
preserveViewState(defaultView.id, keep)
631-
applyViewConfig(defaultView.config, keep)
627+
applyViewConfig(resolveTableViewConfig(tableData?.metadata, defaultView.config), keep)
632628
flushPendingViewConfig(defaultView.id)
633629
} else {
634630
// Nothing to apply, but the URL still names a view that no longer exists.
@@ -654,7 +650,7 @@ export function Table({
654650
preservedViewStateRef.current = null
655651
appliedViewRevisionRef.current = getTableViewRevision(defaultView)
656652
setTableParams({ view: defaultView?.id ?? ALL_VIEW_PARAM })
657-
applyViewConfig(defaultView?.config ?? null)
653+
applyViewConfig(resolveTableViewConfig(tableData?.metadata, defaultView?.config ?? null))
658654
return
659655
}
660656

@@ -683,16 +679,18 @@ export function Table({
683679
pendingCreatedViewIdRef.current = null
684680
}
685681
const keep = preserved?.viewId === nextViewId ? preserved.keep : undefined
686-
applyViewConfig(activeView?.config ?? null, keep)
682+
applyViewConfig(activeViewConfig, keep)
687683
if (activeView) flushPendingViewConfig(activeView.id)
688684
}, [
689685
viewsEnabled,
690686
viewsAvailable,
691687
viewsErrored,
688+
tableAvailable,
692689
views,
693690
selectedView,
694691
defaultView,
695692
activeView,
693+
activeViewConfig,
696694
activeViewId,
697695
embedded,
698696
sortColumn,
@@ -702,6 +700,7 @@ export function Table({
702700
resolvePendingLayout,
703701
preserveViewState,
704702
flushPendingViewConfig,
703+
tableData?.metadata,
705704
])
706705

707706
/**
@@ -810,12 +809,9 @@ export function Table({
810809
return
811810
}
812811
// Owner reads "All", but the resolve effect hasn't confirmed that yet —
813-
// record the touched keys; `resolvePendingLayout` decides at settle.
812+
// retain the exact gesture so adoption can save it to the selected owner.
814813
if (!ownerResolvedRef.current) {
815-
pendingLayoutKeysRef.current ??= new Set()
816-
for (const key of Object.keys(patch) as (keyof TableMetadata)[]) {
817-
pendingLayoutKeysRef.current.add(key)
818-
}
814+
pendingLayoutPatchRef.current = { ...pendingLayoutPatchRef.current, ...patch }
819815
return
820816
}
821817
updateMetadataMutation.mutate(patch)
@@ -1555,7 +1551,7 @@ export function Table({
15551551
onSelectionChange={onSelectionChange}
15561552
queryOptions={queryOptions}
15571553
hiddenColumns={effectiveHiddenColumns}
1558-
viewLayout={activeView?.config ?? null}
1554+
viewLayout={activeViewConfig}
15591555
viewLayoutKey={activeView?.id ?? null}
15601556
// Always bound while views are enabled: the router reads the owner at
15611557
// call time (buffer / view / All-metadata), so no binding gap can send a

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/view-state.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,33 @@ import type { TableViewWire } from '@/lib/api/contracts/tables'
66
import { ALL_VIEW_PARAM } from '@/app/workspace/[workspaceId]/tables/[tableId]/search-params'
77
import {
88
getTableViewRevision,
9+
resolveTableViewConfig,
910
resolveTableViewSelection,
1011
shouldApplyTableViewRevision,
1112
} from '@/app/workspace/[workspaceId]/tables/[tableId]/view-state'
1213

14+
describe('resolveTableViewConfig', () => {
15+
it('inherits layout metadata when an ungated default view is still empty', () => {
16+
const metadata = {
17+
columnWidths: { 'column-1': 240 },
18+
columnOrder: ['column-1'],
19+
pinnedColumns: ['column-1'],
20+
hiddenColumns: ['column-2'],
21+
}
22+
23+
expect(resolveTableViewConfig(metadata, {})).toEqual(metadata)
24+
})
25+
26+
it('lets explicitly stored view fields override the metadata baseline', () => {
27+
expect(
28+
resolveTableViewConfig(
29+
{ columnWidths: { 'column-1': 240 }, pinnedColumns: ['column-1'] },
30+
{ columnWidths: { 'column-1': 180 }, pinnedColumns: [] }
31+
)
32+
).toEqual({ columnWidths: { 'column-1': 180 }, pinnedColumns: [] })
33+
})
34+
})
35+
1336
const DEFAULT_VIEW: TableViewWire = {
1437
id: 'view-default',
1538
tableId: 'table-1',

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/view-state.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { TableViewWire } from '@/lib/api/contracts/tables'
2+
import type { TableMetadata, TableViewConfig } from '@/lib/table'
23
import { ALL_VIEW_PARAM } from '@/app/workspace/[workspaceId]/tables/[tableId]/search-params'
34

45
export interface TableViewSelection {
@@ -7,6 +8,18 @@ export interface TableViewSelection {
78
activeView: TableViewWire | null
89
}
910

11+
/**
12+
* For fields shared with table metadata, a persisted view owns only what it has
13+
* stored. Missing fields inherit values written before table views were enabled.
14+
*/
15+
export function resolveTableViewConfig(
16+
metadata: TableMetadata | null | undefined,
17+
viewConfig: TableViewConfig | null
18+
): TableViewConfig | null {
19+
if (!viewConfig) return null
20+
return { ...(metadata ?? {}), ...viewConfig }
21+
}
22+
1023
/**
1124
* Resolves the persisted default synchronously when the URL has not selected a
1225
* view yet. The URL effect still records that choice, but render-time consumers

0 commit comments

Comments
 (0)