improvement(tables): announce locks on open instead of a header chip - #5979
improvement(tables): announce locks on open instead of a header chip#5979TheodoreSpeaks wants to merge 4 commits into
Conversation
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview Lock toasts use
Reviewed by Cursor Bugbot for commit cad3600. Bugbot is set up for automated code reviews on this repo. Configure here. |
Greptile SummaryThe PR replaces the table-header lock indicator with a one-time lock-status toast and now dismisses actionable lock notices when administrative access is lost.
Confidence Score: 5/5The PR appears safe to merge. The prior stale-permission action is dismissed when administrative access or online capability is lost, and the lock settings modal remains independently gated by current administrator permission, so no blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/tables/[tableId]/lock-copy.ts | Updates documentation to reflect that lock status is announced through toasts rather than a header chip. |
| apps/sim/app/workspace/[workspaceId]/tables/[tableId]/table.tsx | Moves lock-status presentation to a one-time toast and safely removes its captured settings action when administrative access becomes unavailable. |
Reviews (3): Last reviewed commit: "fix(tables): drop the lock notice when i..." | Re-trigger Greptile
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
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.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f3d8588. Configure here.
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.
|
Addressed the outstanding toast-action lifecycle concern in cad3600. The observation was correct: a toast's Rather than trying to keep a live action in a static toast, the view now drops the notice when the action stops being valid: useEffect(() => {
if (canOpenLockSettings || !blockedToastIdRef.current) return
toast.dismiss(blockedToastIdRef.current)
blockedToastIdRef.current = null
}, [canOpenLockSettings])This pairs with the unmount cleanup added in f3d8588 — the view owns its toasts' lifecycle end-to-end, which is what One direction deliberately not handled: a viewer who gains admin mid-toast keeps the buttonless notice for its remaining duration. That fails safe (the breadcrumb dropdown is the permanent entry point) and re-creating a toast under the user to add a button would be worse than leaving it. |
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cad3600. Configure here.
| if (canOpenLockSettings || !blockedToastIdRef.current) return | ||
| toast.dismiss(blockedToastIdRef.current) | ||
| blockedToastIdRef.current = null | ||
| }, [canOpenLockSettings]) |
There was a problem hiding this comment.
Announcement dismissed for non-admins
High Severity
A new useEffect dismisses lock status toasts when canOpenLockSettings is false. Because this effect runs on mount and canOpenLockSettings is always false for non-admins, it immediately dismisses the initial lock status announcement toast. Non-admins therefore don't see the lock status when navigating to a table.
Reviewed by Cursor Bugbot for commit cad3600. Configure here.
| if (canOpenLockSettings || !blockedToastIdRef.current) return | ||
| toast.dismiss(blockedToastIdRef.current) | ||
| blockedToastIdRef.current = null | ||
| }, [canOpenLockSettings]) |
There was a problem hiding this comment.
Stale toast after clearing locks
Medium Severity
The useEffect that announces table lock status uses a latch that prevents re-evaluation. This can lead to stale lock status toasts persisting after locks are cleared, and prevents the "Lock settings" action from appearing if admin permissions were initially false but later granted.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit cad3600. Configure here.
1 participant
Summary
The announcement is marked shown as soon as the table resolves, so an admin who just set locks in the panel doesn't immediately get toasted about them.
Type of Change
Testing
Tested manually.
bun run type-check,bun run lint:check, and the full audit suite (boundaries, api-validation:strict, utils, zustand-v5, react-query, client-boundary, bare-icons, icon-paths, realtime-prune, skills:check, agent-stream-docs:check) all pass.Checklist