improvement(settings): one header action order across detail pages - #6206
Conversation
Detail headers disagreed on where Delete sits. The skills page reads
`Share → Delete → Discard → Save`, but every SettingsPanel page spread
saveDiscardActions() first and appended Delete, rendering it to the RIGHT of
the primary chip: sandboxes, custom tools, custom blocks, permission groups and
data retention all did this.
Fixed in the shell rather than at nine callsites. orderHeaderActions() ranks
actions — secondary, then `id:'discard'`, then `variant:'primary'` — stably
within each band, so writing the array the natural way now produces the right
header and a new detail page cannot get it wrong. This generalizes past Save:
workflow MCP servers' `Add workflows` primary is now right-most with Delete
before it, instead of the reverse.
The ranking has to survive one trap. The shell routes onSelect through
configRef.current.actions[index] to avoid stale closures, so reordering the
render without preserving the source index would bind every chip to the wrong
handler — clicking Delete would Save. orderHeaderActions carries {action,index}
pairs; settings-header-shell.test.tsx pins that at the render level, including
the conditional-Discard case where a missing action shifts every index.
Delete is also now a plain chip on the nine resource-detail headers, matching
skills, each with a stable `id:'delete'` (three lacked one, so the chip
remounted when its label flipped to Deleting...). `variant:'destructive'` is
kept for actions destructive at scale — Delete all passwords, Clear all
browsing data, Sign out all members — which the confirm modal does not cover
the way it covers removing the single resource you are looking at.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview
Delete on resource detail pages is now a plain chip with stable Docs in Reviewed by Cursor Bugbot for commit 2cdda6c. Configure here. |
Greptile SummaryThis PR centralizes settings-header action ordering and standardizes single-resource delete actions.
Confidence Score: 5/5The PR appears safe to merge with no actionable regressions identified. The centralized sort preserves stable within-band order and original action indices, while changed delete actions retain their existing authorization and confirmation paths.
|
| Filename | Overview |
|---|---|
| apps/sim/components/settings/settings-header.tsx | Introduces stable action ranking while retaining source indices for current-ref handler dispatch. |
| apps/sim/components/settings/settings-header-order.test.ts | Covers action bands, stable within-band order, original indices, empty inputs, and input immutability. |
| apps/sim/components/settings/settings-header-shell.test.tsx | Verifies rendered ordering and correct handler routing when sorting or conditional actions shift positions. |
| apps/sim/app/workspace/[workspaceId]/settings/components/workflow-mcp-servers/workflow-mcp-servers.tsx | Standardizes the detail delete action and allows the primary Add workflows action to render right-most. |
| apps/sim/app/workspace/[workspaceId]/settings/components/custom-tools/components/custom-tool-detail/custom-tool-detail.tsx | Gives the delete action stable identity and plain styling while retaining its confirmation flow. |
| apps/sim/app/workspace/[workspaceId]/settings/components/sandboxes/sandboxes.tsx | Standardizes the confirmation-gated sandbox delete action for centralized header ordering. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Caller action array] --> B[Pair action with original index]
B --> C[Stable rank ordering]
C --> D[Secondary actions]
C --> E[Delete]
C --> F[Discard]
C --> G[Primary action]
D --> H[Rendered header]
E --> H
F --> H
G --> H
H --> I[Invoke handler through original source index]
Reviews (1): Last reviewed commit: "improvement(settings): one header action..." | Re-trigger Greptile
Summary
[secondary] → Delete → Discard → Save, matching the skills detail page.Discard → Save → Delete— a destructive chip to the right of the primary one — because they spreadsaveDiscardActions()first and appended Delete: sandboxes, custom tools, custom blocks, permission groups, data retention.orderHeaderActions()ranks actions (secondary →id:'discard'→variant:'primary'), stable within each band. Writing the array the natural way now produces the right header, so a new detail page can't get it wrong.Add workflowsprimary is now right-most with Delete before it.id:'delete'(three lacked one, so the chip remounted when its label flipped toDeleting...).variant:'destructive'kept for actions destructive at scale —Delete allpasswords,Clear allbrowsing data,Sign out all members. Removing the single resource you're viewing is already confirm-gated.action.idadded to the header re-render signature, since the sort key now reads it.Note on the risky part
The shell routes
onSelectthroughconfigRef.current.actions[index]to avoid stale closures. Reordering the render without preserving the source index would bind every chip to the wrong handler — clicking Delete would Save.orderHeaderActionscarries{action, index}pairs, andsettings-header-shell.test.tsxpins that at the render level including the conditional-Discard case where a missing action shifts every index. I verified the tests go red when the indices are renumbered.Type of Change
Testing
tscandbiomeclean; 668 tests pass across settings/skills/integrations/ee. Two new test files (14 cases) cover the ranking and the render-level handler routing; both verified to fail when the mechanism is removed. Not exercised in a browser.Checklist