From a8b2c56f4bf332c958e2a191c7d5c7181114a91b Mon Sep 17 00:00:00 2001
From: Emmanuel Pelletier <manu@habite.la>
Date: Mon, 5 Aug 2024 17:18:14 +0200
Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(frontend)=20new=20Menu=20component?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ditch the "PopoverList" component that was basically a poor Menu. Now
dropdown buttons can be made with react aria Menu+MenuItems components
via the Menu+MenuList components.

The difference now is dropdown menus behave better with keyboard (they
use up/down arrows instead of tabs), like selects.

Popovers are there if we need to show any content in a big tooltip, not
for actionable list items.
---
 src/frontend/src/layout/Header.tsx            | 14 ++--
 src/frontend/src/primitives/Menu.tsx          | 25 +++++++
 src/frontend/src/primitives/MenuList.tsx      | 44 ++++++++++++
 src/frontend/src/primitives/Popover.tsx       |  5 +-
 src/frontend/src/primitives/PopoverList.tsx   | 72 -------------------
 src/frontend/src/primitives/Select.tsx        | 22 +-----
 src/frontend/src/primitives/index.ts          |  9 ++-
 src/frontend/src/primitives/menuItemStyles.ts | 31 ++++++++
 8 files changed, 121 insertions(+), 101 deletions(-)
 create mode 100644 src/frontend/src/primitives/Menu.tsx
 create mode 100644 src/frontend/src/primitives/MenuList.tsx
 delete mode 100644 src/frontend/src/primitives/PopoverList.tsx
 create mode 100644 src/frontend/src/primitives/menuItemStyles.ts

diff --git a/src/frontend/src/layout/Header.tsx b/src/frontend/src/layout/Header.tsx
index 25eae694..2fe8348d 100644
--- a/src/frontend/src/layout/Header.tsx
+++ b/src/frontend/src/layout/Header.tsx
@@ -2,11 +2,13 @@ import { Link } from 'wouter'
 import { css } from '@/styled-system/css'
 import { Stack } from '@/styled-system/jsx'
 import { useTranslation } from 'react-i18next'
-import { A, Button, Popover, PopoverList, Text } from '@/primitives'
+import { A, Text, Button } from '@/primitives'
 import { SettingsButton } from '@/features/settings'
 import { authUrl, logoutUrl, useUser } from '@/features/auth'
 import { useMatchesRoute } from '@/navigation/useMatchesRoute'
 import { Feedback } from '@/components/Feedback'
+import { Menu } from '@/primitives/Menu'
+import { MenuList } from '@/primitives/MenuList'
 
 export const Header = () => {
   const { t } = useTranslation()
@@ -64,7 +66,7 @@ export const Header = () => {
               <A href={authUrl()}>{t('login')}</A>
             )}
             {!!user && (
-              <Popover aria-label={t('logout')}>
+              <Menu>
                 <Button
                   size="sm"
                   invisible
@@ -73,17 +75,15 @@ export const Header = () => {
                 >
                   {user.email}
                 </Button>
-                <PopoverList
-                  items={[
-                    { key: 'logout', value: 'logout', label: t('logout') },
-                  ]}
+                <MenuList
+                  items={[{ value: 'logout', label: t('logout') }]}
                   onAction={(value) => {
                     if (value === 'logout') {
                       window.location.href = logoutUrl()
                     }
                   }}
                 />
-              </Popover>
+              </Menu>
             )}
             <SettingsButton />
           </Stack>
diff --git a/src/frontend/src/primitives/Menu.tsx b/src/frontend/src/primitives/Menu.tsx
new file mode 100644
index 00000000..d26765e9
--- /dev/null
+++ b/src/frontend/src/primitives/Menu.tsx
@@ -0,0 +1,25 @@
+import { ReactNode } from 'react'
+import { MenuTrigger } from 'react-aria-components'
+import { StyledPopover } from './Popover'
+import { Box } from './Box'
+
+/**
+ * a Menu is a tuple of a trigger component (most usually a Button) that toggles menu items in a tooltip around the trigger
+ */
+export const Menu = ({
+  children,
+}: {
+  children: [trigger: ReactNode, menu: ReactNode]
+}) => {
+  const [trigger, menu] = children
+  return (
+    <MenuTrigger>
+      {trigger}
+      <StyledPopover>
+        <Box size="sm" type="popover">
+          {menu}
+        </Box>
+      </StyledPopover>
+    </MenuTrigger>
+  )
+}
diff --git a/src/frontend/src/primitives/MenuList.tsx b/src/frontend/src/primitives/MenuList.tsx
new file mode 100644
index 00000000..c35d76bb
--- /dev/null
+++ b/src/frontend/src/primitives/MenuList.tsx
@@ -0,0 +1,44 @@
+import { ReactNode } from 'react'
+import { Menu, MenuProps, MenuItem as RACMenuItem } from 'react-aria-components'
+import { styled } from '@/styled-system/jsx'
+import { menuItemStyles } from './menuItemStyles'
+
+const MenuItem = styled(RACMenuItem, menuItemStyles)
+
+/**
+ * render a Button primitive that shows a popover showing a list of pressable items
+ */
+export const MenuList = <T extends string | number = string>({
+  onAction,
+  selectedItem,
+  items = [],
+  ...menuProps
+}: {
+  onAction: (key: T) => void
+  selectedItem?: T
+  items: Array<string | { value: T; label: ReactNode }>
+} & MenuProps<unknown>) => {
+  return (
+    <Menu
+      selectionMode={selectedItem !== undefined ? 'single' : undefined}
+      selectedKeys={selectedItem !== undefined ? [selectedItem] : undefined}
+      {...menuProps}
+    >
+      {items.map((item) => {
+        const value = typeof item === 'string' ? item : item.value
+        const label = typeof item === 'string' ? item : item.label
+        return (
+          <MenuItem
+            key={value}
+            id={value as string}
+            onAction={() => {
+              onAction(value as T)
+            }}
+          >
+            {label}
+          </MenuItem>
+        )
+      })}
+    </Menu>
+  )
+}
diff --git a/src/frontend/src/primitives/Popover.tsx b/src/frontend/src/primitives/Popover.tsx
index e50462ab..c0863a15 100644
--- a/src/frontend/src/primitives/Popover.tsx
+++ b/src/frontend/src/primitives/Popover.tsx
@@ -49,7 +49,10 @@ const StyledOverlayArrow = styled(OverlayArrow, {
 })
 
 /**
- * a Popover is a tuple of a trigger component (most usually a Button) that toggles some interactive content in a tooltip around the trigger
+ * a Popover is a tuple of a trigger component (most usually a Button) that toggles some content in a tooltip around the trigger
+ *
+ * Note: to show a list of actionable items, like a dropdown menu, prefer using a <Menu> or <Select>.
+ * This is here when needing to show unrestricted content in a box.
  */
 export const Popover = ({
   children,
diff --git a/src/frontend/src/primitives/PopoverList.tsx b/src/frontend/src/primitives/PopoverList.tsx
deleted file mode 100644
index 33dc0ddb..00000000
--- a/src/frontend/src/primitives/PopoverList.tsx
+++ /dev/null
@@ -1,72 +0,0 @@
-import { ReactNode, useContext } from 'react'
-import {
-  ButtonProps,
-  Button,
-  OverlayTriggerStateContext,
-} from 'react-aria-components'
-import { styled } from '@/styled-system/jsx'
-
-const ListItem = styled(Button, {
-  base: {
-    paddingY: 0.125,
-    paddingX: 0.5,
-    textAlign: 'left',
-    width: 'full',
-    borderRadius: 4,
-    cursor: 'pointer',
-    color: 'box.text',
-    border: '1px solid transparent',
-    '&[data-selected]': {
-      fontWeight: 'bold',
-    },
-    '&[data-focused]': {
-      color: 'primary.text',
-      backgroundColor: 'primary',
-      outline: 'none!',
-    },
-    '&[data-hovered]': {
-      color: 'primary.text',
-      backgroundColor: 'primary',
-      outline: 'none!',
-    },
-  },
-})
-
-/**
- * render a Button primitive that shows a popover showing a list of pressable items
- */
-export const PopoverList = <T extends string | number = string>({
-  onAction,
-  closeOnAction = true,
-  items = [],
-}: {
-  closeOnAction?: boolean
-  onAction: (key: T) => void
-  items: Array<string | { key: string; value: T; label: ReactNode }>
-} & ButtonProps) => {
-  const popoverState = useContext(OverlayTriggerStateContext)!
-  return (
-    <ul>
-      {items.map((item) => {
-        const value = typeof item === 'string' ? item : item.value
-        const label = typeof item === 'string' ? item : item.label
-        const key = typeof item === 'string' ? item : item.key
-        return (
-          <li key={key}>
-            <ListItem
-              key={value}
-              onPress={() => {
-                onAction(value as T)
-                if (closeOnAction) {
-                  popoverState.close()
-                }
-              }}
-            >
-              {label}
-            </ListItem>
-          </li>
-        )
-      })}
-    </ul>
-  )
-}
diff --git a/src/frontend/src/primitives/Select.tsx b/src/frontend/src/primitives/Select.tsx
index 7dffbb12..eb6427ef 100644
--- a/src/frontend/src/primitives/Select.tsx
+++ b/src/frontend/src/primitives/Select.tsx
@@ -11,6 +11,7 @@ import {
 } from 'react-aria-components'
 import { Box } from './Box'
 import { StyledPopover } from './Popover'
+import { menuItemStyles } from './menuItemStyles'
 
 const StyledButton = styled(Button, {
   base: {
@@ -43,26 +44,7 @@ const StyledSelectValue = styled(SelectValue, {
   },
 })
 
-const StyledListBoxItem = styled(ListBoxItem, {
-  base: {
-    paddingY: 0.125,
-    paddingX: 0.5,
-    textAlign: 'left',
-    width: 'full',
-    borderRadius: 4,
-    cursor: 'pointer',
-    color: 'box.text',
-    border: '1px solid transparent',
-    '&[data-selected]': {
-      fontWeight: 'bold',
-    },
-    '&[data-focused]': {
-      color: 'primary.text',
-      backgroundColor: 'primary',
-      outline: 'none!',
-    },
-  },
-})
+const StyledListBoxItem = styled(ListBoxItem, menuItemStyles)
 
 export const Select = <T extends string | number>({
   label,
diff --git a/src/frontend/src/primitives/index.ts b/src/frontend/src/primitives/index.ts
index bbdb59ea..d7f9c706 100644
--- a/src/frontend/src/primitives/index.ts
+++ b/src/frontend/src/primitives/index.ts
@@ -1,3 +1,9 @@
+/**
+ * exposes all primitives we want to use in other parts of the app.
+ *
+ * It's intended not everything is exported: some primitives are meant as building-blocks
+ * for other primitives and don't have any value being exposed.
+ */
 export { A } from './A'
 export { Badge } from './Badge'
 export { Bold } from './Bold'
@@ -13,9 +19,10 @@ export { Hr } from './Hr'
 export { Italic } from './Italic'
 export { Input } from './Input'
 export { Link } from './Link'
+export { Menu } from './Menu'
+export { MenuList } from './MenuList'
 export { P } from './P'
 export { Popover } from './Popover'
-export { PopoverList } from './PopoverList'
 export { Text } from './Text'
 export { Ul } from './Ul'
 export { VerticallyOffCenter } from './VerticallyOffCenter'
diff --git a/src/frontend/src/primitives/menuItemStyles.ts b/src/frontend/src/primitives/menuItemStyles.ts
new file mode 100644
index 00000000..6bb84dd8
--- /dev/null
+++ b/src/frontend/src/primitives/menuItemStyles.ts
@@ -0,0 +1,31 @@
+/**
+ * reusable styles for a menu item, select item, etc… to be used with panda `css()` or `styled()`
+ *
+ * these are in their own files because react hot refresh doesn't like exporting stuff
+ * that aren't components in component files
+ */
+export const menuItemStyles = {
+  base: {
+    paddingY: 0.125,
+    paddingX: 0.5,
+    textAlign: 'left',
+    width: 'full',
+    borderRadius: 4,
+    cursor: 'pointer',
+    color: 'box.text',
+    border: '1px solid transparent',
+    '&[data-selected]': {
+      fontWeight: 'bold!',
+    },
+    '&[data-focused]': {
+      color: 'primary.text',
+      backgroundColor: 'primary',
+      outline: 'none!',
+    },
+    '&[data-hovered]': {
+      color: 'primary.text',
+      backgroundColor: 'primary',
+      outline: 'none!',
+    },
+  },
+}