Skip to content

Radix dropdown -> Headless dropdown #2452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Sep 18, 2024
4 changes: 2 additions & 2 deletions app/components/MoreActionsMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { More12Icon } from '@oxide/design-system/icons/react'

import type { MenuAction } from '~/table/columns/action-col'
import { DropdownMenu } from '~/ui/lib/DropdownMenu'
import * as DropdownMenu from '~/ui/lib/DropdownMenu'
import { Tooltip } from '~/ui/lib/Tooltip'
import { Wrap } from '~/ui/util/wrap'

Expand All @@ -26,7 +26,7 @@ export const MoreActionsMenu = ({ actions, label }: MoreActionsMenuProps) => {
>
<More12Icon className="text-tertiary" />
</DropdownMenu.Trigger>
<DropdownMenu.Content align="end" className="mt-2">
<DropdownMenu.Content className="mt-2">
{actions.map((a) => (
<Wrap key={a.label} when={!!a.disabled} with={<Tooltip content={a.disabled} />}>
<DropdownMenu.Item
Expand Down
35 changes: 16 additions & 19 deletions app/components/TopBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@
*
* Copyright Oxide Computer Company
*/
import cn from 'classnames'
import React from 'react'

import { navToLogin, useApiMutation } from '@oxide/api'
import { DirectionDownIcon, Profile16Icon } from '@oxide/design-system/icons/react'

import { useCurrentUser } from '~/layouts/AuthenticatedLayout'
import { Button } from '~/ui/lib/Button'
import { DropdownMenu } from '~/ui/lib/DropdownMenu'
import { buttonStyle } from '~/ui/lib/Button'
import * as DropdownMenu from '~/ui/lib/DropdownMenu'
import { pb } from '~/util/path-builder'

export function TopBar({ children }: { children: React.ReactNode }) {
Expand Down Expand Up @@ -40,25 +41,21 @@ export function TopBar({ children }: { children: React.ReactNode }) {
<div className="mx-3 flex h-[60px] shrink-0 items-center justify-between">
<div className="flex items-center">{otherPickers}</div>
<div className="flex items-center gap-2">
{/* <Button variant="secondary" size="icon" className="ml-2" title="Notifications">
<Notifications16Icon className="text-quaternary" />
</Button> */}
<DropdownMenu.Root>
<DropdownMenu.Trigger asChild>
<Button
size="sm"
variant="secondary"
aria-label="User menu"
innerClassName="space-x-2"
>
<Profile16Icon className="text-quaternary" />
<span className="normal-case text-sans-md text-secondary">
{me.displayName || 'User'}
</span>
<DirectionDownIcon className="!w-2.5" />
</Button>
<DropdownMenu.Trigger
className={cn(
buttonStyle({ size: 'sm', variant: 'secondary' }),
'flex items-center gap-2'
)}
aria-label="User menu"
>
<Profile16Icon className="text-quaternary" />
<span className="normal-case text-sans-md text-secondary">
{me.displayName || 'User'}
</span>
<DirectionDownIcon className="!w-2.5" />
</DropdownMenu.Trigger>
<DropdownMenu.Content align="end" sideOffset={8}>
<DropdownMenu.Content gap={8} className="!z-topBarPopover">
<DropdownMenu.LinkItem to={pb.profile()}>Settings</DropdownMenu.LinkItem>
<DropdownMenu.Item onSelect={() => logout.mutate({})}>
Sign out
Expand Down
78 changes: 38 additions & 40 deletions app/components/TopBarPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import {
} from '~/hooks/use-params'
import { useCurrentUser } from '~/layouts/AuthenticatedLayout'
import { PAGE_SIZE } from '~/table/QueryTable'
import { Button } from '~/ui/lib/Button'
import { DropdownMenu } from '~/ui/lib/DropdownMenu'
import { buttonStyle } from '~/ui/lib/Button'
import * as DropdownMenu from '~/ui/lib/DropdownMenu'
import { Identicon } from '~/ui/lib/Identicon'
import { Wrap } from '~/ui/util/wrap'
import { pb } from '~/util/path-builder'
Expand Down Expand Up @@ -95,53 +95,51 @@ const TopBarPicker = (props: TopBarPickerProps) => {
{props.items && (
<div className="ml-2 shrink-0">
<DropdownMenu.Trigger
className="group"
className={cn(
'group h-[2rem] w-[1.125rem]',
buttonStyle({ size: 'icon', variant: 'ghost' })
)}
aria-label={props['aria-label']}
asChild
>
<Button size="icon" variant="ghost" className="h-[2rem] w-[1.125rem]">
{/* aria-hidden is a tip from the Reach docs */}
<SelectArrows6Icon className="text-secondary" aria-hidden />
</Button>
{/* aria-hidden is a tip from the Reach docs */}
<SelectArrows6Icon className="text-secondary" aria-hidden />
</DropdownMenu.Trigger>
</div>
)}
</div>
{/* TODO: item size and focus highlight */}
{/* TODO: popover position should be further right */}
{props.items && (
// portal is necessary to avoid the menu popover getting its own after:
// separator thing
<DropdownMenu.Portal>
<DropdownMenu.Content
className="mt-2 max-h-80 min-w-[12.8125rem] overflow-y-auto"
align="start"
>
{props.items.length > 0 ? (
props.items.map(({ label, to }) => {
const isSelected = props.current === label
return (
<DropdownMenu.Item asChild key={label}>
<Link to={to} className={cn({ 'is-selected': isSelected })}>
<span className="flex w-full items-center gap-2">
{label}
{isSelected && <Success12Icon className="-mr-3 block" />}
</span>
</Link>
</DropdownMenu.Item>
)
})
) : (
<DropdownMenu.Item
className="!pr-3 !text-center !text-secondary hover:cursor-default"
onSelect={() => {}}
disabled
>
{props.noItemsText || 'No items found'}
</DropdownMenu.Item>
)}
</DropdownMenu.Content>
</DropdownMenu.Portal>
<DropdownMenu.Content
className="!z-topBarPopover mt-2 max-h-80 min-w-[12.8125rem] overflow-y-auto"
anchor="bottom start"
>
{props.items.length > 0 ? (
props.items.map(({ label, to }) => {
const isSelected = props.current === label
return (
<DropdownMenu.LinkItem
key={label}
to={to}
className={cn({ 'is-selected': isSelected })}
>
<span className="flex w-full items-center gap-2">
{label}
{isSelected && <Success12Icon className="-mr-3 block" />}
</span>
</DropdownMenu.LinkItem>
)
})
) : (
<DropdownMenu.Item
className="!pr-3 !text-center !text-secondary hover:cursor-default"
onSelect={() => {}}
disabled
>
{props.noItemsText || 'No items found'}
</DropdownMenu.Item>
)}
</DropdownMenu.Content>
)}
</DropdownMenu.Root>
)
Expand Down
65 changes: 31 additions & 34 deletions app/table/columns/action-col.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { useMemo } from 'react'

import { More12Icon } from '@oxide/design-system/icons/react'

import { DropdownMenu } from '~/ui/lib/DropdownMenu'
import * as DropdownMenu from '~/ui/lib/DropdownMenu'
import { Tooltip } from '~/ui/lib/Tooltip'
import { Wrap } from '~/ui/util/wrap'
import { kebabCase } from '~/util/str'
Expand Down Expand Up @@ -75,41 +75,38 @@ export const RowActions = ({ id, copyIdLabel = 'Copy ID', actions }: RowActionsP
>
<More12Icon className="text-tertiary" />
</DropdownMenu.Trigger>
{/* portal fixes mysterious z-index issue where menu is behind button */}
<DropdownMenu.Portal>
<DropdownMenu.Content align="end" className="-mt-3 mr-2">
{id && (
<DropdownMenu.Item
onSelect={() => {
window.navigator.clipboard.writeText(id)
}}
{/* offset moves menu in from the right so it doesn't align with the table border */}
<DropdownMenu.Content anchor={{ to: 'bottom end', offset: -6 }} className="-mt-2">
{id && (
<DropdownMenu.Item
onSelect={() => {
window.navigator.clipboard.writeText(id)
}}
>
{copyIdLabel}
</DropdownMenu.Item>
)}
{actions?.map((action) => {
// TODO: Tooltip on disabled button broke, probably due to portal
return (
<Wrap
when={!!action.disabled}
with={<Tooltip content={action.disabled} />}
key={kebabCase(`action-${action.label}`)}
>
{copyIdLabel}
</DropdownMenu.Item>
)}
{actions?.map((action) => {
// TODO: Tooltip on disabled button broke, probably due to portal
return (
<Wrap
when={!!action.disabled}
with={<Tooltip content={action.disabled} />}
key={kebabCase(`action-${action.label}`)}
<DropdownMenu.Item
className={cn(action.className, {
destructive: action.label.toLowerCase() === 'delete' && !action.disabled,
})}
onSelect={action.onActivate}
disabled={!!action.disabled}
>
<DropdownMenu.Item
className={cn(action.className, {
destructive:
action.label.toLowerCase() === 'delete' && !action.disabled,
})}
onSelect={action.onActivate}
disabled={!!action.disabled}
>
{action.label}
</DropdownMenu.Item>
</Wrap>
)
})}
</DropdownMenu.Content>
</DropdownMenu.Portal>
{action.label}
</DropdownMenu.Item>
</Wrap>
)
})}
</DropdownMenu.Content>
</DropdownMenu.Root>
)
}
108 changes: 69 additions & 39 deletions app/ui/lib/DropdownMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,47 +5,77 @@
*
* Copyright Oxide Computer Company
*/

import {
Content,
Item,
Portal,
Root,
Trigger,
type DropdownMenuContentProps,
type DropdownMenuItemProps,
} from '@radix-ui/react-dropdown-menu'
Menu,
MenuButton,
MenuItem,
MenuItems,
type MenuItemsProps,
} from '@headlessui/react'
import cn from 'classnames'
import { forwardRef, type ForwardedRef } from 'react'
import { forwardRef, type ForwardedRef, type ReactNode } from 'react'
import { Link } from 'react-router-dom'

type DivRef = ForwardedRef<HTMLDivElement>

// remove possibility of disabling links for now. if we put it back, make sure
// to forwardRef on LinkItem so the disabled tooltip can work
type LinkitemProps = Omit<DropdownMenuItemProps, 'disabled'> & { to: string }

export const DropdownMenu = {
Root,
Trigger,
Portal,
// don't need to forward ref here for a particular reason but Radix gives a
// big angry warning if we don't
Content: forwardRef(({ className, ...props }: DropdownMenuContentProps, ref: DivRef) => (
<Content
{...props}
// prevents focus ring showing up on trigger when you close the menu
onCloseAutoFocus={(e) => e.preventDefault()}
className={cn('DropdownMenuContent', className)}
ref={ref}
/>
)),
// need to forward ref because of tooltips on disabled menu buttons
Item: forwardRef(({ className, ...props }: DropdownMenuItemProps, ref: DivRef) => (
<Item {...props} className={cn('DropdownMenuItem ox-menu-item', className)} ref={ref} />
)),
LinkItem: ({ className, children, to, ...props }: LinkitemProps) => (
<Item {...props} className={cn('DropdownMenuItem ox-menu-item', className)} asChild>
<Link to={to}>{children}</Link>
</Item>
),
export const Root = Menu

export const Trigger = MenuButton

type ContentProps = {
className?: string
children: ReactNode
anchor?: MenuItemsProps['anchor']
/** Spacing in px, passed as --anchor-gap */
gap?: 8
}

export function Content({ className, children, anchor = 'bottom end', gap }: ContentProps) {
return (
<MenuItems
anchor={anchor}
// goofy gap because tailwind hates string interpolation
className={cn('DropdownMenuContent', gap === 8 && `[--anchor-gap:8px]`, className)}
// necessary to turn off scroll locking so the scrollbar doesn't pop in
// and out as menu closes and opens
modal={false}
>
{children}
</MenuItems>
)
}

type LinkItemProps = { className?: string; to: string; children: ReactNode }

export function LinkItem({ className, to, children }: LinkItemProps) {
return (
<MenuItem>
<Link className={cn('DropdownMenuItem ox-menu-item', className)} to={to}>
{children}
</Link>
</MenuItem>
)
}

type ButtonRef = ForwardedRef<HTMLButtonElement>
type ItemProps = {
className?: string
onSelect?: () => void
children: ReactNode
disabled?: boolean
}

// need to forward ref because of tooltips on disabled menu buttons
export const Item = forwardRef(
({ className, onSelect, children, disabled }: ItemProps, ref: ButtonRef) => (
<MenuItem disabled={disabled}>
<button
type="button"
className={cn('DropdownMenuItem ox-menu-item', className)}
ref={ref}
onClick={onSelect}
>
{children}
</button>
</MenuItem>
)
)
7 changes: 3 additions & 4 deletions app/ui/styles/components/menu-button.css
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
*/

.DropdownMenuContent {
@apply z-30 min-w-36 rounded border p-0 bg-raise border-secondary;
@apply z-popover min-w-36 rounded border p-0 bg-raise border-secondary;

& .DropdownMenuItem {
@apply block cursor-pointer select-none border-b py-2 pl-3 pr-6 text-left text-sans-md text-secondary border-secondary last:border-b-0;
@apply block w-full cursor-pointer select-none border-b py-2 pl-3 pr-6 text-left text-sans-md text-secondary border-secondary last:border-b-0;

&.destructive {
@apply text-destructive;
Expand All @@ -24,8 +24,7 @@
@apply text-destructive-disabled;
}

&[data-highlighted] {
/* background: hsl(211, 81%, 36%); */
&[data-focus] {
outline: none;
@apply bg-tertiary;
}
Expand Down
Loading
Loading