Skip to content

Commit e2acf9e

Browse files
fix(tables): read layout from the grid instead of mirroring it
The wrapper kept two shadow copies of the grid's column layout — liveLayoutRef and pendingLayoutRef — and all three of this round's findings were that mirror going stale: - liveLayoutRef was only cleared on activeView.id change, so widths buffered before the views query settled survived into All, where layout writes bypass the mirror entirely. Both create paths spread it last, so a saved view stored those snapped-back widths. - currentViewConfig memoized a spread of that ref, and mutating a ref doesn't re-run a memo, so Save as view sent the pre-gesture layout. - The flush effect keyed on activeView, but adoption writes the view id through the URL, so for one render the query had settled while activeView was still null — flushing to All in exactly the case that had to drop. The grid owns this state, so it now publishes a reader through a sink ref and the wrapper asks at the moment it needs a value. Nothing to keep in sync, so nothing to go stale. Only whether an unowned change happened is tracked, and the resolve effect — which is what actually picks the owner — decides to persist or drop.
1 parent b614085 commit e2acf9e

2 files changed

Lines changed: 64 additions & 50 deletions

File tree

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,12 @@ interface TableGridProps {
247247
* `columnWidths` / `columnOrder` keys). The wrapper just forwards the call.
248248
*/
249249
columnRenameSinkRef: React.MutableRefObject<((oldName: string, newName: string) => void) | null>
250+
/**
251+
* Ref the grid populates with a reader for its CURRENT column layout. The grid
252+
* owns this state, so the wrapper asks for it when creating a view rather than
253+
* mirroring every patch — a mirror goes stale the moment a write bypasses it.
254+
*/
255+
layoutSnapshotSinkRef?: React.MutableRefObject<(() => TableMetadata) | null>
250256
/**
251257
* Ref the grid populates with its post-row-delete cleanup (push undo,
252258
* clear selection). The wrapper invokes after the row-delete modal's
@@ -383,6 +389,7 @@ export function TableGrid({
383389
viewLayoutKey = null,
384390
onPersistLayout,
385391
columnRenameSinkRef,
392+
layoutSnapshotSinkRef,
386393
afterDeleteRowsSinkRef,
387394
afterDeleteAllSinkRef,
388395
confirmDeleteColumnsSinkRef,
@@ -666,6 +673,14 @@ export function TableGrid({
666673
// the grid. Reads through refs, so identity stability isn't required.
667674
columnRenameSinkRef.current = handleColumnRename
668675

676+
if (layoutSnapshotSinkRef) {
677+
layoutSnapshotSinkRef.current = () => ({
678+
columnWidths: columnWidthsRef.current,
679+
...(columnOrderRef.current ? { columnOrder: columnOrderRef.current } : {}),
680+
pinnedColumns: pinnedColumnsRef.current,
681+
})
682+
}
683+
669684
function getColumnWidths() {
670685
return columnWidthsRef.current
671686
}

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

Lines changed: 49 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,42 @@ export function Table({
439439
[setTableParams]
440440
)
441441

442+
/** Reader for the grid's CURRENT column layout, populated by the grid itself.
443+
* The grid owns widths/order/pinning, so the wrapper asks at the moment it
444+
* needs them instead of mirroring every patch — a mirror only stays right
445+
* while every write flows through it, and layout writes bypass it whenever
446+
* All is active. */
447+
const layoutSnapshotRef = useRef<(() => TableMetadata) | null>(null)
448+
const readLayout = useCallback((): TableMetadata => layoutSnapshotRef.current?.() ?? {}, [])
449+
450+
/** Whether the user changed layout before the views query settled, when there
451+
* was no owner to write to. What changed isn't recorded — the grid still holds
452+
* it — only that something did, so a settle to All knows to persist it. */
453+
const layoutTouchedWhileUnownedRef = useRef(false)
454+
455+
/**
456+
* Resolves that pending layout once the resolve effect has picked an owner.
457+
*
458+
* Settling on All re-seeds nothing — `viewLayoutKey` never changed — so the
459+
* user's resize is still on screen and has to be persisted or it silently
460+
* disappears on refresh. Adopting a view instead re-seeds the grid from that
461+
* view's config, which already replaced the gesture on screen, so it is dropped.
462+
*
463+
* Called from the resolve effect rather than keyed on `activeView`: adoption
464+
* writes the view id through the URL, so for one render the query has settled
465+
* while `activeView` is still null, and an effect would flush to All in exactly
466+
* the case that must drop.
467+
*/
468+
const resolvePendingLayout = useCallback(
469+
(adoptedView: boolean) => {
470+
if (!layoutTouchedWhileUnownedRef.current) return
471+
layoutTouchedWhileUnownedRef.current = false
472+
if (adoptedView || !userPermissions.canEdit) return
473+
updateMetadataMutation.mutate(readLayout())
474+
},
475+
[userPermissions.canEdit, readLayout]
476+
)
477+
442478
/** What the user has already set by hand, for the first-resolve `keep`. */
443479
const localWork = () => ({
444480
sort: sortColumn !== null,
@@ -481,23 +517,27 @@ export function Table({
481517
seededViewIdRef.current = defaultView.id
482518
setTableParams({ view: defaultView.id })
483519
applyViewConfig(defaultView.config, keep)
520+
resolvePendingLayout(true)
484521
return
485522
}
486523
// No view to adopt. Deliberately does NOT apply an empty config — that
487524
// would clear a deep-linked `?sort=` on mount. Inherited params are the
488525
// exception: nothing about them refers to this table, so they're cleared.
489526
seededViewIdRef.current = null
490527
if (inheritedParams) setTableParams({ view: ALL_VIEW_PARAM, sort: null, dir: null })
528+
resolvePendingLayout(false)
491529
return
492530
}
493531
if (activeViewId === ALL_VIEW_PARAM) {
494532
seededViewIdRef.current = null
533+
resolvePendingLayout(false)
495534
return
496535
}
497536
// A `?view=` that resolves to nothing (deleted view, stale bookmark) falls
498537
// back to "All" without touching state, for the same reason. An explicit
499538
// `?sort=` alongside `?view=` also wins over the view's stored sort.
500539
seededViewIdRef.current = activeView?.id ?? null
540+
resolvePendingLayout(activeView !== null)
501541
if (activeView) {
502542
applyViewConfig(activeView.config, localWork())
503543
} else {
@@ -541,6 +581,7 @@ export function Table({
541581
sortColumn,
542582
applyViewConfig,
543583
setTableParams,
584+
resolvePendingLayout,
544585
])
545586

546587
/**
@@ -576,48 +617,9 @@ export function Table({
576617
* order / pins the grid is rendering (they live in the table's shared metadata
577618
* until a view owns them) instead of creating a layout-less view that then
578619
* resets the grid. Updates never send this — they send a merge patch. */
579-
/** Last layout the grid reported, patch by patch. View layout writes have no
580-
* optimistic cache update, so `activeView.config` lags an in-flight resize —
581-
* this is what a new view copies so the grid can't snap back.
582-
*
583-
* Declared above the consumers that read it during render, and reset on every
584-
* view change: it holds patches for ONE view, so carrying them across a switch
585-
* would let a new view inherit the previous view's widths/order/pins. */
586-
const liveLayoutRef = useRef<TableMetadata>({})
587-
588-
useEffect(() => {
589-
liveLayoutRef.current = {}
590-
}, [activeView?.id])
591-
592-
/** Layout captured before the views query settled, when no owner was known. */
593-
const pendingLayoutRef = useRef<TableMetadata>({})
594-
595-
/**
596-
* Resolves that buffer once the owner is known.
597-
*
598-
* Settling on All re-seeds nothing — `viewLayoutKey` never changed — so the
599-
* user's resize is still on screen and has to be persisted or it silently
600-
* disappears on refresh. Adopting a view instead re-seeds the grid from that
601-
* view's config, which already replaced the gesture on screen, so the buffer is
602-
* dropped to match what the user is looking at.
603-
*/
604-
useEffect(() => {
605-
if (viewOwnerUnknown) return
606-
const pending = pendingLayoutRef.current
607-
pendingLayoutRef.current = {}
608-
if (activeView || !userPermissions.canEdit) return
609-
if (Object.keys(pending).length === 0) return
610-
updateMetadataMutation.mutate(pending)
611-
// `.mutate` is stable in TanStack Query v5, so it stays out of the deps.
612-
}, [viewOwnerUnknown, activeView, userPermissions.canEdit])
613-
614620
const currentViewConfig = useMemo<TableViewConfig>(
615621
() => ({
616-
// `liveLayoutRef` after the cached config for the same reason the blank-view
617-
// payload does it: a resize/reorder/pin still in flight would otherwise be
618-
// captured at its pre-drag value and snap back when the view is selected.
619622
...(activeView?.config ?? tableData?.metadata),
620-
...liveLayoutRef.current,
621623
filter: effectiveFilter,
622624
sort: sortQuery,
623625
hiddenColumns: effectiveHiddenColumns,
@@ -683,14 +685,15 @@ export function Table({
683685
* suppressed, because the owner isn't known yet. */
684686
const handlePersistLayout = useCallback(
685687
(patch: TableMetadata) => {
686-
liveLayoutRef.current = { ...liveLayoutRef.current, ...patch }
687688
// The resize grip and drag handles stay live for read-only members, so
688689
// without this a resize fires a write-gated PATCH and an error toast. Local
689690
// layout still updates — only the persist is suppressed.
690691
if (!userPermissions.canEdit) return
691-
// Owner not known yet — hold the patch until the views query settles.
692+
// Owner not known yet. The grid keeps the layout either way, so only the
693+
// fact of the change is recorded; `resolvePendingLayout` reads the live
694+
// value once an owner exists.
692695
if (viewOwnerUnknown) {
693-
pendingLayoutRef.current = { ...pendingLayoutRef.current, ...patch }
696+
layoutTouchedWhileUnownedRef.current = true
694697
return
695698
}
696699
if (!activeView) return
@@ -741,18 +744,13 @@ export function Table({
741744
const blank = viewModal?.blank === true
742745
const config: TableViewConfig = blank
743746
? {
744-
// `liveLayoutRef` last, so a resize/reorder/pin still in flight wins over
745-
// the cached copy — otherwise the new view stores the pre-drag layout and
746-
// the grid snaps back to it on selection. (With no active view the grid
747-
// writes table metadata directly, which `useUpdateTableMetadata` updates
748-
// optimistically, so that branch is already current.)
749747
...(activeView?.config ?? tableData?.metadata),
750-
...liveLayoutRef.current,
748+
...readLayout(),
751749
filter: null,
752750
sort: null,
753751
hiddenColumns: [],
754752
}
755-
: currentViewConfig
753+
: { ...currentViewConfig, ...readLayout() }
756754
createViewMutation.mutate(
757755
{ name, config },
758756
{
@@ -1385,12 +1383,13 @@ export function Table({
13851383
onPersistLayout={
13861384
// While the views query is in flight the layout owner is unknown, so
13871385
// the sink is bound anyway: `handlePersistLayout` buffers into
1388-
// `liveLayoutRef` and suppresses the write. Leaving it unset would fall
1386+
// records the change and suppresses the write. Leaving it unset would fall
13891387
// through to the table's shared metadata and corrupt All's layout for a
13901388
// table that is about to adopt a view.
13911389
viewOwnerUnknown || activeView ? handlePersistLayout : undefined
13921390
}
13931391
columnRenameSinkRef={columnRenameSinkRef}
1392+
layoutSnapshotSinkRef={layoutSnapshotRef}
13941393
afterDeleteRowsSinkRef={afterDeleteRowsSinkRef}
13951394
afterDeleteAllSinkRef={afterDeleteAllSinkRef}
13961395
confirmDeleteColumnsSinkRef={confirmDeleteColumnsSinkRef}

0 commit comments

Comments
 (0)