diff --git a/.claude/rules/sim-list-ordering.md b/.claude/rules/sim-list-ordering.md index 2966eb4a1a6..5f2bbee86e9 100644 --- a/.claude/rules/sim-list-ordering.md +++ b/.claude/rules/sim-list-ordering.md @@ -26,6 +26,71 @@ Left-to-right becomes top-to-bottom. A toolbar reading `Filter · Sort · Export Platform-only entries (desktop **Browser** and **Terminal**) trail the shared set rather than interleaving, so the common prefix is identical on every platform. +## Grouping: one rule, against the consequential group + +Order is governed above. **Separators are governed here** — and the answer is: use at most one. + +Put a single `DropdownMenuSeparator` against the **consequential group** — the actions that +delete, detach, or change a run — and nowhere else. Everything on the other side of it runs +uninterrupted in toolbar-mirroring order. + +That group trails in almost every menu, so in practice the rule reads "one rule immediately +before Delete / Leave / Close / Hide". It leads in exactly one place: the **logs row menu**, +where `Retry` and `Cancel Run` are the primary actions on a failed run and sit at the top, with +the rule beneath them. Ordering follows the surface (see "The rule" above); the separator simply +fences whichever end the consequential group occupies. A menu whose consequential actions are +merely *disabled* still gets no extra rule — `disabled` is not a group. + +```tsx +// ✗ Bad — four semantic bands the user meets nowhere else +Open in new tab │─── Rename, Lock │─── Duplicate, Export │─── Delete + +// ✓ Good — one rule, isolating the irreversible action +Open in new tab, Rename, Lock, Duplicate, Export │─── Delete +``` + +**Why one.** No toolbar in this app renders a divider — every header is a flat +`HEADER_ACTION_CLUSTER` (`gap-1`) chip row and every bulk action bar a flat `gap-[5px]` run. A +menu banded into navigation / status / edit / copy / destructive therefore teaches a taxonomy +that appears on no other surface, and because each band is conditional, the same action lands in +a different group depending on which sibling items happen to be visible. The one thing a rule +genuinely buys is a stop before the action you cannot undo. + +A second rule is justified only when a menu mixes genuinely different *scopes* — cell-level and +table-level actions in one menu, say — not different verbs. + +**The one standing exception: menus that emulate a native menu.** The text-editor menu +(`editor-context-menu.tsx`), the terminal menu (`terminal-context-menu.tsx`), and the browser +page menu (`browser-session.tsx`) each mirror the OS menu the user already knows — clipboard +banding (`Cut · Copy · Paste │ Select all`) is a convention every text field on their machine +teaches them. These keep their native banding, and that is the *same* principle as the ordering +rule above: mirror the surface the user already reads. The test is whether a real menu outside +Sim taught them the grouping. Our own resource, row, and action menus have no such precedent — +the toolbars they mirror are flat — so they take the single rule. + +**Both sides of every rule must be guaranteed non-empty.** Write the separator's guard out of +the *exact* render conditions of the items around it, never a looser approximation: + +```tsx +// ✗ Bad — `showLeave` alone, while the Leave item needs `showLeave && onLeave`. +// A caller passing showLeave from a permission check with a conditional +// onLeave renders a trailing rule under the last item. +{hasActionsAbove && (showLeave || showDelete) && } + +// ✓ Good — each term is the item's own condition, verbatim +const hasDestructiveSection = (showLeave && onLeave) || showDelete +{hasActionsAboveDestructive && hasDestructiveSection && } +``` + +This is the failure that put a dangling rule at the bottom of the logs row menu, where two +unconditional separators sat above conditional items. + +**Do not add a prop to move a rule.** The shared workflow context menu grew +`groupNonDestructiveActions` and `separateNavigationAction` for this; 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. Both are gone. A menu that wants different +grouping wants the standard grouping. + ## Encode the order once 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. diff --git a/CLAUDE.md b/CLAUDE.md index 60c0990eecc..7faf1862acc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -386,7 +386,9 @@ Co-locate a `search-params.ts` per feature exporting the parser map (single sour 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. -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`. +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. + +**Grouping**: at most ONE `DropdownMenuSeparator` per menu, fencing the consequential group — immediately before Delete/Leave/Close/Hide in almost every menu, and immediately after Retry/Cancel Run in the logs row menu, where those lead. 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`. ## Styling diff --git a/apps/sim/app/workspace/[workspaceId]/components/folders/folder-context-menu.tsx b/apps/sim/app/workspace/[workspaceId]/components/folders/folder-context-menu.tsx index 41b8682aef6..62d0ec5cacf 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/folders/folder-context-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/components/folders/folder-context-menu.tsx @@ -91,7 +91,6 @@ export const FolderContextMenu = memo(function FolderContextMenu({ )} {canEdit && ( <> - Rename diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-row-context-menu/file-row-context-menu.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-row-context-menu/file-row-context-menu.tsx index 1fda2da80d7..e3706877b81 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-row-context-menu/file-row-context-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-row-context-menu/file-row-context-menu.tsx @@ -55,6 +55,16 @@ export const FileRowContextMenu = memo(function FileRowContextMenu({ }: FileRowContextMenuProps) { const isMultiSelect = selectedCount > 1 + /** + * Everything that can render above `Delete`: Open/Pin need a single selection, + * Download needs its handler, and the edit trio needs `canEdit` — so a multi-select + * with no download and only a move target leaves `Move to` alone above the rule. + * + * @see `.claude/rules/sim-list-ordering.md` — one rule, before the destructive group. + */ + const hasActionsAboveDestructive = + !isMultiSelect || !!onDownload || (!!onMove && !!moveOptions && moveOptions.length > 0) + return ( !open && onClose()} modal={false}> @@ -91,7 +101,6 @@ export const FileRowContextMenu = memo(function FileRowContextMenu({ )} {canEdit && ( <> - {!isMultiSelect && ( @@ -120,6 +129,7 @@ export const FileRowContextMenu = memo(function FileRowContextMenu({ )} + {hasActionsAboveDestructive && } {isMultiSelect ? `Delete ${selectedCount} items` : 'Delete'} diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/files-list-context-menu/files-list-context-menu.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/files-list-context-menu/files-list-context-menu.tsx index 1fbe5068162..60b2b06d8c5 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/files-list-context-menu/files-list-context-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/files-list-context-menu/files-list-context-menu.tsx @@ -48,7 +48,7 @@ export const FilesListContextMenu = memo(function FilesListContextMenu({ {onUploadFile && ( - Upload file + Upload )} {onCreateFolder && ( diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-tab-strip.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-tab-strip.tsx index df5adc320c8..2945162eac1 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-tab-strip.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-tab-strip.tsx @@ -216,9 +216,7 @@ export function BrowserTabStrip({ onOpenInNewTab={openTabInExternalBrowser} openInNewTabLabel='Open in External Browser' openInNewTabPosition='last' - separateNavigationAction showOpenInNewTab={Boolean(contextTab?.url && contextTab.url !== 'about:blank')} - groupNonDestructiveActions onTogglePin={ contextTab ? () => onSetTabPinned(contextTab.tabId, !contextTab.pinned) : undefined } diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/chunk-context-menu/chunk-context-menu.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/chunk-context-menu/chunk-context-menu.tsx index 605dfa1f53a..8a73d105395 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/chunk-context-menu/chunk-context-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/chunk-context-menu/chunk-context-menu.tsx @@ -33,7 +33,7 @@ interface ChunkContextMenuProps { /** * Context menu for chunks table. - * Shows chunk actions when right-clicking a row, or "Create chunk" when right-clicking empty space. + * Shows chunk actions when right-clicking a row, or "New chunk" when right-clicking empty space. * Supports batch operations when multiple chunks are selected. */ export function ChunkContextMenu({ @@ -71,6 +71,7 @@ export function ChunkContextMenu({ const hasEditSection = !isMultiSelect && (!!onEdit || !!onCopyContent) const hasStateSection = !!onToggleEnabled const hasDestructiveSection = !!onDelete + const hasActionsAboveDestructive = hasNavigationSection || hasEditSection || hasStateSection return ( !open && onClose()} modal={false}> @@ -102,11 +103,6 @@ export function ChunkContextMenu({ Open in new tab )} - {hasNavigationSection && - (hasEditSection || hasStateSection || hasDestructiveSection) && ( - - )} - {!isMultiSelect && onEdit && ( @@ -119,10 +115,6 @@ export function ChunkContextMenu({ Copy content )} - {hasEditSection && (hasStateSection || hasDestructiveSection) && ( - - )} - {onToggleEnabled && ( @@ -130,7 +122,7 @@ export function ChunkContextMenu({ )} - {hasStateSection && hasDestructiveSection && } + {hasActionsAboveDestructive && hasDestructiveSection && } {onDelete && ( @@ -142,7 +134,7 @@ export function ChunkContextMenu({ onAddChunk && ( - Create chunk + New chunk ) )} diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/document-context-menu/document-context-menu.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/document-context-menu/document-context-menu.tsx index 7050da64725..3f3cb03f63c 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/document-context-menu/document-context-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/[id]/components/document-context-menu/document-context-menu.tsx @@ -71,6 +71,7 @@ export function DocumentContextMenu({ const hasEditSection = !isMultiSelect && (!!onRename || !!onViewTags) const hasStateSection = !!onToggleEnabled const hasDestructiveSection = !!onDelete + const hasActionsAboveDestructive = hasNavigationSection || hasEditSection || hasStateSection return ( !open && onClose()} modal={false}> @@ -108,11 +109,6 @@ export function DocumentContextMenu({ Open source )} - {hasNavigationSection && - (hasEditSection || hasStateSection || hasDestructiveSection) && ( - - )} - {!isMultiSelect && onRename && ( @@ -125,10 +121,6 @@ export function DocumentContextMenu({ Tags )} - {hasEditSection && (hasStateSection || hasDestructiveSection) && ( - - )} - {onToggleEnabled && ( @@ -136,7 +128,7 @@ export function DocumentContextMenu({ )} - {hasStateSection && hasDestructiveSection && } + {hasActionsAboveDestructive && hasDestructiveSection && } {onDelete && ( @@ -148,7 +140,7 @@ export function DocumentContextMenu({ onAddDocument && ( - Add document + New documents ) )} diff --git a/apps/sim/app/workspace/[workspaceId]/knowledge/components/knowledge-base-context-menu/knowledge-base-context-menu.tsx b/apps/sim/app/workspace/[workspaceId]/knowledge/components/knowledge-base-context-menu/knowledge-base-context-menu.tsx index b809950a158..2f5fdad09e7 100644 --- a/apps/sim/app/workspace/[workspaceId]/knowledge/components/knowledge-base-context-menu/knowledge-base-context-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/knowledge/components/knowledge-base-context-menu/knowledge-base-context-menu.tsx @@ -75,6 +75,7 @@ export const KnowledgeBaseContextMenu = memo(function KnowledgeBaseContextMenu({ const hasMoveSection = !disableEdit && !!onMove && !!moveOptions && moveOptions.length > 0 const hasEditSection = (showEdit && !!onEdit) || hasMoveSection const hasDestructiveSection = showDelete && !!onDelete + const hasActionsAboveDestructive = hasNavigationSection || hasInfoSection || hasEditSection return ( !open && onClose()} modal={false}> @@ -104,10 +105,6 @@ export const KnowledgeBaseContextMenu = memo(function KnowledgeBaseContextMenu({ Open in new tab )} - {hasNavigationSection && (hasInfoSection || hasEditSection || hasDestructiveSection) && ( - - )} - {showViewTags && onViewTags && ( @@ -126,8 +123,6 @@ export const KnowledgeBaseContextMenu = memo(function KnowledgeBaseContextMenu({ {pinned ? 'Unpin' : 'Pin'} )} - {hasInfoSection && (hasEditSection || hasDestructiveSection) && } - {showEdit && onEdit && ( @@ -147,7 +142,7 @@ export const KnowledgeBaseContextMenu = memo(function KnowledgeBaseContextMenu({ )} - {hasEditSection && hasDestructiveSection && } + {hasActionsAboveDestructive && hasDestructiveSection && } {showDelete && onDelete && ( diff --git a/apps/sim/app/workspace/[workspaceId]/logs/components/log-row-context-menu/log-row-context-menu.tsx b/apps/sim/app/workspace/[workspaceId]/logs/components/log-row-context-menu/log-row-context-menu.tsx index 4f52bd9ff65..c24c7029a57 100644 --- a/apps/sim/app/workspace/[workspaceId]/logs/components/log-row-context-menu/log-row-context-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/logs/components/log-row-context-menu/log-row-context-menu.tsx @@ -103,23 +103,18 @@ export const LogRowContextMenu = memo(function LogRowContextMenu({ onCloseAutoFocus={(e) => e.preventDefault()} > {isRetryable && ( - <> - - - {isRetryPending ? 'Retrying...' : 'Retry'} - - - + + + {isRetryPending ? 'Retrying...' : 'Retry'} + )} {showCancelAction && ( - <> - - - {isStopping ? 'Stopping…' : 'Cancel Run'} - - - + + + {isStopping ? 'Stopping…' : 'Cancel Run'} + )} + {(isRetryable || showCancelAction) && } Copy Run ID @@ -128,8 +123,6 @@ export const LogRowContextMenu = memo(function LogRowContextMenu({ Copy Link - - Open Workflow @@ -138,8 +131,6 @@ export const LogRowContextMenu = memo(function LogRowContextMenu({ Open Snapshot - - {!isFilteredByThisWorkflow && ( diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/context-menu/context-menu.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/context-menu/context-menu.tsx index 1b5ebe5ce06..4b1395afd32 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/context-menu/context-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/context-menu/context-menu.tsx @@ -169,13 +169,10 @@ export function ContextMenu({ onCloseAutoFocus={(e) => e.preventDefault()} > {onAddToChat && ( - <> - - - {addToChatLabel} - - - + + + {addToChatLabel} + )} {contextMenu.columnName && canEditCell && ( diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/workflow-group-meta-cell.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/workflow-group-meta-cell.tsx index 68ee553cdad..2e1f34ee226 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/workflow-group-meta-cell.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/headers/workflow-group-meta-cell.tsx @@ -44,6 +44,12 @@ const LIMITED_RUN_PRESETS = [10, 1000] as const /** Labels for the table-scoped run items. With an active filter the run is * scoped to matching rows, so the labels say "filtered rows" to make the * narrowed target visible. Shared by both menu surfaces. */ +/** + * Incomplete before all, matching the action bar and the row context menu, which both + * present Play (empty or failed) ahead of Refresh (every row). These two menus read the + * same four run actions the user already met on the action bar, so they must not invert + * the pair — see `.claude/rules/sim-list-ordering.md`. + */ function runMenuLabels(hasActiveFilter: boolean) { const rows = hasActiveFilter ? 'filtered rows' : 'rows' return { @@ -160,34 +166,31 @@ export function ColumnOptionsMenu({ onCloseAutoFocus={(e) => e.preventDefault()} > {showRunActions && ( - <> - - - - Run - - - {showRunSelected && ( - onRunColumnSelected?.()}> - {`Run ${selectedRowCount} selected ${selectedRowCount === 1 ? 'row' : 'rows'}`} - - )} - onRunColumnAll?.()}> - {runLabels.all} + + + + Run + + + {showRunSelected && ( + onRunColumnSelected?.()}> + {`Run ${selectedRowCount} selected ${selectedRowCount === 1 ? 'row' : 'rows'}`} - onRunColumnIncomplete?.()}> - {runLabels.incomplete} - - {onRunColumnLimited && - LIMITED_RUN_PRESETS.map((max) => ( - onRunColumnLimited(max)}> - {runLabels.limited(max)} - - ))} - - - - + )} + onRunColumnIncomplete?.()}> + {runLabels.incomplete} + + {onRunColumnLimited && + LIMITED_RUN_PRESETS.map((max) => ( + onRunColumnLimited(max)}> + {runLabels.limited(max)} + + ))} + onRunColumnAll?.()}> + {runLabels.all} + + + )} {/* Sort leads the column-scoped block: the options bar reads Filter · Sort · Columns, and this menu carries no Filter item, so Sort is the @@ -217,7 +220,6 @@ export function ColumnOptionsMenu({ Sort descending - )} {onViewWorkflow && ( @@ -236,7 +238,6 @@ export function ColumnOptionsMenu({ {isPinned ? 'Unpin column' : 'Pin column'} )} - onInsertLeft(column.key)}> Insert column left @@ -515,7 +516,6 @@ export function WorkflowGroupMetaCell({ {`Run ${selectedCount} selected ${selectedCount === 1 ? 'row' : 'rows'}`} )} - {runLabels.all} {runLabels.incomplete} @@ -524,6 +524,7 @@ export function WorkflowGroupMetaCell({ {runLabels.limited(max)} ))} + {runLabels.all} )} diff --git a/apps/sim/app/workspace/[workspaceId]/tables/components/table-context-menu/table-context-menu.tsx b/apps/sim/app/workspace/[workspaceId]/tables/components/table-context-menu/table-context-menu.tsx index c5360fec673..a04fdb91445 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/components/table-context-menu/table-context-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/components/table-context-menu/table-context-menu.tsx @@ -57,6 +57,23 @@ export function TableContextMenu({ disableImport = false, disableExport = false, }: TableContextMenuProps) { + /** + * `Move to` needs a NON-EMPTY `moveOptions`, not just the handler — the looser + * `onMove` alone draws the rule with nothing above it for a table whose other + * actions are all absent and whose move list is empty. + * + * @see `.claude/rules/sim-list-ordering.md` — one rule, before the destructive + * group, with both sides built from the items' exact render conditions. + */ + const hasActionsAboveDestructive = + onViewSchema || + onRename || + onImportCsv || + onExportCsv || + (onMove && moveOptions && moveOptions.length > 0) || + onCopyId || + onTogglePin + return ( !open && onClose()} modal={false}> @@ -114,8 +131,6 @@ export function TableContextMenu({ )} - {(onViewSchema || onRename || onImportCsv || onExportCsv || onMove) && - (onCopyId || onTogglePin || onDelete) && } {onTogglePin && ( @@ -128,7 +143,7 @@ export function TableContextMenu({ Copy ID )} - {(onCopyId || onTogglePin) && onDelete && } + {hasActionsAboveDestructive && onDelete && } {onDelete && ( diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.test.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.test.tsx index 47bea5f08c4..12b7d7c8902 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.test.tsx @@ -137,3 +137,108 @@ describe('sidebar context menu dismissal', () => { expect(onClose).toHaveBeenCalled() }) }) + +describe('separators', () => { + function renderWith(props: Partial>) { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + act(() => + root?.render( + {}} + onDelete={() => {}} + showRename={false} + showDuplicate={false} + {...props} + /> + ) + ) + } + + function menuShape(): string[] { + const content = document.querySelector('[role="menu"]') + if (!content) return [] + return Array.from(content.children).map((el) => + el.getAttribute('role') === 'separator' ? 'sep' : (el.textContent ?? '') + ) + } + + it('puts its one rule directly before the destructive action', () => { + renderWith({ showRename: true, onRename: () => {}, showDelete: true }) + const shape = menuShape() + expect(shape.filter((s) => s === 'sep')).toHaveLength(1) + expect(shape[shape.indexOf('sep') + 1]).toBe('Delete') + }) + + it('renders no rule when nothing precedes the destructive action', () => { + renderWith({ showDelete: true }) + expect(menuShape()).toEqual(['Delete']) + }) + + it('renders no rule when there is no destructive action', () => { + renderWith({ showRename: true, onRename: () => {}, showDelete: false }) + const shape = menuShape() + expect(shape).not.toContain('sep') + expect(shape).toEqual(['Rename']) + }) + + it('never trails a rule when showLeave is set without an onLeave handler', () => { + renderWith({ + showRename: true, + onRename: () => {}, + showLeave: true, + onLeave: undefined, + showDelete: false, + }) + const shape = menuShape() + expect(shape.at(-1)).not.toBe('sep') + expect(shape).toEqual(['Rename']) + }) + + it('draws the rule for Leave when its handler is present', () => { + renderWith({ + showRename: true, + onRename: () => {}, + showLeave: true, + onLeave: () => {}, + showDelete: false, + }) + expect(menuShape()).toEqual(['Rename', 'sep', 'Leave']) + }) + + it('never renders two rules back to back across a broad flag sweep', () => { + const noop = () => {} + for (const showOpenInNewTab of [true, false]) { + for (const showPin of [true, false]) { + for (const showLock of [true, false]) { + for (const showExport of [true, false]) { + for (const showDelete of [true, false]) { + act(() => root?.unmount()) + container?.remove() + renderWith({ + showOpenInNewTab, + onOpenInNewTab: noop, + showPin, + onTogglePin: noop, + showLock, + onToggleLock: noop, + showExport, + onExport: noop, + showDelete, + }) + const shape = menuShape() + expect(shape.filter((s) => s === 'sep').length).toBeLessThanOrEqual(1) + expect(shape.at(0)).not.toBe('sep') + expect(shape.at(-1)).not.toBe('sep') + } + } + } + } + } + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.tsx index 8be1feef7f3..3d4cfef1a2e 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workflow-list/components/context-menu/context-menu.tsx @@ -35,8 +35,6 @@ interface ContextMenuProps { onOpenInNewTab?: () => void openInNewTabLabel?: string openInNewTabPosition?: 'first' | 'last' - separateNavigationAction?: boolean - groupNonDestructiveActions?: boolean onMarkAsRead?: () => void onMarkAsUnread?: () => void onTogglePin?: () => void @@ -121,8 +119,6 @@ export function ContextMenu({ onOpenInNewTab, openInNewTabLabel = 'Open in new tab', openInNewTabPosition = 'first', - separateNavigationAction = false, - groupNonDestructiveActions = false, onMarkAsRead, onMarkAsUnread, onTogglePin, @@ -169,18 +165,24 @@ export function ContextMenu({ showUploadLogo = false, disableUploadLogo = false, }: ContextMenuProps) { - const hasNavigationSection = showOpenInNewTab && onOpenInNewTab - const hasStatusSection = + const hasActionsAboveDestructive = + (showOpenInNewTab && onOpenInNewTab) || (showMarkAsRead && onMarkAsRead) || (showMarkAsUnread && onMarkAsUnread) || - (showPin && onTogglePin) - const hasEditSection = + (showPin && onTogglePin) || (showRename && onRename) || (showCreate && onCreate) || (showCreateFolder && onCreateFolder) || (showLock && onToggleLock) || - (showUploadLogo && onUploadLogo) - const hasCopySection = (showDuplicate && onDuplicate) || (showExport && onExport) + (showUploadLogo && onUploadLogo) || + (showDuplicate && onDuplicate) || + (showExport && onExport) + const hasDestructiveSection = + (showLeave && onLeave) || + showDelete || + (showCloseTab && onCloseTab) || + onCloseOtherTabs || + onCloseTabsToRight /** * Only the "Rename" item should trigger the `onCloseAutoFocus` refocus below — @@ -237,11 +239,6 @@ export function ContextMenu({ {openInNewTabLabel} )} - {openInNewTabPosition === 'first' && - (!groupNonDestructiveActions || separateNavigationAction) && - hasNavigationSection && - (hasStatusSection || hasEditSection || hasCopySection) && } - {showMarkAsRead && onMarkAsRead && ( )} - {!groupNonDestructiveActions && hasStatusSection && (hasEditSection || hasCopySection) && ( - - )} - {showRename && onRename && ( )} - {!groupNonDestructiveActions && hasEditSection && hasCopySection && ( - - )} {showDuplicate && onDuplicate && ( )} - {openInNewTabPosition === 'last' && - (!groupNonDestructiveActions || separateNavigationAction) && - hasNavigationSection && - (hasStatusSection || hasEditSection || hasCopySection) && } {openInNewTabPosition === 'last' && showOpenInNewTab && onOpenInNewTab && ( { @@ -386,12 +372,7 @@ export function ContextMenu({ )} - {(hasNavigationSection || hasStatusSection || hasEditSection || hasCopySection) && - (showLeave || - showDelete || - (showCloseTab && onCloseTab) || - onCloseOtherTabs || - onCloseTabsToRight) && } + {hasActionsAboveDestructive && hasDestructiveSection && } {showLeave && onLeave && (