Skip to content

Commit ab35ff4

Browse files
waleedlatif1waleedlatif1
authored
fix(resources): make context menus selection-aware (#6980)
* fix(resources): make context menus selection-aware * fix(resources): correct bulk action scopes * fix(knowledge): avoid overstating select-all scope * fix(knowledge): keep select-all actions consistent
1 parent e46fc1d commit ab35ff4

17 files changed

Lines changed: 476 additions & 100 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/components/folders/folder-context-menu.tsx‎

Lines changed: 52 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
import { Duplicate, Eye, FolderInput, Pencil, Pin, Trash } from '@sim/emcn/icons'
1515
import type { MoveOptionNode } from '@/app/workspace/[workspaceId]/components/folders/move-options'
1616
import { renderMoveOptions } from '@/app/workspace/[workspaceId]/components/folders/move-options'
17+
import { selectionActionLabel } from '@/app/workspace/[workspaceId]/components/resource/selection-label'
1718

1819
interface FolderContextMenuProps {
1920
isOpen: boolean
@@ -29,6 +30,7 @@ interface FolderContextMenuProps {
2930
pinned: boolean
3031
moveOptions?: MoveOptionNode[]
3132
canEdit: boolean
33+
selectedCount: number
3234
}
3335

3436
/**
@@ -56,8 +58,12 @@ export const FolderContextMenu = memo(function FolderContextMenu({
5658
pinned,
5759
moveOptions,
5860
canEdit,
61+
selectedCount,
5962
}: FolderContextMenuProps) {
63+
const isMultiSelect = selectedCount > 1
6064
const hasMove = Boolean(onMove && moveOptions && moveOptions.length > 0)
65+
const hasActionsAboveDestructive = !isMultiSelect || hasMove
66+
const hasAvailableActions = !isMultiSelect || canEdit
6167

6268
return (
6369
<DropdownMenu open={isOpen} onOpenChange={(open) => !open && onClose()} modal={false}>
@@ -75,42 +81,54 @@ export const FolderContextMenu = memo(function FolderContextMenu({
7581
sideOffset={4}
7682
onCloseAutoFocus={(e) => e.preventDefault()}
7783
>
78-
<DropdownMenuItem onSelect={onOpen}>
79-
<Eye />
80-
Open
81-
</DropdownMenuItem>
82-
<DropdownMenuItem onSelect={onTogglePin}>
83-
<Pin />
84-
{pinned ? 'Unpin' : 'Pin'}
85-
</DropdownMenuItem>
86-
{onCopyId && (
87-
<DropdownMenuItem onSelect={onCopyId}>
88-
<Duplicate />
89-
Copy ID
90-
</DropdownMenuItem>
91-
)}
92-
{canEdit && (
84+
{!hasAvailableActions ? (
85+
<DropdownMenuItem disabled>No actions available</DropdownMenuItem>
86+
) : (
9387
<>
94-
<DropdownMenuItem onSelect={onRename}>
95-
<Pencil />
96-
Rename
97-
</DropdownMenuItem>
98-
{hasMove && (
99-
<DropdownMenuSub>
100-
<DropdownMenuSubTrigger>
101-
<FolderInput />
102-
Move to
103-
</DropdownMenuSubTrigger>
104-
<DropdownMenuSubContent>
105-
{renderMoveOptions(moveOptions!, onMove!)}
106-
</DropdownMenuSubContent>
107-
</DropdownMenuSub>
88+
{!isMultiSelect && (
89+
<>
90+
<DropdownMenuItem onSelect={onOpen}>
91+
<Eye />
92+
Open
93+
</DropdownMenuItem>
94+
<DropdownMenuItem onSelect={onTogglePin}>
95+
<Pin />
96+
{pinned ? 'Unpin' : 'Pin'}
97+
</DropdownMenuItem>
98+
{onCopyId && (
99+
<DropdownMenuItem onSelect={onCopyId}>
100+
<Duplicate />
101+
Copy ID
102+
</DropdownMenuItem>
103+
)}
104+
</>
105+
)}
106+
{canEdit && (
107+
<>
108+
{!isMultiSelect && (
109+
<DropdownMenuItem onSelect={onRename}>
110+
<Pencil />
111+
Rename
112+
</DropdownMenuItem>
113+
)}
114+
{hasMove && (
115+
<DropdownMenuSub>
116+
<DropdownMenuSubTrigger>
117+
<FolderInput />
118+
{selectionActionLabel('Move', selectedCount, 'Move to')}
119+
</DropdownMenuSubTrigger>
120+
<DropdownMenuSubContent>
121+
{renderMoveOptions(moveOptions!, onMove!)}
122+
</DropdownMenuSubContent>
123+
</DropdownMenuSub>
124+
)}
125+
{hasActionsAboveDestructive && <DropdownMenuSeparator />}
126+
<DropdownMenuItem onSelect={onDelete}>
127+
<Trash />
128+
{selectionActionLabel('Delete', selectedCount)}
129+
</DropdownMenuItem>
130+
</>
108131
)}
109-
<DropdownMenuSeparator />
110-
<DropdownMenuItem onSelect={onDelete}>
111-
<Trash />
112-
Delete
113-
</DropdownMenuItem>
114132
</>
115133
)}
116134
</DropdownMenuContent>

‎apps/sim/app/workspace/[workspaceId]/components/resource/selection-aware-context-menus.test.tsx‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
import type { ReactNode } from 'react'
2+
import { renderToStaticMarkup } from 'react-dom/server'
3+
import { describe, expect, it, vi } from 'vitest'
4+
5+
vi.mock('@sim/emcn', () => ({
6+
DropdownMenu: ({ children, open }: { children: ReactNode; open: boolean }) =>
7+
open ? <>{children}</> : null,
8+
DropdownMenuContent: ({ children }: { children: ReactNode }) => <>{children}</>,
9+
DropdownMenuItem: ({ children }: { children: ReactNode }) => <span>{children}</span>,
10+
DropdownMenuSeparator: () => <hr />,
11+
DropdownMenuSub: ({ children }: { children: ReactNode }) => <>{children}</>,
12+
DropdownMenuSubContent: ({ children }: { children: ReactNode }) => <>{children}</>,
13+
DropdownMenuSubTrigger: ({ children }: { children: ReactNode }) => <span>{children}</span>,
14+
DropdownMenuTrigger: ({ children }: { children: ReactNode }) => <>{children}</>,
15+
Upload: () => null,
16+
}))
17+
18+
vi.mock('@sim/emcn/icons', () => ({
19+
Database: () => null,
20+
Download: () => null,
21+
Duplicate: () => null,
22+
Eye: () => null,
23+
FolderInput: () => null,
24+
Pencil: () => null,
25+
Pin: () => null,
26+
Plus: () => null,
27+
SquareArrowUpRight: () => null,
28+
TagIcon: () => null,
29+
Trash: () => null,
30+
}))
31+
32+
vi.mock('@/app/workspace/[workspaceId]/components/folders', () => ({
33+
renderMoveOptions: () => <span>Destination</span>,
34+
}))
35+
36+
vi.mock('@/app/workspace/[workspaceId]/components/folders/move-options', () => ({
37+
renderMoveOptions: () => <span>Destination</span>,
38+
}))
39+
40+
import { FolderContextMenu } from '@/app/workspace/[workspaceId]/components/folders/folder-context-menu'
41+
import { ChunkContextMenu } from '@/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/chunk-context-menu/chunk-context-menu'
42+
import { DocumentContextMenu } from '@/app/workspace/[workspaceId]/knowledge/[id]/components/document-context-menu/document-context-menu'
43+
import { KnowledgeBaseContextMenu } from '@/app/workspace/[workspaceId]/knowledge/components/knowledge-base-context-menu/knowledge-base-context-menu'
44+
import { TableContextMenu } from '@/app/workspace/[workspaceId]/tables/components/table-context-menu/table-context-menu'
45+
46+
const POSITION = { x: 0, y: 0 }
47+
const MOVE_OPTIONS = [{ value: '__root__', label: 'Root', children: [] }]
48+
49+
describe('selection-aware resource context menus', () => {
50+
it('limits a multi-table menu to actions that can target the selection', () => {
51+
const menu = renderToStaticMarkup(
52+
<TableContextMenu
53+
isOpen
54+
position={POSITION}
55+
onClose={() => {}}
56+
onCopyId={() => {}}
57+
onTogglePin={() => {}}
58+
onDelete={() => {}}
59+
onViewSchema={() => {}}
60+
onRename={() => {}}
61+
onImportCsv={() => {}}
62+
onExportCsv={() => {}}
63+
onMove={() => {}}
64+
moveOptions={MOVE_OPTIONS}
65+
selectedCount={3}
66+
/>
67+
)
68+
69+
expect(menu).toContain('Move 3 items')
70+
expect(menu).toContain('Delete 3 items')
71+
expect(menu).not.toContain('View Schema')
72+
expect(menu).not.toContain('Rename')
73+
expect(menu).not.toContain('Copy ID')
74+
expect(menu).not.toContain('Pin')
75+
})
76+
77+
it('limits a multi-base menu to actions that can target the selection', () => {
78+
const menu = renderToStaticMarkup(
79+
<KnowledgeBaseContextMenu
80+
isOpen
81+
position={POSITION}
82+
onClose={() => {}}
83+
onOpenInNewTab={() => {}}
84+
onViewTags={() => {}}
85+
onCopyId={() => {}}
86+
onTogglePin={() => {}}
87+
onEdit={() => {}}
88+
onDelete={() => {}}
89+
onMove={() => {}}
90+
moveOptions={MOVE_OPTIONS}
91+
selectedCount={2}
92+
/>
93+
)
94+
95+
expect(menu).toContain('Move 2 items')
96+
expect(menu).toContain('Delete 2 items')
97+
expect(menu).not.toContain('Open in new tab')
98+
expect(menu).not.toContain('View tags')
99+
expect(menu).not.toContain('Copy ID')
100+
expect(menu).not.toContain('Pin')
101+
expect(menu).not.toContain('Edit')
102+
})
103+
104+
it('uses the same group-action contract when a selected folder opens the menu', () => {
105+
const menu = renderToStaticMarkup(
106+
<FolderContextMenu
107+
isOpen
108+
position={POSITION}
109+
onClose={() => {}}
110+
onOpen={() => {}}
111+
onRename={() => {}}
112+
onDelete={() => {}}
113+
onCopyId={() => {}}
114+
onMove={() => {}}
115+
onTogglePin={() => {}}
116+
pinned={false}
117+
moveOptions={MOVE_OPTIONS}
118+
canEdit
119+
selectedCount={4}
120+
/>
121+
)
122+
123+
expect(menu).toContain('Move 4 items')
124+
expect(menu).toContain('Delete 4 items')
125+
expect(menu).not.toContain('Open')
126+
expect(menu).not.toContain('Rename')
127+
expect(menu).not.toContain('Copy ID')
128+
expect(menu).not.toContain('Pin')
129+
})
130+
131+
it('explains when a read-only multi-folder selection has no actions', () => {
132+
const menu = renderToStaticMarkup(
133+
<FolderContextMenu
134+
isOpen
135+
position={POSITION}
136+
onClose={() => {}}
137+
onOpen={() => {}}
138+
onRename={() => {}}
139+
onDelete={() => {}}
140+
onTogglePin={() => {}}
141+
pinned={false}
142+
canEdit={false}
143+
selectedCount={2}
144+
/>
145+
)
146+
147+
expect(menu).toContain('No actions available')
148+
expect(menu).not.toContain('Open')
149+
expect(menu).not.toContain('Delete')
150+
})
151+
152+
it('counts only the documents affected by a mixed-selection toggle', () => {
153+
const menu = renderToStaticMarkup(
154+
<DocumentContextMenu
155+
isOpen
156+
position={POSITION}
157+
onClose={() => {}}
158+
hasDocument
159+
selectedCount={25}
160+
enabledCount={7}
161+
disabledCount={18}
162+
onToggleEnabled={() => {}}
163+
onDelete={() => {}}
164+
/>
165+
)
166+
167+
expect(menu).toContain('Enable 18 items')
168+
expect(menu).toContain('Delete 25 items')
169+
})
170+
171+
it('does not overstate an unknown select-all toggle count', () => {
172+
const menu = renderToStaticMarkup(
173+
<DocumentContextMenu
174+
isOpen
175+
position={POSITION}
176+
onClose={() => {}}
177+
hasDocument
178+
selectedCount={25}
179+
enabledCount={25}
180+
disabledCount={25}
181+
hasExactToggleCount={false}
182+
onToggleEnabled={() => {}}
183+
/>
184+
)
185+
186+
expect(menu).toContain('Enable selected items')
187+
expect(menu).not.toContain('Enable 25 items')
188+
})
189+
190+
it('counts only the chunks affected by a multi-selection toggle', () => {
191+
const menu = renderToStaticMarkup(
192+
<ChunkContextMenu
193+
isOpen
194+
position={POSITION}
195+
onClose={() => {}}
196+
hasChunk
197+
selectedCount={3}
198+
enabledCount={3}
199+
onToggleEnabled={() => {}}
200+
onDelete={() => {}}
201+
/>
202+
)
203+
204+
expect(menu).toContain('Disable 3 items')
205+
expect(menu).toContain('Delete 3 items')
206+
})
207+
})

‎apps/sim/app/workspace/[workspaceId]/components/resource/selection-label.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { describe, expect, it } from 'vitest'
2+
import {
3+
selectionActionLabel,
4+
selectionLabel,
5+
selectionToggleActionLabel,
6+
} from '@/app/workspace/[workspaceId]/components/resource/selection-label'
7+
8+
describe('selection labels', () => {
9+
it('uses the selected item name for a single-row confirmation', () => {
10+
expect(selectionLabel(1, 'Quarterly data')).toBe('Quarterly data')
11+
})
12+
13+
it('uses the selection count for a multi-row confirmation', () => {
14+
expect(selectionLabel(3, 'Quarterly data')).toBe('3 selected items')
15+
})
16+
17+
it('keeps single-row action labels terse', () => {
18+
expect(selectionActionLabel('Move', 1, 'Move to')).toBe('Move to')
19+
})
20+
21+
it('states the scope of a multi-row action', () => {
22+
expect(selectionActionLabel('Delete', 3)).toBe('Delete 3 items')
23+
})
24+
25+
it('counts only disabled items for a mixed-selection enable action', () => {
26+
expect(
27+
selectionToggleActionLabel({
28+
selectedCount: 5,
29+
enabledCount: 2,
30+
disabledCount: 3,
31+
isSelectedItemEnabled: true,
32+
})
33+
).toBe('Enable 3 items')
34+
})
35+
36+
it('keeps a singular affected count visible within a larger selection', () => {
37+
expect(
38+
selectionToggleActionLabel({
39+
selectedCount: 5,
40+
enabledCount: 4,
41+
disabledCount: 1,
42+
isSelectedItemEnabled: true,
43+
})
44+
).toBe('Enable 1 item')
45+
})
46+
47+
it('counts enabled items when a selection can only be disabled', () => {
48+
expect(
49+
selectionToggleActionLabel({
50+
selectedCount: 4,
51+
enabledCount: 4,
52+
disabledCount: 0,
53+
isSelectedItemEnabled: true,
54+
})
55+
).toBe('Disable 4 items')
56+
})
57+
58+
it('keeps the action selection-aware when the affected subset count is unknown', () => {
59+
expect(
60+
selectionToggleActionLabel({
61+
selectedCount: 10,
62+
enabledCount: 10,
63+
disabledCount: 10,
64+
isSelectedItemEnabled: true,
65+
hasExactAffectedCount: false,
66+
})
67+
).toBe('Enable selected items')
68+
})
69+
})

0 commit comments

Comments
 (0)