From 250258da9b6a37b507857ecab7ae5c29faf3d0bf Mon Sep 17 00:00:00 2001
From: Eric Bailey <git@esb.lol>
Date: Tue, 5 Mar 2024 14:09:48 -0600
Subject: [PATCH] Usability improvements

---
 package.json                      |  1 +
 src/components/Menu/index.tsx     | 38 ++++++++++++++++++++++---------
 src/components/Menu/index.web.tsx | 32 ++++++++++++++++++--------
 src/components/Menu/types.ts      | 13 +++++++----
 yarn.lock                         |  7 ++++++
 5 files changed, 66 insertions(+), 25 deletions(-)

diff --git a/package.json b/package.json
index 0fc1e8bc95..cd8cc76519 100644
--- a/package.json
+++ b/package.json
@@ -149,6 +149,7 @@
     "react-avatar-editor": "^13.0.0",
     "react-circular-progressbar": "^2.1.0",
     "react-dom": "^18.2.0",
+    "react-keyed-flatten-children": "^3.0.0",
     "react-native": "0.73.2",
     "react-native-appstate-hook": "^1.0.6",
     "react-native-drawer-layout": "^4.0.0-alpha.3",
diff --git a/src/components/Menu/index.tsx b/src/components/Menu/index.tsx
index 5cb77b2cdc..f9d6fd8832 100644
--- a/src/components/Menu/index.tsx
+++ b/src/components/Menu/index.tsx
@@ -1,5 +1,6 @@
 import React from 'react'
 import {View, Pressable} from 'react-native'
+import flattenReactChildren from 'react-keyed-flatten-children'
 
 import {logger} from '#/logger'
 import {atoms as a, useTheme} from '#/alf'
@@ -80,21 +81,26 @@ export function Trigger({children, label}: TriggerProps) {
 }
 
 export function Outer({children}: React.PropsWithChildren<{}>) {
-  const {control} = React.useContext(Context)
+  const context = React.useContext(Context)
 
   return (
-    <Dialog.Outer control={control}>
+    <Dialog.Outer control={context.control}>
       <Dialog.Handle />
-      <Dialog.ScrollableInner label="Menu TODO">
-        <View style={[a.gap_lg]}>{children}</View>
-        <View style={{height: a.gap_lg.gap}} />
-      </Dialog.ScrollableInner>
+
+      {/* Re-wrap with context since Dialogs are portal-ed to root */}
+      <Context.Provider value={context}>
+        <Dialog.ScrollableInner label="Menu TODO">
+          <View style={[a.gap_lg]}>{children}</View>
+          <View style={{height: a.gap_lg.gap}} />
+        </Dialog.ScrollableInner>
+      </Context.Provider>
     </Dialog.Outer>
   )
 }
 
-export function Item({children, label, style, onPress}: ItemProps) {
+export function Item({children, label, style, onPress, ...rest}: ItemProps) {
   const t = useTheme()
+  const {control} = React.useContext(Context)
   const {state: focused, onIn: onFocus, onOut: onBlur} = useInteractionState()
   const {
     state: pressed,
@@ -104,9 +110,16 @@ export function Item({children, label, style, onPress}: ItemProps) {
 
   return (
     <Pressable
+      {...rest}
       accessibilityHint=""
       accessibilityLabel={label}
-      onPress={onPress}
+      onPress={e => {
+        onPress(e)
+
+        if (!e.defaultPrevented) {
+          control?.close()
+        }
+      }}
       onFocus={onFocus}
       onBlur={onBlur}
       onPressIn={onPressIn}
@@ -115,12 +128,12 @@ export function Item({children, label, style, onPress}: ItemProps) {
         a.flex_row,
         a.align_center,
         a.gap_sm,
-        a.p_md,
+        a.px_md,
         a.rounded_md,
         a.border,
         t.atoms.bg_contrast_25,
         t.atoms.border_contrast_low,
-        {minHeight: 48},
+        {minHeight: 44, paddingVertical: 10},
         style,
         (focused || pressed) && [t.atoms.bg_contrast_50],
       ]}>
@@ -133,7 +146,10 @@ export function ItemText({children, style}: ItemTextProps) {
   const t = useTheme()
   return (
     <Text
+      numberOfLines={1}
+      ellipsizeMode="middle"
       style={[
+        a.flex_1,
         a.text_md,
         a.font_bold,
         t.atoms.text_contrast_medium,
@@ -161,7 +177,7 @@ export function Group({children, style}: GroupProps) {
         t.atoms.border_contrast_low,
         style,
       ]}>
-      {React.Children.toArray(children).map((child, i) => {
+      {flattenReactChildren(children).map((child, i) => {
         // ignore null children, like Dividers
         return React.isValidElement(child) && child.props.children ? (
           <React.Fragment key={i}>
diff --git a/src/components/Menu/index.web.tsx b/src/components/Menu/index.web.tsx
index 962e3c4416..88452c8939 100644
--- a/src/components/Menu/index.web.tsx
+++ b/src/components/Menu/index.web.tsx
@@ -140,8 +140,9 @@ export function Outer({children}: React.PropsWithChildren<{}>) {
   )
 }
 
-export function Item({children, label, onPress}: ItemProps) {
+export function Item({children, label, onPress, ...rest}: ItemProps) {
   const t = useTheme()
+  const {control} = React.useContext(Context)
   const {
     state: hovered,
     onIn: onMouseEnter,
@@ -150,12 +151,23 @@ export function Item({children, label, onPress}: ItemProps) {
   const {state: focused, onIn: onFocus, onOut: onBlur} = useInteractionState()
 
   return (
-    <DropdownMenu.Item asChild onSelect={onPress}>
+    <DropdownMenu.Item asChild>
       <Pressable
+        {...rest}
         className="radix-dropdown-item"
         accessibilityHint=""
         accessibilityLabel={label}
-        onPress={onPress}
+        onPress={e => {
+          onPress(e)
+
+          /**
+           * Ported forward from Radix
+           * @see https://www.radix-ui.com/primitives/docs/components/dropdown-menu#item
+           */
+          if (!e.defaultPrevented) {
+            control.close()
+          }
+        }}
         onFocus={onFocus}
         onBlur={onBlur}
         style={flatten([
@@ -163,9 +175,8 @@ export function Item({children, label, onPress}: ItemProps) {
           a.align_center,
           a.gap_sm,
           a.py_sm,
-          a.px_sm,
           a.rounded_xs,
-          {minHeight: 36},
+          {minHeight: 32, paddingHorizontal: 10},
           web({outline: 0}),
           (hovered || focused) && [
             web({outline: '0 !important'}),
@@ -185,7 +196,7 @@ export function Item({children, label, onPress}: ItemProps) {
 export function ItemText({children, style}: ItemTextProps) {
   const t = useTheme()
   return (
-    <Text style={[a.font_bold, t.atoms.text_contrast_high, style]}>
+    <Text style={[a.flex_1, a.font_bold, t.atoms.text_contrast_high, style]}>
       {children}
     </Text>
   )
@@ -198,9 +209,12 @@ export function ItemIcon({icon: Comp, position = 'left'}: ItemIconProps) {
       size="md"
       fill={t.atoms.text_contrast_medium.color}
       style={[
-        {
-          marginLeft: position === 'left' ? -2 : 0,
-          marginRight: position === 'right' ? -2 : 0,
+        position === 'left' && {
+          marginLeft: -2,
+        },
+        position === 'right' && {
+          marginRight: -2,
+          marginLeft: 12,
         },
       ]}
     />
diff --git a/src/components/Menu/types.ts b/src/components/Menu/types.ts
index 4ea77368b4..a29ef16f9e 100644
--- a/src/components/Menu/types.ts
+++ b/src/components/Menu/types.ts
@@ -1,6 +1,7 @@
 import React from 'react'
-import {Props as SVGIconProps} from '#/components/icons/common'
+import {GestureResponderEvent, PressableProps} from 'react-native'
 
+import {Props as SVGIconProps} from '#/components/icons/common'
 import * as Dialog from '#/components/Dialog'
 import {TextStyleProp, ViewStyleProp} from '#/alf'
 
@@ -46,11 +47,13 @@ export type TriggerChildProps =
       handlers: {}
     }
 
+// TODO test id
 export type ItemProps = React.PropsWithChildren<
-  ViewStyleProp & {
-    label: string
-    onPress: () => void
-  }
+  Omit<PressableProps, 'style'> &
+    ViewStyleProp & {
+      label: string
+      onPress: (e: GestureResponderEvent) => void
+    }
 >
 
 export type ItemTextProps = React.PropsWithChildren<TextStyleProp & {}>
diff --git a/yarn.lock b/yarn.lock
index e63d3af12d..c3ddb8ee04 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -18458,6 +18458,13 @@ react-is@^17.0.1:
   resolved "https://registry.yarnpkg.com/react-is/-/react-is-17.0.2.tgz#e691d4a8e9c789365655539ab372762b0efb54f0"
   integrity sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==
 
+react-keyed-flatten-children@^3.0.0:
+  version "3.0.0"
+  resolved "https://registry.yarnpkg.com/react-keyed-flatten-children/-/react-keyed-flatten-children-3.0.0.tgz#b6ad0bde437d3ab86c8af3a1902d164be2a29d67"
+  integrity sha512-tSH6gvOyQjt3qtjG+kU9sTypclL1672yjpVufcE3aHNM0FhvjBUQZqsb/awIux4zEuVC3k/DP4p0GdTT/QUt/Q==
+  dependencies:
+    react-is "^18.2.0"
+
 react-native-appstate-hook@^1.0.6:
   version "1.0.6"
   resolved "https://registry.yarnpkg.com/react-native-appstate-hook/-/react-native-appstate-hook-1.0.6.tgz#cbc16e7b89cfaea034cabd999f00e99053cabd06"