Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ import {
} from '@sim/emcn/icons'
import type { ContextMenuState } from '../../types'

/**
* Wider than the menu's 220px default. The row-scoped workflow labels name both
* the action and the selected row count ("Run empty or failed cells on 2 rows"),
* which does not fit the default width.
*/
const CONTENT_WIDTH_CLASS = 'max-w-[320px]'

interface ContextMenuProps {
contextMenu: ContextMenuState
onClose: () => void
Expand Down Expand Up @@ -150,6 +157,7 @@ export function ContextMenu({
align='start'
side='bottom'
sideOffset={4}
className={CONTENT_WIDTH_CLASS}
onCloseAutoFocus={(e) => e.preventDefault()}
>
{onAddToChat && (
Expand Down
112 changes: 112 additions & 0 deletions packages/emcn/src/components/dropdown-menu/dropdown-menu.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/**
* @vitest-environment jsdom
*
* Menu rows are a fixed height, so a label that wraps overflows its row and paints over its
* neighbours. Rows are held to one line and their bare text is wrapped in a truncating box so an
* over-long label ellipsizes instead of being cut mid-word. These tests cover that wrapping:
* that it happens, that adjacent text stays in ONE box (two boxes would be two flex items, and
* the row's `gap` would open between the words), and that it steps aside for `asChild`, where
* Radix's `Slot` requires exactly one element child.
*/
import { act, type ReactNode } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, describe, expect, it } from 'vitest'
import {
DropdownMenu,
DropdownMenuCheckboxItem,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuTrigger,
} from './dropdown-menu'

let root: Root | null = null
let container: HTMLDivElement | null = null

function openMenu(children: ReactNode) {
;(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(
<DropdownMenu open modal={false}>
<DropdownMenuTrigger />
<DropdownMenuContent>{children}</DropdownMenuContent>
</DropdownMenu>
)
)
}

function row(selector = '[role="menuitem"]'): HTMLElement {
const node = document.querySelector(selector)
if (!node) throw new Error(`No ${selector} rendered`)
return node as HTMLElement
}

afterEach(() => {
if (root) act(() => root?.unmount())
container?.remove()
root = null
container = null
})

describe('menu row labels', () => {
it('wraps a bare text label in a truncating box', () => {
openMenu(<DropdownMenuItem>Run empty or failed cells on 2 rows</DropdownMenuItem>)

const labels = row().querySelectorAll('span')
expect(labels).toHaveLength(1)
expect(labels[0].textContent).toBe('Run empty or failed cells on 2 rows')
expect(labels[0].className).toContain('truncate')
})

it('keeps the row on one line', () => {
openMenu(<DropdownMenuItem>Delete 2 rows</DropdownMenuItem>)

expect(row().className).toContain('whitespace-nowrap')
})

it('coalesces adjacent text into a single box', () => {
const count = 2
openMenu(
<DropdownMenuItem>
<svg aria-hidden />
Delete {count} rows
</DropdownMenuItem>
)

const labels = row().querySelectorAll('span')
expect(labels).toHaveLength(1)
expect(labels[0].textContent).toBe('Delete 2 rows')
})

it('wraps a checkbox row label, leaving the check indicator its own box', () => {
openMenu(<DropdownMenuCheckboxItem checked>Show archived workflows</DropdownMenuCheckboxItem>)

const labels = row('[role="menuitemcheckbox"]').querySelectorAll('span')
const label = Array.from(labels).find((node) => node.className.includes('truncate'))
expect(label?.textContent).toBe('Show archived workflows')
})

it('leaves an asChild row alone so Slot still sees one element child', () => {
openMenu(
<DropdownMenuItem asChild>
<a href='/workflows'>Open workflow</a>
</DropdownMenuItem>
)

const link = row('a')
expect(link.textContent).toBe('Open workflow')
expect(link.querySelector('span')).toBeNull()
})

it('leaves a label the consumer already wrapped as a single box', () => {
openMenu(
<DropdownMenuItem>
<span>Add 2 rows to Chat</span>
</DropdownMenuItem>
)

expect(row().querySelectorAll('span')).toHaveLength(1)
})
})
69 changes: 59 additions & 10 deletions packages/emcn/src/components/dropdown-menu/dropdown-menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,48 @@ const ANIMATION_CLASSES =
const MENU_ROW_HEIGHT_CLASS = 'h-[28px]'
const MENU_ROW_RADIUS_CLASS = 'rounded-lg'

/**
* Rows are a fixed height, so a label that wraps overflows its row and paints
* over its neighbours instead of growing the row. Every row is therefore held
* to one line, and its label ellipsizes — see {@link withEllipsizedLabel}.
*/
const MENU_ROW_SINGLE_LINE_CLASS = 'whitespace-nowrap [&>span]:min-w-0 [&>span]:truncate'

/**
* Wraps a row's bare text children in a truncating box so a label wider than
* the menu ends in an ellipsis rather than being cut mid-word at the surface
* edge. Consumers that already wrap their label in a `<span>` are unaffected —
* the row's `[&>span]` rule truncates those in place.
*
* Adjacent text is coalesced into a single box: a row is a flex container, so
* wrapping `Insert row {n}` as two boxes would make them two flex items and
* open the row's `gap` between the words. `React.Children.toArray` keys the
* element children it returns, so the rebuilt array needs no keys of its own.
*/
function withEllipsizedLabel(children: React.ReactNode): React.ReactNode {
const rebuilt: React.ReactNode[] = []
let text: React.ReactNode[] = []
const flushText = () => {
if (text.length === 0) return
rebuilt.push(
<span key={`label-${rebuilt.length}`} className='min-w-0 truncate'>
{text}
</span>
)
text = []
}
for (const child of React.Children.toArray(children)) {
if (typeof child === 'string' || typeof child === 'number') {
text.push(child)
continue
}
flushText()
rebuilt.push(child)
}
flushText()
return rebuilt
}

/**
* Surface corner, shared by the root menu and submenus — they previously
* disagreed, at 12px and 8px.
Expand Down Expand Up @@ -110,13 +152,13 @@ const DropdownMenuSubTrigger = React.forwardRef<
<DropdownMenuPrimitive.SubTrigger
ref={ref}
className={cn(
`flex ${MENU_ROW_HEIGHT_CLASS} min-w-0 cursor-default select-none items-center gap-2 ${MENU_ROW_RADIUS_CLASS} px-2 text-[var(--text-body)] text-small outline-none transition-colors focus:bg-[var(--surface-active)] data-[state=open]:bg-[var(--surface-active)] [&>span]:min-w-0 [&>span]:truncate [&_svg]:pointer-events-none [&_svg]:size-[14px] [&_svg]:shrink-0 [&_svg]:text-[var(--text-icon)]`,
`flex ${MENU_ROW_HEIGHT_CLASS} min-w-0 cursor-default select-none items-center gap-2 ${MENU_ROW_RADIUS_CLASS} px-2 text-[var(--text-body)] text-small outline-none transition-colors focus:bg-[var(--surface-active)] data-[state=open]:bg-[var(--surface-active)] ${MENU_ROW_SINGLE_LINE_CLASS} [&_svg]:pointer-events-none [&_svg]:size-[14px] [&_svg]:shrink-0 [&_svg]:text-[var(--text-icon)]`,
inset && 'pl-7',
className
)}
{...props}
>
{children}
{withEllipsizedLabel(children)}
<ChevronRight className='ml-auto size-[14px] shrink-0' />
</DropdownMenuPrimitive.SubTrigger>
)
Expand Down Expand Up @@ -172,7 +214,7 @@ const DropdownMenuContent = React.forwardRef<
))
DropdownMenuContent.displayName = DropdownMenuPrimitive.Content.displayName

const DROPDOWN_MENU_ITEM_BASE_CLASSES = `relative flex ${MENU_ROW_HEIGHT_CLASS} min-w-0 cursor-pointer select-none items-center gap-2 ${MENU_ROW_RADIUS_CLASS} px-2 text-[var(--text-body)] text-small outline-none transition-colors focus:bg-[var(--surface-active)] data-[disabled]:pointer-events-none data-[disabled]:opacity-50 [&>span]:min-w-0 [&>span]:truncate [&_svg]:pointer-events-none [&_svg]:size-[14px] [&_svg]:shrink-0 [&_svg]:text-[var(--text-icon)]`
const DROPDOWN_MENU_ITEM_BASE_CLASSES = `relative flex ${MENU_ROW_HEIGHT_CLASS} min-w-0 cursor-pointer select-none items-center gap-2 ${MENU_ROW_RADIUS_CLASS} px-2 text-[var(--text-body)] text-small outline-none transition-colors focus:bg-[var(--surface-active)] data-[disabled]:pointer-events-none data-[disabled]:opacity-50 ${MENU_ROW_SINGLE_LINE_CLASS} [&_svg]:pointer-events-none [&_svg]:size-[14px] [&_svg]:shrink-0 [&_svg]:text-[var(--text-icon)]`

const DropdownMenuItem = React.forwardRef<
React.ElementRef<typeof DropdownMenuPrimitive.Item>,
Expand All @@ -185,7 +227,8 @@ const DropdownMenuItem = React.forwardRef<
*/
action?: React.ReactNode
}
>(({ className, inset, action, ...props }, ref) => {
>(({ className, inset, action, asChild, children, ...props }, ref) => {
const content = asChild ? children : withEllipsizedLabel(children)
if (action) {
return (
<div className='group/dropdownitem relative'>
Expand All @@ -197,8 +240,11 @@ const DropdownMenuItem = React.forwardRef<
inset && 'pl-7',
className
)}
asChild={asChild}
{...props}
/>
>
{content}
</DropdownMenuPrimitive.Item>
<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'>
{action}
</div>
Expand All @@ -209,8 +255,11 @@ const DropdownMenuItem = React.forwardRef<
<DropdownMenuPrimitive.Item
ref={ref}
className={cn(DROPDOWN_MENU_ITEM_BASE_CLASSES, inset && 'pl-7', className)}
asChild={asChild}
{...props}
/>
>
{content}
</DropdownMenuPrimitive.Item>
)
})
DropdownMenuItem.displayName = DropdownMenuPrimitive.Item.displayName
Expand Down Expand Up @@ -252,7 +301,7 @@ const DropdownMenuCheckboxItem = React.forwardRef<
<DropdownMenuPrimitive.CheckboxItem
Comment thread
waleedlatif1 marked this conversation as resolved.
ref={ref}
className={cn(
`relative flex ${MENU_ROW_HEIGHT_CLASS} cursor-default select-none items-center ${MENU_ROW_RADIUS_CLASS} 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`,
`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`,
className
)}
checked={checked}
Expand All @@ -263,7 +312,7 @@ const DropdownMenuCheckboxItem = React.forwardRef<
<Check className='size-[14px]' />
</DropdownMenuPrimitive.ItemIndicator>
</span>
{children}
{withEllipsizedLabel(children)}
</DropdownMenuPrimitive.CheckboxItem>
))
DropdownMenuCheckboxItem.displayName = DropdownMenuPrimitive.CheckboxItem.displayName
Expand All @@ -275,7 +324,7 @@ const DropdownMenuRadioItem = React.forwardRef<
<DropdownMenuPrimitive.RadioItem
ref={ref}
className={cn(
`relative flex ${MENU_ROW_HEIGHT_CLASS} cursor-default select-none items-center ${MENU_ROW_RADIUS_CLASS} 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`,
`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`,
className
)}
{...props}
Expand All @@ -285,7 +334,7 @@ const DropdownMenuRadioItem = React.forwardRef<
<Circle className='size-[6px] fill-current' />
</DropdownMenuPrimitive.ItemIndicator>
</span>
{children}
{withEllipsizedLabel(children)}
</DropdownMenuPrimitive.RadioItem>
))
DropdownMenuRadioItem.displayName = DropdownMenuPrimitive.RadioItem.displayName
Expand Down
Loading