Skip to content

Commit e5ae445

Browse files
improvement(tables): announce locks on open instead of a header chip (#5979)
* improvement(tables): announce locks on open instead of a header chip Drop the lock entry from the table header actions. A locked table now announces itself once on open via an info toast carrying the Lock settings action for admins; the breadcrumb dropdown remains the permanent entry point. * fix(tables): wait for permissions before announcing table locks The lock announcement fires once per table, so a canAdmin that is still false because permissions have not resolved permanently drops the toast's Lock settings action. Arm the one-shot on the permissions decision rather than on table resolution. * fix(tables): keep the lock announcement from being swept on navigation The toast provider clears route-scoped toasts in a pathname effect that runs after child effects, so an announcement fired on a warm-cache navigation was added and removed in the same commit. Opt these toasts out of the sweep and dismiss them on unmount so they still don't trail the user. * fix(tables): drop the lock notice when its action stops being valid A toast's action is captured at creation, so a viewer whose admin access is revoked while the announcement is on screen kept a Lock settings button that opened nothing. Dismiss the notice when the action is no longer available. * fix(tables): only drop the lock notice when access is actually lost The dismiss effect treated "has no access" the same as "just lost access", so on a warm-cache mount — where the announcement and the dismiss both run in the mount commit — a non-admin's notice was torn down the moment it was created. Dismiss on the true-to-false transition only. * fix(tables): clear the lock notice when switching tables Switching tables reuses this component rather than remounting it, and the notice is exempt from the provider's route sweep, so a locked table's announcement stayed on screen over the next table — where its action would have opened that table's lock settings instead. Key the cleanup on tableId. * fix(emcn): sweep route-scoped toasts during render, not after child effects The provider cleared route-scoped toasts from a pathname effect. Effects run child-first, so it also swept toasts the newly rendered route had just raised: any toast added from a child's mount effect — which is what happens whenever that child's data is already cached — was appended and filtered out in the same commit, before it painted. Move the sweep into render, where it runs before children render, so only toasts predating the navigation are cleared. Timers and heights were already reconciled by effects keyed off `toasts`, so this stays a pure state update. Drops the persistAcrossRoutes workaround the table lock notice needed to dodge the bug; its tableId-keyed cleanup stays, covering embedded swaps that change the prop without a route change. * fix(tables): reset the announce latch when leaving a table The tableId cleanup dismissed the notice on departure but left the latch holding that table's id, so returning before another table announced — a quick there-and-back, or a second table that never loaded — found the latch matching and stayed silent. Leaving ends the visit, so clear the latch with it.
1 parent b148a52 commit e5ae445

3 files changed

Lines changed: 73 additions & 60 deletions

File tree

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
2-
* Single source of truth for lock vocabulary shared by the lock settings modal,
3-
* the blocked-action toast, and the table header chip. Kept out of
2+
* Single source of truth for lock vocabulary shared by the lock settings modal
3+
* and the lock toasts (the on-open announcement and blocked actions). Kept out of
44
* `lib/table/mutation-locks.ts` — that module is server-tainted (importing it
55
* from a client component pulls `next/headers` into the browser bundle).
66
*/
@@ -76,8 +76,8 @@ export function describeLocks(locks: TableLocks): { name: string; detail: string
7676

7777
/**
7878
* Why a locked-table notice was raised. `'status'` is the informational case
79-
* (a non-admin clicking the header lock chip); the rest are actions the user
80-
* just tried and couldn't do.
79+
* (the announcement shown once when a locked table is opened); the rest are
80+
* actions the user just tried and couldn't do.
8181
*/
8282
export type BlockedTableAction = 'add-row' | 'add-column' | 'delete-column' | 'edit-cell' | 'status'
8383

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

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

3-
import { useCallback, useMemo, useReducer, useRef, useState } from 'react'
3+
import { useCallback, useEffect, useMemo, useReducer, useRef, useState } from 'react'
44
import { Chip, ChipConfirmModal, toast } from '@sim/emcn'
55
import { Download, Lock, Pencil, Table as TableIcon, Trash, Upload } from '@sim/emcn/icons'
66
import { createLogger } from '@sim/logger'
@@ -61,12 +61,7 @@ import {
6161
import { COLUMN_SIDEBAR_WIDTH } from './components/table-grid/constants'
6262
import { COLUMN_TYPE_ICONS } from './components/table-grid/headers'
6363
import { useTable, useTableEventStream } from './hooks'
64-
import {
65-
type BlockedTableAction,
66-
describeBlockedAction,
67-
describeLocks,
68-
lockedNouns,
69-
} from './lock-copy'
64+
import { type BlockedTableAction, describeBlockedAction, lockedNouns } from './lock-copy'
7065
import {
7166
DEFAULT_TABLE_DETAIL_SORT_DIRECTION,
7267
tableDetailParsers,
@@ -627,7 +622,10 @@ export function Table({
627622
if (!tableData) return
628623
if (blockedToastIdRef.current) toast.dismiss(blockedToastIdRef.current)
629624
const { title, text } = describeBlockedAction(action, tableData.locks)
630-
blockedToastIdRef.current = toast.warning(title, {
625+
// 'status' is the on-open announcement — nothing was refused, so it reads
626+
// as information rather than a warning.
627+
const notify = action === 'status' ? toast.info : toast.warning
628+
blockedToastIdRef.current = notify(title, {
631629
description: text,
632630
...(canOpenLockSettings
633631
? {
@@ -641,23 +639,48 @@ export function Table({
641639
[tableData, canOpenLockSettings]
642640
)
643641

642+
// Announce the lock state once per table on open. Unlike the re-rendering
643+
// permission gates, this fires once and can't self-correct, so it waits for
644+
// `canAdmin` to settle instead of treating loading as permitted.
645+
const announcedLockTableIdRef = useRef<string | null>(null)
646+
useEffect(() => {
647+
if (!tableData || userPermissions.isLoading) return
648+
if (announcedLockTableIdRef.current === tableData.id) return
649+
announcedLockTableIdRef.current = tableData.id
650+
if (lockedNouns(tableData.locks).length === 0) return
651+
showBlockedToast('status')
652+
}, [tableData, userPermissions.isLoading, showBlockedToast])
653+
654+
// A notice must not outlive the table it describes — its action targets
655+
// whichever table is current. Keyed on `tableId` so an embedded swap that
656+
// changes the prop without a route change is covered too. Leaving ends the
657+
// visit, so the latch resets and coming back announces again.
658+
useEffect(
659+
() => () => {
660+
announcedLockTableIdRef.current = null
661+
if (!blockedToastIdRef.current) return
662+
toast.dismiss(blockedToastIdRef.current)
663+
blockedToastIdRef.current = null
664+
},
665+
[tableId]
666+
)
667+
668+
// A toast's action is captured when it is created, so a viewer who loses
669+
// admin access mid-toast would keep a Lock settings button that opens
670+
// nothing. Dismiss on that transition only — a viewer who never had access
671+
// has a legitimate action-less notice that must survive.
672+
const couldOpenLockSettingsRef = useRef(canOpenLockSettings)
673+
useEffect(() => {
674+
const lostAccess = couldOpenLockSettingsRef.current && !canOpenLockSettings
675+
couldOpenLockSettingsRef.current = canOpenLockSettings
676+
if (!lostAccess || !blockedToastIdRef.current) return
677+
toast.dismiss(blockedToastIdRef.current)
678+
blockedToastIdRef.current = null
679+
}, [canOpenLockSettings])
680+
644681
const headerActions = useMemo(() => {
645682
if (!tableData) return undefined
646-
// Header space is for state, not for settings: the chip appears only once
647-
// something is actually locked, and names the mode so it reads at a glance.
648-
// Reaching the panel on an unlocked table is the dropdown's job.
649-
const anyLocked = lockedNouns(tableData.locks).length > 0
650683
return [
651-
...(anyLocked
652-
? [
653-
{
654-
label: describeLocks(tableData.locks).name,
655-
icon: Lock,
656-
onClick: () =>
657-
userPermissions.canAdmin ? setShowLockSettings(true) : showBlockedToast('status'),
658-
},
659-
]
660-
: []),
661684
{
662685
label: 'Import CSV',
663686
icon: Upload,
@@ -673,14 +696,7 @@ export function Table({
673696
disabled: tableData.rowCount === 0,
674697
},
675698
]
676-
}, [
677-
tableData,
678-
userPermissions.canEdit,
679-
userPermissions.canAdmin,
680-
handleExportCsv,
681-
onRequestImportCsv,
682-
showBlockedToast,
683-
])
699+
}, [tableData, userPermissions.canEdit, handleExportCsv, onRequestImportCsv])
684700

685701
// Adding a column is a schema change. The trigger stays visible when the
686702
// table is schema-locked and explains itself instead of disappearing.

packages/emcn/src/components/toast/toast.tsx

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,29 @@ export function ToastProvider({ children }: { children?: ReactNode }) {
396396
const [mounted, setMounted] = useState(false)
397397
const timersRef = useRef(new Map<string, ReturnType<typeof setTimeout>>())
398398

399+
/**
400+
* Clear the previous route's toasts when the route changes. Toasts flagged
401+
* `persistAcrossRoutes` — global, ongoing-state notifications like the
402+
* connection status — survive; page-scoped ones do not.
403+
*
404+
* Done during render rather than in an effect. Effects run child-first, so an
405+
* effect here would also sweep toasts the newly rendered route just raised: a
406+
* toast added from a child's mount effect — which is what happens whenever
407+
* that child's data is already cached — would be appended and filtered out in
408+
* the same commit, before it ever painted. Adjusting state during render runs
409+
* before children render, so only toasts predating the navigation are cleared.
410+
*
411+
* Stays a pure state update: orphaned timers and measured heights are already
412+
* reconciled by the effects below, which key off `toasts`.
413+
*/
414+
const [sweptPathname, setSweptPathname] = useState(pathname)
415+
if (pathname !== sweptPathname) {
416+
setSweptPathname(pathname)
417+
setToasts((prev) =>
418+
prev.some((t) => !t.persistAcrossRoutes) ? prev.filter((t) => t.persistAcrossRoutes) : prev
419+
)
420+
}
421+
399422
useEffect(() => {
400423
setMounted(true)
401424
}, [])
@@ -459,27 +482,6 @@ export function ToastProvider({ children }: { children?: ReactNode }) {
459482
setHeights({})
460483
}, [])
461484

462-
/**
463-
* Clear only route-scoped toasts. Toasts flagged `persistAcrossRoutes` —
464-
* global, ongoing-state notifications like the connection status — survive,
465-
* everything else (page-scoped notifications) is cleared on navigation.
466-
*/
467-
const dismissRouteScopedToasts = useCallback(() => {
468-
setToasts((prev) => {
469-
const kept = prev.filter((t) => t.persistAcrossRoutes)
470-
if (kept.length === prev.length) return prev
471-
for (const t of prev) {
472-
if (t.persistAcrossRoutes) continue
473-
const timer = timersRef.current.get(t.id)
474-
if (timer) {
475-
clearTimeout(timer)
476-
timersRef.current.delete(t.id)
477-
}
478-
}
479-
return kept
480-
})
481-
}, [])
482-
483485
const measureToast = useCallback((id: string, height: number) => {
484486
setHeights((prev) => (prev[id] === height ? prev : { ...prev, [id]: height }))
485487
}, [])
@@ -536,11 +538,6 @@ export function ToastProvider({ children }: { children?: ReactNode }) {
536538
}
537539
}, [])
538540

539-
/** On navigation, clear route-scoped toasts so they don't trail the user; `persistAcrossRoutes` toasts survive. */
540-
useEffect(() => {
541-
dismissRouteScopedToasts()
542-
}, [pathname, dismissRouteScopedToasts])
543-
544541
/** Held in a ref (seeded once from the stable `addToast`) so the module-level `toast` binds to the live provider. */
545542
const toastFn = useRef<ToastFn>(createToastFn(addToast))
546543

0 commit comments

Comments
 (0)