Skip to content

Commit 3e586ee

Browse files
waleedlatif1waleedlatif1
committed
improvement(menus): one separator per menu, before the destructive action
Menus banded themselves into semantic groups — navigation, status, edit, copy, destructive — behind two to four separators each. No toolbar in the app renders a divider: every header is a flat gap-1 chip row and every bulk action bar a flat gap-[5px] run. The bands therefore taught a taxonomy the user met on no other surface, and because each band is conditional, the same action landed in a different group depending on which siblings happened to be visible. Pin sat alone in one caller of the shared workflow menu and beside Duplicate in another. Every menu now carries at most one rule, immediately before the destructive group. Order is untouched, so the toolbar-mirroring the ordering rule requires is unaffected. Two separator bugs fixed. The logs row menu had two unconditional separators above conditional items, so a log already filtered by its workflow with no active filters ended on a dangling rule. The shared workflow menu guarded its destructive rule on showLeave alone while the Leave item required showLeave && onLeave, so a caller passing showLeave from a permission check with a conditional onLeave would trail a rule under the last item; every term in both guards is now the exact render condition of the item it stands for. Removes groupNonDestructiveActions and separateNavigationAction. Between them they moved one separator for one caller, four of six branches were unreachable, and separateNavigationAction had no observable effect anywhere in the repo. The separator matrix was previously untested, which is how the showLeave asymmetry survived; it now has invariants including a flag sweep.
1 parent b02fee1 commit 3e586ee

13 files changed

Lines changed: 263 additions & 120 deletions

File tree

‎.claude/rules/sim-list-ordering.md‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,54 @@ Left-to-right becomes top-to-bottom. A toolbar reading `Filter · Sort · Export
2626

2727
Platform-only entries (desktop **Browser** and **Terminal**) trail the shared set rather than interleaving, so the common prefix is identical on every platform.
2828

29+
## Grouping: one rule, before the destructive action
30+
31+
Order is governed above. **Separators are governed here** — and the answer is: use at most one.
32+
33+
Put a single `DropdownMenuSeparator` immediately before the destructive group (Delete, Leave,
34+
Close, Hide) and nowhere else. Everything above it runs uninterrupted in toolbar-mirroring order.
35+
36+
```tsx
37+
// ✗ Bad — four semantic bands the user meets nowhere else
38+
Open in new tab │─── Rename, Lock │─── Duplicate, Export │─── Delete
39+
40+
// ✓ Good — one rule, isolating the irreversible action
41+
Open in new tab, Rename, Lock, Duplicate, Export │─── Delete
42+
```
43+
44+
**Why one.** No toolbar in this app renders a divider — every header is a flat
45+
`HEADER_ACTION_CLUSTER` (`gap-1`) chip row and every bulk action bar a flat `gap-[5px]` run. A
46+
menu banded into navigation / status / edit / copy / destructive therefore teaches a taxonomy
47+
that appears on no other surface, and because each band is conditional, the same action lands in
48+
a different group depending on which sibling items happen to be visible. The one thing a rule
49+
genuinely buys is a stop before the action you cannot undo.
50+
51+
A second rule is justified only when a menu mixes genuinely different *scopes* — cell-level and
52+
table-level actions in one menu, say — not different verbs.
53+
54+
**Both sides of every rule must be guaranteed non-empty.** Write the separator's guard out of
55+
the *exact* render conditions of the items around it, never a looser approximation:
56+
57+
```tsx
58+
// ✗ Bad — `showLeave` alone, while the Leave item needs `showLeave && onLeave`.
59+
// A caller passing showLeave from a permission check with a conditional
60+
// onLeave renders a trailing rule under the last item.
61+
{hasActionsAbove && (showLeave || showDelete) && <DropdownMenuSeparator />}
62+
63+
// ✓ Good — each term is the item's own condition, verbatim
64+
const hasDestructiveSection = (showLeave && onLeave) || showDelete
65+
{hasActionsAboveDestructive && hasDestructiveSection && <DropdownMenuSeparator />}
66+
```
67+
68+
This is the failure that put a dangling rule at the bottom of the logs row menu, where two
69+
unconditional separators sat above conditional items.
70+
71+
**Do not add a prop to move a rule.** The shared workflow context menu grew
72+
`groupNonDestructiveActions` and `separateNavigationAction` for this; between them they moved one
73+
separator for one caller, four of six branches were unreachable, and `separateNavigationAction`
74+
had no observable effect anywhere in the repo. Both are gone. A menu that wants different
75+
grouping wants the standard grouping.
76+
2977
## Encode the order once
3078

3179
An order duplicated across surfaces is an order that will drift. Export **one** constant and sort by it — do not hand-maintain a matching literal per menu.

‎CLAUDE.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,9 @@ Co-locate a `search-params.ts` per feature exporting the parser map (single sour
386386

387387
A list orders itself the way the user already reads the same things somewhere else. Resource menus (`+` attach, `@` mention, resource-tab `+`) mirror the **sidebar** top-down; a row or root **context menu** mirrors that surface's **toolbar**, left-to-right becoming top-to-bottom; tab strips mirror their nav. Platform-only entries (desktop Browser, Terminal) trail the shared set.
388388

389-
Encode the order in ONE exported constant and sort by it — never a hand-maintained literal per menu (`RESOURCE_MENU_ORDER` / `byResourceMenuOrder` in `home/components/mothership-view/components/resource-registry`). Render mixed item kinds in a single ordered pass; emitting all submenu-backed families and then all flat ones silently pins every submenu to the top no matter what the constant says. Divergence is allowed only for search ranking, user-controlled ordering, and recency. Full rule in `.claude/rules/sim-list-ordering.md`.
389+
Encode the order in ONE exported constant and sort by it — never a hand-maintained literal per menu (`RESOURCE_MENU_ORDER` / `byResourceMenuOrder` in `home/components/mothership-view/components/resource-registry`). Render mixed item kinds in a single ordered pass; emitting all submenu-backed families and then all flat ones silently pins every submenu to the top no matter what the constant says. Divergence is allowed only for search ranking, user-controlled ordering, and recency.
390+
391+
**Grouping**: at most ONE `DropdownMenuSeparator` per menu, immediately before the destructive group (Delete/Leave/Close/Hide). No toolbar in the app renders a divider, so multi-band menus teach a taxonomy that exists on no other surface. Build each separator's guard from the EXACT render conditions of the items on both sides — a looser guard is what leaves a dangling rule when its group is conditional. Never add a prop to move a rule. Full rule in `.claude/rules/sim-list-ordering.md`.
390392

391393
## Styling
392394

‎apps/sim/app/workspace/[workspaceId]/files/components/file-row-context-menu/file-row-context-menu.tsx‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,16 @@ export const FileRowContextMenu = memo(function FileRowContextMenu({
5555
}: FileRowContextMenuProps) {
5656
const isMultiSelect = selectedCount > 1
5757

58+
/**
59+
* Everything that can render above `Delete`: Open/Pin need a single selection,
60+
* Download needs its handler, and the edit trio needs `canEdit` — so a multi-select
61+
* with no download and only a move target leaves `Move to` alone above the rule.
62+
*
63+
* @see `.claude/rules/sim-list-ordering.md` — one rule, before the destructive group.
64+
*/
65+
const hasActionsAboveDestructive =
66+
!isMultiSelect || !!onDownload || (!!onMove && !!moveOptions && moveOptions.length > 0)
67+
5868
return (
5969
<DropdownMenu open={isOpen} onOpenChange={(open) => !open && onClose()} modal={false}>
6070
<DropdownMenuTrigger asChild>
@@ -91,7 +101,6 @@ export const FileRowContextMenu = memo(function FileRowContextMenu({
91101
)}
92102
{canEdit && (
93103
<>
94-
<DropdownMenuSeparator />
95104
{!isMultiSelect && (
96105
<DropdownMenuItem onSelect={onRename}>
97106
<Pencil />
@@ -120,6 +129,7 @@ export const FileRowContextMenu = memo(function FileRowContextMenu({
120129
</DropdownMenuSubContent>
121130
</DropdownMenuSub>
122131
)}
132+
{hasActionsAboveDestructive && <DropdownMenuSeparator />}
123133
<DropdownMenuItem onSelect={onDelete}>
124134
<Trash />
125135
{isMultiSelect ? `Delete ${selectedCount} items` : 'Delete'}

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-tab-strip.tsx‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,9 +216,7 @@ export function BrowserTabStrip({
216216
onOpenInNewTab={openTabInExternalBrowser}
217217
openInNewTabLabel='Open in External Browser'
218218
openInNewTabPosition='last'
219-
separateNavigationAction
220219
showOpenInNewTab={Boolean(contextTab?.url && contextTab.url !== 'about:blank')}
221-
groupNonDestructiveActions
222220
onTogglePin={
223221
contextTab ? () => onSetTabPinned(contextTab.tabId, !contextTab.pinned) : undefined
224222
}

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/chunk-context-menu/chunk-context-menu.tsx‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ export function ChunkContextMenu({
7171
const hasEditSection = !isMultiSelect && (!!onEdit || !!onCopyContent)
7272
const hasStateSection = !!onToggleEnabled
7373
const hasDestructiveSection = !!onDelete
74+
/** @see `.claude/rules/sim-list-ordering.md` — one rule, before the destructive group. */
75+
const hasActionsAboveDestructive = hasNavigationSection || hasEditSection || hasStateSection
7476

7577
return (
7678
<DropdownMenu open={isOpen} onOpenChange={(open) => !open && onClose()} modal={false}>
@@ -102,11 +104,6 @@ export function ChunkContextMenu({
102104
Open in new tab
103105
</DropdownMenuItem>
104106
)}
105-
{hasNavigationSection &&
106-
(hasEditSection || hasStateSection || hasDestructiveSection) && (
107-
<DropdownMenuSeparator />
108-
)}
109-
110107
{!isMultiSelect && onEdit && (
111108
<DropdownMenuItem disabled={disableEdit} onSelect={onEdit}>
112109
<Pencil />
@@ -119,18 +116,14 @@ export function ChunkContextMenu({
119116
Copy content
120117
</DropdownMenuItem>
121118
)}
122-
{hasEditSection && (hasStateSection || hasDestructiveSection) && (
123-
<DropdownMenuSeparator />
124-
)}
125-
126119
{onToggleEnabled && (
127120
<DropdownMenuItem disabled={disableToggleEnabled} onSelect={onToggleEnabled}>
128121
<Eye />
129122
{getToggleLabel()}
130123
</DropdownMenuItem>
131124
)}
132125

133-
{hasStateSection && hasDestructiveSection && <DropdownMenuSeparator />}
126+
{hasActionsAboveDestructive && hasDestructiveSection && <DropdownMenuSeparator />}
134127
{onDelete && (
135128
<DropdownMenuItem disabled={disableDelete} onSelect={onDelete}>
136129
<Trash />

‎apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/document-context-menu/document-context-menu.tsx‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ export function DocumentContextMenu({
7171
const hasEditSection = !isMultiSelect && (!!onRename || !!onViewTags)
7272
const hasStateSection = !!onToggleEnabled
7373
const hasDestructiveSection = !!onDelete
74+
/** @see `.claude/rules/sim-list-ordering.md` — one rule, before the destructive group. */
75+
const hasActionsAboveDestructive = hasNavigationSection || hasEditSection || hasStateSection
7476

7577
return (
7678
<DropdownMenu open={isOpen} onOpenChange={(open) => !open && onClose()} modal={false}>
@@ -108,11 +110,6 @@ export function DocumentContextMenu({
108110
Open source
109111
</DropdownMenuItem>
110112
)}
111-
{hasNavigationSection &&
112-
(hasEditSection || hasStateSection || hasDestructiveSection) && (
113-
<DropdownMenuSeparator />
114-
)}
115-
116113
{!isMultiSelect && onRename && (
117114
<DropdownMenuItem disabled={disableRename} onSelect={onRename}>
118115
<Pencil />
@@ -125,18 +122,14 @@ export function DocumentContextMenu({
125122
Tags
126123
</DropdownMenuItem>
127124
)}
128-
{hasEditSection && (hasStateSection || hasDestructiveSection) && (
129-
<DropdownMenuSeparator />
130-
)}
131-
132125
{onToggleEnabled && (
133126
<DropdownMenuItem disabled={disableToggleEnabled} onSelect={onToggleEnabled}>
134127
<Eye />
135128
{getToggleLabel()}
136129
</DropdownMenuItem>
137130
)}
138131

139-
{hasStateSection && hasDestructiveSection && <DropdownMenuSeparator />}
132+
{hasActionsAboveDestructive && hasDestructiveSection && <DropdownMenuSeparator />}
140133
{onDelete && (
141134
<DropdownMenuItem disabled={disableDelete} onSelect={onDelete}>
142135
<Trash />

‎apps/sim/app/workspace/[workspaceId]/knowledge/components/knowledge-base-context-menu/knowledge-base-context-menu.tsx‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ export const KnowledgeBaseContextMenu = memo(function KnowledgeBaseContextMenu({
7575
const hasMoveSection = !disableEdit && !!onMove && !!moveOptions && moveOptions.length > 0
7676
const hasEditSection = (showEdit && !!onEdit) || hasMoveSection
7777
const hasDestructiveSection = showDelete && !!onDelete
78+
/** @see `.claude/rules/sim-list-ordering.md` — one rule, before the destructive group. */
79+
const hasActionsAboveDestructive = hasNavigationSection || hasInfoSection || hasEditSection
7880

7981
return (
8082
<DropdownMenu open={isOpen} onOpenChange={(open) => !open && onClose()} modal={false}>
@@ -104,10 +106,6 @@ export const KnowledgeBaseContextMenu = memo(function KnowledgeBaseContextMenu({
104106
Open in new tab
105107
</DropdownMenuItem>
106108
)}
107-
{hasNavigationSection && (hasInfoSection || hasEditSection || hasDestructiveSection) && (
108-
<DropdownMenuSeparator />
109-
)}
110-
111109
{showViewTags && onViewTags && (
112110
<DropdownMenuItem onSelect={onViewTags}>
113111
<TagIcon />
@@ -126,8 +124,6 @@ export const KnowledgeBaseContextMenu = memo(function KnowledgeBaseContextMenu({
126124
{pinned ? 'Unpin' : 'Pin'}
127125
</DropdownMenuItem>
128126
)}
129-
{hasInfoSection && (hasEditSection || hasDestructiveSection) && <DropdownMenuSeparator />}
130-
131127
{showEdit && onEdit && (
132128
<DropdownMenuItem disabled={disableEdit} onSelect={onEdit}>
133129
<Pencil />
@@ -147,7 +143,7 @@ export const KnowledgeBaseContextMenu = memo(function KnowledgeBaseContextMenu({
147143
</DropdownMenuSub>
148144
)}
149145

150-
{hasEditSection && hasDestructiveSection && <DropdownMenuSeparator />}
146+
{hasActionsAboveDestructive && hasDestructiveSection && <DropdownMenuSeparator />}
151147
{showDelete && onDelete && (
152148
<DropdownMenuItem disabled={disableDelete} onSelect={onDelete}>
153149
<Trash />

‎apps/sim/app/workspace/[workspaceId]/logs/components/log-row-context-menu/log-row-context-menu.tsx‎

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -103,23 +103,18 @@ export const LogRowContextMenu = memo(function LogRowContextMenu({
103103
onCloseAutoFocus={(e) => e.preventDefault()}
104104
>
105105
{isRetryable && (
106-
<>
107-
<DropdownMenuItem onSelect={onRetryExecution} disabled={isRetryPending}>
108-
<Redo />
109-
{isRetryPending ? 'Retrying...' : 'Retry'}
110-
</DropdownMenuItem>
111-
<DropdownMenuSeparator />
112-
</>
106+
<DropdownMenuItem onSelect={onRetryExecution} disabled={isRetryPending}>
107+
<Redo />
108+
{isRetryPending ? 'Retrying...' : 'Retry'}
109+
</DropdownMenuItem>
113110
)}
114111
{showCancelAction && (
115-
<>
116-
<DropdownMenuItem onSelect={onCancelExecution} disabled={isStopping}>
117-
<X />
118-
{isStopping ? 'Stopping…' : 'Cancel Run'}
119-
</DropdownMenuItem>
120-
<DropdownMenuSeparator />
121-
</>
112+
<DropdownMenuItem onSelect={onCancelExecution} disabled={isStopping}>
113+
<X />
114+
{isStopping ? 'Stopping…' : 'Cancel Run'}
115+
</DropdownMenuItem>
122116
)}
117+
{(isRetryable || showCancelAction) && <DropdownMenuSeparator />}
123118
<DropdownMenuItem disabled={!hasExecutionId} onSelect={onCopyExecutionId}>
124119
<Duplicate />
125120
Copy Run ID
@@ -128,8 +123,6 @@ export const LogRowContextMenu = memo(function LogRowContextMenu({
128123
<Link />
129124
Copy Link
130125
</DropdownMenuItem>
131-
132-
<DropdownMenuSeparator />
133126
<DropdownMenuItem disabled={!hasOpenableWorkflow} onSelect={onOpenWorkflow}>
134127
<SquareArrowUpRight />
135128
Open Workflow
@@ -138,8 +131,6 @@ export const LogRowContextMenu = memo(function LogRowContextMenu({
138131
<Eye />
139132
Open Snapshot
140133
</DropdownMenuItem>
141-
142-
<DropdownMenuSeparator />
143134
{!isFilteredByThisWorkflow && (
144135
<DropdownMenuItem disabled={!hasWorkflow} onSelect={onToggleWorkflowFilter}>
145136
<ListFilter />

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/context-menu/context-menu.tsx‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -169,13 +169,10 @@ export function ContextMenu({
169169
onCloseAutoFocus={(e) => e.preventDefault()}
170170
>
171171
{onAddToChat && (
172-
<>
173-
<DropdownMenuItem onSelect={onAddToChat}>
174-
<Blimp />
175-
{addToChatLabel}
176-
</DropdownMenuItem>
177-
<DropdownMenuSeparator />
178-
</>
172+
<DropdownMenuItem onSelect={onAddToChat}>
173+
<Blimp />
174+
{addToChatLabel}
175+
</DropdownMenuItem>
179176
)}
180177
{contextMenu.columnName && canEditCell && (
181178
<DropdownMenuItem disabled={disableEdit} onSelect={onEditCell}>

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/workflow-group-meta-cell.tsx‎

Lines changed: 24 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -160,34 +160,31 @@ export function ColumnOptionsMenu({
160160
onCloseAutoFocus={(e) => e.preventDefault()}
161161
>
162162
{showRunActions && (
163-
<>
164-
<DropdownMenuSub>
165-
<DropdownMenuSubTrigger>
166-
<PlayOutline />
167-
Run
168-
</DropdownMenuSubTrigger>
169-
<DropdownMenuSubContent>
170-
{showRunSelected && (
171-
<DropdownMenuItem onSelect={() => onRunColumnSelected?.()}>
172-
{`Run ${selectedRowCount} selected ${selectedRowCount === 1 ? 'row' : 'rows'}`}
173-
</DropdownMenuItem>
174-
)}
175-
<DropdownMenuItem onSelect={() => onRunColumnAll?.()}>
176-
{runLabels.all}
177-
</DropdownMenuItem>
178-
<DropdownMenuItem onSelect={() => onRunColumnIncomplete?.()}>
179-
{runLabels.incomplete}
163+
<DropdownMenuSub>
164+
<DropdownMenuSubTrigger>
165+
<PlayOutline />
166+
Run
167+
</DropdownMenuSubTrigger>
168+
<DropdownMenuSubContent>
169+
{showRunSelected && (
170+
<DropdownMenuItem onSelect={() => onRunColumnSelected?.()}>
171+
{`Run ${selectedRowCount} selected ${selectedRowCount === 1 ? 'row' : 'rows'}`}
180172
</DropdownMenuItem>
181-
{onRunColumnLimited &&
182-
LIMITED_RUN_PRESETS.map((max) => (
183-
<DropdownMenuItem key={max} onSelect={() => onRunColumnLimited(max)}>
184-
{runLabels.limited(max)}
185-
</DropdownMenuItem>
186-
))}
187-
</DropdownMenuSubContent>
188-
</DropdownMenuSub>
189-
<DropdownMenuSeparator />
190-
</>
173+
)}
174+
<DropdownMenuItem onSelect={() => onRunColumnAll?.()}>
175+
{runLabels.all}
176+
</DropdownMenuItem>
177+
<DropdownMenuItem onSelect={() => onRunColumnIncomplete?.()}>
178+
{runLabels.incomplete}
179+
</DropdownMenuItem>
180+
{onRunColumnLimited &&
181+
LIMITED_RUN_PRESETS.map((max) => (
182+
<DropdownMenuItem key={max} onSelect={() => onRunColumnLimited(max)}>
183+
{runLabels.limited(max)}
184+
</DropdownMenuItem>
185+
))}
186+
</DropdownMenuSubContent>
187+
</DropdownMenuSub>
191188
)}
192189
{/* Sort leads the column-scoped block: the options bar reads Filter ·
193190
Sort · Columns, and this menu carries no Filter item, so Sort is the
@@ -217,7 +214,6 @@ export function ColumnOptionsMenu({
217214
<ArrowDown />
218215
Sort descending
219216
</DropdownMenuItem>
220-
<DropdownMenuSeparator />
221217
</>
222218
)}
223219
{onViewWorkflow && (
@@ -236,7 +232,6 @@ export function ColumnOptionsMenu({
236232
{isPinned ? 'Unpin column' : 'Pin column'}
237233
</DropdownMenuItem>
238234
)}
239-
<DropdownMenuSeparator />
240235
<DropdownMenuItem onSelect={() => onInsertLeft(column.key)}>
241236
<ArrowLeft />
242237
Insert column left

0 commit comments

Comments
 (0)