Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.8k
improvement(tables): announce locks on open instead of a header chip#5979
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
7e3a19e7ee27fef3d8588cad3600260314a072f658d2d2156b505ef6File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| 'use client' | ||
| import { useCallback, useMemo, useReducer, useRef, useState } from 'react' | ||
| import { useCallback, useEffect, useMemo, useReducer, useRef, useState } from 'react' | ||
| import { Chip, ChipConfirmModal, toast } from '@sim/emcn' | ||
| import { Download, Lock, Pencil, Table as TableIcon, Trash, Upload } from '@sim/emcn/icons' | ||
| import { createLogger } from '@sim/logger' | ||
| @@ -61,12 +61,7 @@ import { | ||
| import { COLUMN_SIDEBAR_WIDTH } from './components/table-grid/constants' | ||
| import { COLUMN_TYPE_ICONS } from './components/table-grid/headers' | ||
| import { useTable, useTableEventStream } from './hooks' | ||
| import { | ||
| type BlockedTableAction, | ||
| describeBlockedAction, | ||
| describeLocks, | ||
| lockedNouns, | ||
| } from './lock-copy' | ||
| import { type BlockedTableAction, describeBlockedAction, lockedNouns } from './lock-copy' | ||
| import { | ||
| DEFAULT_TABLE_DETAIL_SORT_DIRECTION, | ||
| tableDetailParsers, | ||
| @@ -627,7 +622,10 @@ export function Table({ | ||
| if (!tableData) return | ||
| if (blockedToastIdRef.current) toast.dismiss(blockedToastIdRef.current) | ||
| const { title, text } = describeBlockedAction(action, tableData.locks) | ||
| blockedToastIdRef.current = toast.warning(title, { | ||
| // 'status' is the on-open announcement — nothing was refused, so it reads | ||
| // as information rather than a warning. | ||
| const notify = action === 'status' ? toast.info : toast.warning | ||
| blockedToastIdRef.current = notify(title, { | ||
| description: text, | ||
| ...(canOpenLockSettings | ||
| ? { | ||
| @@ -641,23 +639,48 @@ export function Table({ | ||
| [tableData, canOpenLockSettings] | ||
| ) | ||
| // Announce the lock state once per table on open. Unlike the re-rendering | ||
| // permission gates, this fires once and can't self-correct, so it waits for | ||
| // `canAdmin` to settle instead of treating loading as permitted. | ||
| const announcedLockTableIdRef = useRef<string | null>(null) | ||
| useEffect(() => { | ||
| if (!tableData || userPermissions.isLoading) return | ||
| if (announcedLockTableIdRef.current === tableData.id) return | ||
| announcedLockTableIdRef.current = tableData.id | ||
| if (lockedNouns(tableData.locks).length === 0) return | ||
| showBlockedToast('status') | ||
| }, [tableData, userPermissions.isLoading, showBlockedToast]) | ||
| // A notice must not outlive the table it describes — its action targets | ||
| // whichever table is current. Keyed on `tableId` so an embedded swap that | ||
| // changes the prop without a route change is covered too. Leaving ends the | ||
| // visit, so the latch resets and coming back announces again. | ||
| useEffect( | ||
| () => () => { | ||
| announcedLockTableIdRef.current = null | ||
| if (!blockedToastIdRef.current) return | ||
| toast.dismiss(blockedToastIdRef.current) | ||
| blockedToastIdRef.current = null | ||
| }, | ||
| [tableId] | ||
| ) | ||
cursor[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. cursor[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // A toast's action is captured when it is created, so a viewer who loses | ||
| // admin access mid-toast would keep a Lock settings button that opens | ||
| // nothing. Dismiss on that transition only — a viewer who never had access | ||
| // has a legitimate action-less notice that must survive. | ||
| const couldOpenLockSettingsRef = useRef(canOpenLockSettings) | ||
| useEffect(() => { | ||
| const lostAccess = couldOpenLockSettingsRef.current && !canOpenLockSettings | ||
| couldOpenLockSettingsRef.current = canOpenLockSettings | ||
| if (!lostAccess || !blockedToastIdRef.current) return | ||
| toast.dismiss(blockedToastIdRef.current) | ||
| blockedToastIdRef.current = null | ||
| }, [canOpenLockSettings]) | ||
TheodoreSpeaks marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. TheodoreSpeaks marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| const headerActions = useMemo(() => { | ||
| if (!tableData) return undefined | ||
| // Header space is for state, not for settings: the chip appears only once | ||
| // something is actually locked, and names the mode so it reads at a glance. | ||
| // Reaching the panel on an unlocked table is the dropdown's job. | ||
| const anyLocked = lockedNouns(tableData.locks).length > 0 | ||
| return [ | ||
| ...(anyLocked | ||
| ? [ | ||
| { | ||
| label: describeLocks(tableData.locks).name, | ||
| icon: Lock, | ||
| onClick: () => | ||
| userPermissions.canAdmin ? setShowLockSettings(true) : showBlockedToast('status'), | ||
| }, | ||
| ] | ||
| : []), | ||
| { | ||
| label: 'Import CSV', | ||
| icon: Upload, | ||
| @@ -673,14 +696,7 @@ export function Table({ | ||
| disabled: tableData.rowCount === 0, | ||
| }, | ||
| ] | ||
| }, [ | ||
| tableData, | ||
| userPermissions.canEdit, | ||
| userPermissions.canAdmin, | ||
| handleExportCsv, | ||
| onRequestImportCsv, | ||
| showBlockedToast, | ||
| ]) | ||
| }, [tableData, userPermissions.canEdit, handleExportCsv, onRequestImportCsv]) | ||
| // Adding a column is a schema change. The trigger stays visible when the | ||
| // table is schema-locked and explains itself instead of disappearing. | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.