Radix dropdown -> Headless dropdown (#2452)

david-crespo · github-actions[bot] · web-flow · commit e3d424599f5c · 2024-09-18T17:48:44.000Z
* use headless dropdown menu for user menu in topbar

* extract DropdownMenu and convert TopBarPicker

* convert the last two dropdowns and uninstall @radix-ui/react-dropdown-menu

* fix menu item width and hover bg color

* Bot commit: format with prettier

* fix menu position on more actions column

* remove done todo

* use z-popover (10) on menu contents and add z-topBarPopover (25)

* remove unused portal prop and remove an unnecessary anchor prop

---------

Co-authored-by: github-actions &lt;41898282+github-actions[bot]@users.noreply.github.com&gt;
diff --git a/app/components/MoreActionsMenu.tsx b/app/components/MoreActionsMenu.tsx
@@ -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'
 
@@ -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
diff --git a/app/components/TopBar.tsx b/app/components/TopBar.tsx
@@ -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 }) {
@@ -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
diff --git a/app/components/TopBarPicker.tsx b/app/components/TopBarPicker.tsx
@@ -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'
@@ -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>
   )
diff --git a/app/table/columns/action-col.tsx b/app/table/columns/action-col.tsx
@@ -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'
@@ -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>
   )
 }
diff --git a/app/ui/lib/DropdownMenu.tsx b/app/ui/lib/DropdownMenu.tsx
@@ -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>
+  )
+)
diff --git a/app/ui/styles/components/menu-button.css b/app/ui/styles/components/menu-button.css
@@ -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;
@@ -24,8 +24,7 @@
       @apply text-destructive-disabled;
     }
 
-    &[data-highlighted] {
-      /* background: hsl(211, 81%, 36%); */
+    &[data-focus] {
       outline: none;
       @apply bg-tertiary;
     }
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
diff --git a/tailwind.config.js b/tailwind.config.js
