Skip to content

Commit 963be2d

Browse files
committed
improvement(emcn): ellipsize menu row labels instead of clipping them
1 parent 27f328f commit 963be2d

3 files changed

Lines changed: 177 additions & 21 deletions

File tree

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ import {
1919
} from '@sim/emcn/icons'
2020
import type { ContextMenuState } from '../../types'
2121

22+
/**
23+
* Wider than the menu's 220px default. The row-scoped workflow labels name both
24+
* the action and the selected row count ("Run empty or failed cells on 2 rows"),
25+
* which does not fit the default width.
26+
*/
27+
const CONTENT_WIDTH_CLASS = 'max-w-[320px]'
28+
2229
interface ContextMenuProps {
2330
contextMenu: ContextMenuState
2431
onClose: () => void
@@ -146,22 +153,18 @@ export function ContextMenu({
146153
aria-hidden
147154
/>
148155
</DropdownMenuTrigger>
149-
{/* Wider than the 220px menu default: the row-scoped workflow labels name
150-
both the action and the selected row count ("Run empty or failed cells
151-
on 2 rows"), which wrapped — and so overlapped the rows beneath — at
152-
the default width. */}
153156
<DropdownMenuContent
154157
align='start'
155158
side='bottom'
156159
sideOffset={4}
157-
className='max-w-[320px]'
160+
className={CONTENT_WIDTH_CLASS}
158161
onCloseAutoFocus={(e) => e.preventDefault()}
159162
>
160163
{onAddToChat && (
161164
<>
162165
<DropdownMenuItem onSelect={onAddToChat}>
163166
<Blimp />
164-
<span>{addToChatLabel}</span>
167+
{addToChatLabel}
165168
</DropdownMenuItem>
166169
<DropdownMenuSeparator />
167170
</>
@@ -184,19 +187,19 @@ export function ContextMenu({
184187
{hasWorkflowColumns && onRunWorkflows && (
185188
<DropdownMenuItem onSelect={onRunWorkflows}>
186189
<PlayOutline />
187-
<span>{runLabel}</span>
190+
{runLabel}
188191
</DropdownMenuItem>
189192
)}
190193
{hasWorkflowColumns && onRefreshWorkflows && (
191194
<DropdownMenuItem onSelect={onRefreshWorkflows}>
192195
<RefreshCw />
193-
<span>{refreshLabel}</span>
196+
{refreshLabel}
194197
</DropdownMenuItem>
195198
)}
196199
{hasWorkflowColumns && onStopWorkflows && runningInSelectionCount > 0 && (
197200
<DropdownMenuItem onSelect={onStopWorkflows}>
198201
<Square className='size-[14px] text-[var(--text-icon)]' />
199-
<span>{stopLabel}</span>
202+
{stopLabel}
200203
</DropdownMenuItem>
201204
)}
202205
<DropdownMenuItem disabled={disableInsert} onSelect={onInsertAbove}>
@@ -217,7 +220,7 @@ export function ContextMenu({
217220
<DropdownMenuSeparator />
218221
<DropdownMenuItem disabled={disableDelete} onSelect={onDelete}>
219222
<Trash />
220-
<span>{deleteLabel}</span>
223+
{deleteLabel}
221224
</DropdownMenuItem>
222225
</DropdownMenuContent>
223226
</DropdownMenu>
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*
4+
* Menu rows are a fixed height, so a label that wraps overflows its row and paints over its
5+
* neighbours. Rows are held to one line and their bare text is wrapped in a truncating box so an
6+
* over-long label ellipsizes instead of being cut mid-word. These tests cover that wrapping:
7+
* that it happens, that adjacent text stays in ONE box (two boxes would be two flex items, and
8+
* the row's `gap` would open between the words), and that it steps aside for `asChild`, where
9+
* Radix's `Slot` requires exactly one element child.
10+
*/
11+
import { act, type ReactNode } from 'react'
12+
import { createRoot, type Root } from 'react-dom/client'
13+
import { afterEach, describe, expect, it } from 'vitest'
14+
import {
15+
DropdownMenu,
16+
DropdownMenuCheckboxItem,
17+
DropdownMenuContent,
18+
DropdownMenuItem,
19+
DropdownMenuTrigger,
20+
} from './dropdown-menu'
21+
22+
let root: Root | null = null
23+
let container: HTMLDivElement | null = null
24+
25+
function openMenu(children: ReactNode) {
26+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
27+
container = document.createElement('div')
28+
document.body.appendChild(container)
29+
root = createRoot(container)
30+
act(() =>
31+
root?.render(
32+
<DropdownMenu open modal={false}>
33+
<DropdownMenuTrigger />
34+
<DropdownMenuContent>{children}</DropdownMenuContent>
35+
</DropdownMenu>
36+
)
37+
)
38+
}
39+
40+
function row(selector = '[role="menuitem"]'): HTMLElement {
41+
const node = document.querySelector(selector)
42+
if (!node) throw new Error(`No ${selector} rendered`)
43+
return node as HTMLElement
44+
}
45+
46+
afterEach(() => {
47+
if (root) act(() => root?.unmount())
48+
container?.remove()
49+
root = null
50+
container = null
51+
})
52+
53+
describe('menu row labels', () => {
54+
it('wraps a bare text label in a truncating box', () => {
55+
openMenu(<DropdownMenuItem>Run empty or failed cells on 2 rows</DropdownMenuItem>)
56+
57+
const labels = row().querySelectorAll('span')
58+
expect(labels).toHaveLength(1)
59+
expect(labels[0].textContent).toBe('Run empty or failed cells on 2 rows')
60+
expect(labels[0].className).toContain('truncate')
61+
})
62+
63+
it('keeps the row on one line', () => {
64+
openMenu(<DropdownMenuItem>Delete 2 rows</DropdownMenuItem>)
65+
66+
expect(row().className).toContain('whitespace-nowrap')
67+
})
68+
69+
it('coalesces adjacent text into a single box', () => {
70+
const count = 2
71+
openMenu(
72+
<DropdownMenuItem>
73+
<svg aria-hidden />
74+
Delete {count} rows
75+
</DropdownMenuItem>
76+
)
77+
78+
const labels = row().querySelectorAll('span')
79+
expect(labels).toHaveLength(1)
80+
expect(labels[0].textContent).toBe('Delete 2 rows')
81+
})
82+
83+
it('wraps a checkbox row label, leaving the check indicator its own box', () => {
84+
openMenu(<DropdownMenuCheckboxItem checked>Show archived workflows</DropdownMenuCheckboxItem>)
85+
86+
const labels = row('[role="menuitemcheckbox"]').querySelectorAll('span')
87+
const label = Array.from(labels).find((node) => node.className.includes('truncate'))
88+
expect(label?.textContent).toBe('Show archived workflows')
89+
})
90+
91+
it('leaves an asChild row alone so Slot still sees one element child', () => {
92+
openMenu(
93+
<DropdownMenuItem asChild>
94+
<a href='/workflows'>Open workflow</a>
95+
</DropdownMenuItem>
96+
)
97+
98+
const link = row('a')
99+
expect(link.textContent).toBe('Open workflow')
100+
expect(link.querySelector('span')).toBeNull()
101+
})
102+
103+
it('leaves a label the consumer already wrapped as a single box', () => {
104+
openMenu(
105+
<DropdownMenuItem>
106+
<span>Add 2 rows to Chat</span>
107+
</DropdownMenuItem>
108+
)
109+
110+
expect(row().querySelectorAll('span')).toHaveLength(1)
111+
})
112+
})

packages/emcn/src/components/dropdown-menu/dropdown-menu.tsx

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,46 @@ const MENU_ROW_RADIUS_CLASS = 'rounded-lg'
4242

4343
/**
4444
* Rows are a fixed height, so a label that wraps overflows its row and paints
45-
* over its neighbours instead of growing the row. Keep every row on one line:
46-
* a bare text label is clipped at the surface edge, and a label wrapped in a
47-
* `<span>` (the shape long or count-bearing labels should use) ellipsizes.
45+
* over its neighbours instead of growing the row. Every row is therefore held
46+
* to one line, and its label ellipsizes — see {@link withEllipsizedLabel}.
4847
*/
4948
const MENU_ROW_SINGLE_LINE_CLASS = 'whitespace-nowrap [&>span]:min-w-0 [&>span]:truncate'
5049

50+
/**
51+
* Wraps a row's bare text children in a truncating box so a label wider than
52+
* the menu ends in an ellipsis rather than being cut mid-word at the surface
53+
* edge. Consumers that already wrap their label in a `<span>` are unaffected —
54+
* the row's `[&>span]` rule truncates those in place.
55+
*
56+
* Adjacent text is coalesced into a single box: a row is a flex container, so
57+
* wrapping `Insert row {n}` as two boxes would make them two flex items and
58+
* open the row's `gap` between the words. `React.Children.toArray` keys the
59+
* element children it returns, so the rebuilt array needs no keys of its own.
60+
*/
61+
function withEllipsizedLabel(children: React.ReactNode): React.ReactNode {
62+
const rebuilt: React.ReactNode[] = []
63+
let text: React.ReactNode[] = []
64+
const flushText = () => {
65+
if (text.length === 0) return
66+
rebuilt.push(
67+
<span key={`label-${rebuilt.length}`} className='min-w-0 truncate'>
68+
{text}
69+
</span>
70+
)
71+
text = []
72+
}
73+
for (const child of React.Children.toArray(children)) {
74+
if (typeof child === 'string' || typeof child === 'number') {
75+
text.push(child)
76+
continue
77+
}
78+
flushText()
79+
rebuilt.push(child)
80+
}
81+
flushText()
82+
return rebuilt
83+
}
84+
5185
/**
5286
* Surface corner, shared by the root menu and submenus — they previously
5387
* disagreed, at 12px and 8px.
@@ -124,7 +158,7 @@ const DropdownMenuSubTrigger = React.forwardRef<
124158
)}
125159
{...props}
126160
>
127-
{children}
161+
{withEllipsizedLabel(children)}
128162
<ChevronRight className='ml-auto size-[14px] shrink-0' />
129163
</DropdownMenuPrimitive.SubTrigger>
130164
)
@@ -193,7 +227,8 @@ const DropdownMenuItem = React.forwardRef<
193227
*/
194228
action?: React.ReactNode
195229
}
196-
>(({ className, inset, action, ...props }, ref) => {
230+
>(({ className, inset, action, asChild, children, ...props }, ref) => {
231+
const content = asChild ? children : withEllipsizedLabel(children)
197232
if (action) {
198233
return (
199234
<div className='group/dropdownitem relative'>
@@ -205,8 +240,11 @@ const DropdownMenuItem = React.forwardRef<
205240
inset && 'pl-7',
206241
className
207242
)}
243+
asChild={asChild}
208244
{...props}
209-
/>
245+
>
246+
{content}
247+
</DropdownMenuPrimitive.Item>
210248
<div className='-translate-y-1/2 absolute top-1/2 right-1 flex items-center opacity-0 transition-opacity group-focus-within/dropdownitem:opacity-100 group-hover/dropdownitem:opacity-100'>
211249
{action}
212250
</div>
@@ -217,8 +255,11 @@ const DropdownMenuItem = React.forwardRef<
217255
<DropdownMenuPrimitive.Item
218256
ref={ref}
219257
className={cn(DROPDOWN_MENU_ITEM_BASE_CLASSES, inset && 'pl-7', className)}
258+
asChild={asChild}
220259
{...props}
221-
/>
260+
>
261+
{content}
262+
</DropdownMenuPrimitive.Item>
222263
)
223264
})
224265
DropdownMenuItem.displayName = DropdownMenuPrimitive.Item.displayName
@@ -260,7 +301,7 @@ const DropdownMenuCheckboxItem = React.forwardRef<
260301
<DropdownMenuPrimitive.CheckboxItem
261302
ref={ref}
262303
className={cn(
263-
`relative flex ${MENU_ROW_HEIGHT_CLASS} cursor-default select-none items-center ${MENU_ROW_RADIUS_CLASS} whitespace-nowrap pr-2 pl-7 text-[var(--text-body)] text-small outline-none transition-colors focus:bg-[var(--surface-active)] data-[disabled]:pointer-events-none data-[disabled]:opacity-50`,
304+
`relative flex ${MENU_ROW_HEIGHT_CLASS} min-w-0 cursor-default select-none items-center ${MENU_ROW_RADIUS_CLASS} whitespace-nowrap pr-2 pl-7 text-[var(--text-body)] text-small outline-none transition-colors focus:bg-[var(--surface-active)] data-[disabled]:pointer-events-none data-[disabled]:opacity-50`,
264305
className
265306
)}
266307
checked={checked}
@@ -271,7 +312,7 @@ const DropdownMenuCheckboxItem = React.forwardRef<
271312
<Check className='size-[14px]' />
272313
</DropdownMenuPrimitive.ItemIndicator>
273314
</span>
274-
{children}
315+
{withEllipsizedLabel(children)}
275316
</DropdownMenuPrimitive.CheckboxItem>
276317
))
277318
DropdownMenuCheckboxItem.displayName = DropdownMenuPrimitive.CheckboxItem.displayName
@@ -283,7 +324,7 @@ const DropdownMenuRadioItem = React.forwardRef<
283324
<DropdownMenuPrimitive.RadioItem
284325
ref={ref}
285326
className={cn(
286-
`relative flex ${MENU_ROW_HEIGHT_CLASS} cursor-default select-none items-center ${MENU_ROW_RADIUS_CLASS} whitespace-nowrap pr-2 pl-7 text-[var(--text-body)] text-small outline-none transition-colors focus:bg-[var(--surface-active)] data-[disabled]:pointer-events-none data-[disabled]:opacity-50`,
327+
`relative flex ${MENU_ROW_HEIGHT_CLASS} min-w-0 cursor-default select-none items-center ${MENU_ROW_RADIUS_CLASS} whitespace-nowrap pr-2 pl-7 text-[var(--text-body)] text-small outline-none transition-colors focus:bg-[var(--surface-active)] data-[disabled]:pointer-events-none data-[disabled]:opacity-50`,
287328
className
288329
)}
289330
{...props}
@@ -293,7 +334,7 @@ const DropdownMenuRadioItem = React.forwardRef<
293334
<Circle className='size-[6px] fill-current' />
294335
</DropdownMenuPrimitive.ItemIndicator>
295336
</span>
296-
{children}
337+
{withEllipsizedLabel(children)}
297338
</DropdownMenuPrimitive.RadioItem>
298339
))
299340
DropdownMenuRadioItem.displayName = DropdownMenuPrimitive.RadioItem.displayName

0 commit comments

Comments
 (0)