MenuItem decomposition - description text style

src/components/MenuItem/MenuItemAccessibilityContext.tsx

@@ -24,116 +24,108 @@ type MenuItemAccessibilityAnnouncement = ValueOf<typeof MENU_ITEM_ACCESSIBILITY_
 const MENU_ITEM_LABEL_ANNOUNCEMENT_SLOTS = [MENU_ITEM_ACCESSIBILITY_ANNOUNCEMENT.OPENS_IN_NEW_TAB, MENU_ITEM_ACCESSIBILITY_ANNOUNCEMENT.REVIEW_REQUIRED];
 const MENU_ITEM_HINT_ANNOUNCEMENT_SLOTS = [MENU_ITEM_ACCESSIBILITY_ANNOUNCEMENT.CONTEXT_MENU_AVAILABLE];
 
-type MenuItemAccessibilityActions = {
-    /** Registers a label (the title or description text) under a fixed slot key */
-    registerLabel: (slot: MenuItemLabelSlot, text: string) => void;
-
-    /** Removes the label registered under the given slot */
-    unregisterLabel: (slot: MenuItemLabelSlot) => void;
+/** Where sub-components hand `MenuItem.Root` text, one entry per key */
+type MenuItemAccessibilityRegistry<TKey> = {
+    register: (key: TKey, text: string) => void;
+    unregister: (key: TKey) => void;
+};
 
-    /** Announces a fact about the row under a fixed key. Announcing the same fact twice announces it once */
-    registerAnnouncement: (announcement: MenuItemAccessibilityAnnouncement, text: string) => void;
+type MenuItemAccessibilityRegistries = {
+    /** Text leaves register their text here. Left out when the row names itself, so the leaves skip the work */
+    label: MenuItemAccessibilityRegistry<MenuItemLabelSlot> | undefined;
 
-    /** Stops announcing the given fact */
-    unregisterAnnouncement: (announcement: MenuItemAccessibilityAnnouncement) => void;
+    /** Leaves announce facts about the row here. Always there, since the facts are read even on a row that names itself */
+    announcement: MenuItemAccessibilityRegistry<MenuItemAccessibilityAnnouncement>;
 };
 
-const MenuItemAccessibilityContext = createContext<MenuItemAccessibilityActions | undefined>(undefined);
+const MenuItemAccessibilityContext = createContext<MenuItemAccessibilityRegistries | undefined>(undefined);
 
-/**
- * Contributes text to the label `MenuItem.Root` derives. The fixed slot key keeps the announced order
- * deterministic (`top`, then `bottom`). No-op when `text` is empty or outside a `MenuItem.Root`.
- */
-function useMenuItemAccessibilityLabel(slot: MenuItemLabelSlot, text: string | undefined) {
-    const actions = useContext(MenuItemAccessibilityContext);
-    const registerLabel = actions?.registerLabel;
-    const unregisterLabel = actions?.unregisterLabel;
+/** Keeps `text` registered under `key` while the caller is mounted. No-op without a registry or text */
+function useRegistration<TKey>(registry: MenuItemAccessibilityRegistry<TKey> | undefined, key: TKey, text: string | undefined) {
+    const register = registry?.register;
+    const unregister = registry?.unregister;
 
     useEffect(() => {
-        if (!text || !registerLabel || !unregisterLabel) {
+        if (!text || !register || !unregister) {
             return;
         }
-        registerLabel(slot, text);
-        return () => unregisterLabel(slot);
-    }, [slot, text, registerLabel, unregisterLabel]);
+        register(key, text);
+        return () => unregister(key);
+    }, [key, text, register, unregister]);
 }
 
-/** Contributes an already translated announcement about the row */
-function useMenuItemAccessibilityAnnouncement(announcement: MenuItemAccessibilityAnnouncement | undefined, text: string | undefined) {
-    const actions = useContext(MenuItemAccessibilityContext);
-    const registerAnnouncement = actions?.registerAnnouncement;
-    const unregisterAnnouncement = actions?.unregisterAnnouncement;
+/** Contributes text to the label `MenuItem.Root` derives, announced in slot order (`top`, then `bottom`) */
+function useMenuItemAccessibilityLabel(slot: MenuItemLabelSlot, text: string | undefined) {
+    useRegistration(useContext(MenuItemAccessibilityContext)?.label, slot, text);
+}
 
-    useEffect(() => {
-        if (!announcement || !text || !registerAnnouncement || !unregisterAnnouncement) {
-            return;
-        }
-        registerAnnouncement(announcement, text);
-        return () => unregisterAnnouncement(announcement);
-    }, [announcement, text, registerAnnouncement, unregisterAnnouncement]);
+/** Contributes an already translated announcement about the row */
+function useMenuItemAccessibilityAnnouncement(announcement: MenuItemAccessibilityAnnouncement, text: string | undefined) {
+    useRegistration(useContext(MenuItemAccessibilityContext)?.announcement, announcement, text);
 }
 
 /**
- * Small `key -> value` registry backed by an immutable `Map`. Writing back an unchanged value is a
+ * Small `key -> text` registry backed by an immutable `Map`. Writing back an unchanged text is a
  * no-op, so unrelated re-renders don't churn the map identity.
  */
-function useKeyedRegistry<TKey, TValue>() {
-    const [entries, setEntries] = useState<Map<TKey, TValue>>(() => new Map());
-
-    const register = (key: TKey, value: TValue) => {
-        setEntries((prev) => {
-            if (prev.get(key) === value) {
-                return prev;
-            }
-            const next = new Map(prev);
-            next.set(key, value);
-            return next;
-        });
-    };
-
-    const unregister = (key: TKey) => {
-        setEntries((prev) => {
-            if (!prev.has(key)) {
-                return prev;
-            }
-            const next = new Map(prev);
-            next.delete(key);
-            return next;
-        });
+function useKeyedRegistry<TKey>() {
+    const [entries, setEntries] = useState<Map<TKey, string>>(() => new Map());
+
+    const registry: MenuItemAccessibilityRegistry<TKey> = {
+        register: (key, text) => {
+            setEntries((prev) => {
+                if (prev.get(key) === text) {
+                    return prev;
+                }
+                const next = new Map(prev);
+                next.set(key, text);
+                return next;
+            });
+        },
+        unregister: (key) => {
+            setEntries((prev) => {
+                if (!prev.has(key)) {
+                    return prev;
+                }
+                const next = new Map(prev);
+                next.delete(key);
+                return next;
+            });
+        },
     };
 
-    return {entries, register, unregister};
+    return {entries, registry};
 }
 
 /**
- * Builds the row's accessibility label and hint out of what its sub-components registered, plus the
- * value for `MenuItemAccessibilityContext.Provider`
+ * Builds the row's accessibility label and hint out of what its sub-components registered. An explicit
+ * `accessibilityLabel` replaces the derived one and switches the label registry off, but the announcements
+ * are still appended to it.
  */
-function useMenuItemAccessibility() {
-    // Text contributed by the text leaves, keyed by the line each one occupies
-    const {entries: labels, register: registerLabel, unregister: unregisterLabel} = useKeyedRegistry<MenuItemLabelSlot, string>();
-
-    // Facts contributed by any child, keyed by the fact
-    const {entries: announcements, register: registerAnnouncement, unregister: unregisterAnnouncement} = useKeyedRegistry<MenuItemAccessibilityAnnouncement, string>();
-
-    const accessibilityActions: MenuItemAccessibilityActions = {registerLabel, unregisterLabel, registerAnnouncement, unregisterAnnouncement};
+function useMenuItemAccessibility(accessibilityLabel?: string) {
+    const labels = useKeyedRegistry<MenuItemLabelSlot>();
+    const announcements = useKeyedRegistry<MenuItemAccessibilityAnnouncement>();
 
-    const derivedLabel = MENU_ITEM_LABEL_SLOTS.map((slot) => labels.get(slot))
+    const derivedLabel = MENU_ITEM_LABEL_SLOTS.map((slot) => labels.entries.get(slot))
         .filter(Boolean)
         .join(', ');
-
-    const labelAnnouncements = MENU_ITEM_LABEL_ANNOUNCEMENT_SLOTS.map((announcement) => announcements.get(announcement)).filter(Boolean);
-    const hintAnnouncements = MENU_ITEM_HINT_ANNOUNCEMENT_SLOTS.map((announcement) => announcements.get(announcement)).filter(Boolean);
+    const labelAnnouncements = MENU_ITEM_LABEL_ANNOUNCEMENT_SLOTS.map((announcement) => announcements.entries.get(announcement)).filter(Boolean);
+    const hintAnnouncements = MENU_ITEM_HINT_ANNOUNCEMENT_SLOTS.map((announcement) => announcements.entries.get(announcement)).filter(Boolean);
 
     // Keeps the hints out of the name on native, and folds them into it on the web
-    const {accessibilityLabel, accessibilityHint} = getContextMenuAccessibilityProps({
-        accessibilityLabel: [derivedLabel, ...labelAnnouncements].filter(Boolean).join('. '),
+    const accessibilityProps = getContextMenuAccessibilityProps({
+        accessibilityLabel: [accessibilityLabel ?? derivedLabel, ...labelAnnouncements].filter(Boolean).join('. '),
         contextMenuHint: hintAnnouncements.join('. ') || undefined,
     });
 
-    return {accessibilityLabel, accessibilityHint, accessibilityActions};
+    const registries: MenuItemAccessibilityRegistries = {
+        label: accessibilityLabel === undefined ? labels.registry : undefined,
+        announcement: announcements.registry,
+    };
+
+    return {...accessibilityProps, registries};
 }
 
-export default MenuItemAccessibilityContext;
 export type {MenuItemLabelSlot};
-export {MENU_ITEM_ACCESSIBILITY_ANNOUNCEMENT, useMenuItemAccessibilityLabel, useMenuItemAccessibilityAnnouncement, useMenuItemAccessibility};
+export default MenuItemAccessibilityContext;
+export {MENU_ITEM_ACCESSIBILITY_ANNOUNCEMENT, useMenuItemAccessibility, useMenuItemAccessibilityAnnouncement, useMenuItemAccessibilityLabel};

src/components/MenuItem/index.ts

@@ -42,6 +42,7 @@ import MenuItemTitle from './leaves/content/MenuItemTitle';
 import MenuItemHelpText from './leaves/helpText/MenuItemHelpText';
 import MenuItemHelpTextHTML from './leaves/helpText/MenuItemHelpTextHTML';
 import MenuItemIcon from './leaves/leading/MenuItemIcon';
+import MenuItemIconNarrow from './leaves/leading/MenuItemIconNarrow';
 import MenuItemBrickRoadIndicator from './leaves/trailing/icons/MenuItemBrickRoadIndicator';
 import MenuItemChevron from './leaves/trailing/icons/MenuItemChevron';
 import MenuItemDownCaret from './leaves/trailing/icons/MenuItemDownCaret';
@@ -58,6 +59,7 @@ const MenuItem = Object.assign(LegacyMenuItem, {
     Content: MenuItemContent,
     Trailing: MenuItemTrailing,
     Icon: MenuItemIcon,
+    IconNarrow: MenuItemIconNarrow,
     Label: MenuItemLabel,
     Title: MenuItemTitle,
     Description: MenuItemDescription,

src/components/MenuItem/layout/MenuItemRoot.tsx

@@ -35,9 +35,9 @@ type MenuItemRootProps = PropsWithChildren &
         isDisabled?: boolean;
 
         /**
-         * Pre-computed accessibility label. When provided, `Root` uses it directly instead of
-         * deriving the label from registered `Title`/`Description` children. Presets that know
-         * their text statically should pass it.
+         * Pre-computed accessibility label. When provided, `Root` uses it instead of deriving the label
+         * from the text leaves. Announcements such as "opens in a new tab" are still appended.
+         * Presets that know their text statically should pass it.
          */
         accessibilityLabel?: string;
 
@@ -56,7 +56,7 @@ function MenuItemRoot({children, onPress, isDisabled = false, sentryLabel, testI
     const {shouldUseNarrowLayout} = useResponsiveLayout();
     const isInteractive = !!onPress;
 
-    const {accessibilityLabel: derivedAccessibilityLabel, accessibilityHint, accessibilityActions} = useMenuItemAccessibility();
+    const {accessibilityLabel: rowAccessibilityLabel, accessibilityHint, registries: accessibilityRegistries} = useMenuItemAccessibility(accessibilityLabel);
     const {handler: registeredSecondaryInteraction, register: registerSecondaryInteraction} = useMenuItemSecondaryInteractionRegistry();
 
     useRemoveNonInteractiveClickHandler(pressableRef, isInteractive);
@@ -108,15 +108,15 @@ function MenuItemRoot({children, onPress, isDisabled = false, sentryLabel, testI
                         disabled={isDisabled}
                         ref={pressableRef}
                         role={isInteractive ? CONST.ROLE.BUTTON : undefined}
-                        accessibilityLabel={accessibilityLabel ?? derivedAccessibilityLabel}
+                        accessibilityLabel={rowAccessibilityLabel}
                         accessibilityHint={accessibilityHint}
                         accessible
                         tabIndex={isInteractive ? 0 : -1}
                         sentryLabel={sentryLabel}
                         testID={testID}
                     >
                         {({pressed}) => (
-                            <MenuItemAccessibilityContext.Provider value={accessibilityLabel === undefined ? accessibilityActions : undefined}>
+                            <MenuItemAccessibilityContext.Provider value={accessibilityRegistries}>
                                 <MenuItemSecondaryInteractionContext.Provider value={registerSecondaryInteraction}>
                                     <MenuItemInteractionContext.Provider
                                         value={{

src/components/MenuItem/leaves/leading/MenuItemIcon.tsx

@@ -1,42 +1,19 @@
-import Icon from '@components/Icon';
-import useIsCompactPopover from '@components/MenuItem/hooks/useIsCompactPopover';
-import {useMenuItemConfig, useMenuItemInteraction} from '@components/MenuItem/MenuItemContext';
-
-import useStyleUtils from '@hooks/useStyleUtils';
 import useThemeStyles from '@hooks/useThemeStyles';
 
-import getButtonState from '@libs/getButtonState';
-
-import type IconAsset from '@src/types/utils/IconAsset';
-
 import React from 'react';
 
-type MenuItemIconProps = {
-    src: IconAsset;
-};
+import type {MenuItemIconProps} from './base/BaseMenuItemIcon';
+
+import BaseMenuItemIcon from './base/BaseMenuItemIcon';
 
 /** An icon glyph, filled from the row's interaction state */
 function MenuItemIcon({src}: MenuItemIconProps) {
     const styles = useThemeStyles();
-    const StyleUtils = useStyleUtils();
-    const {isDisabled, isInteractive} = useMenuItemConfig();
-    const {isHovered, isPressed} = useMenuItemInteraction();
-    const isCompactPopover = useIsCompactPopover();
-
-    const iconFill = StyleUtils.getIconFillColor({
-        buttonState: getButtonState({isActive: isHovered, isPressed, isDisabled, isInteractive}),
-        isMenuIcon: true,
-        isPane: true,
-    });
 
     return (
-        <Icon
-            contentFit="cover"
-            hovered={isHovered}
-            pressed={isPressed}
+        <BaseMenuItemIcon
             src={src}
-            fill={iconFill}
-            additionalStyles={[styles.popoverMenuIcon, isCompactPopover && styles.wAuto]}
+            style={styles.popoverMenuIcon}
         />
     );
 }

src/components/MenuItem/leaves/leading/MenuItemIconNarrow.tsx

@@ -0,0 +1,21 @@
+import useThemeStyles from '@hooks/useThemeStyles';
+
+import React from 'react';
+
+import type {MenuItemIconProps} from './base/BaseMenuItemIcon';
+
+import BaseMenuItemIcon from './base/BaseMenuItemIcon';
+
+/** An icon glyph that takes only its own width */
+function MenuItemIconNarrow({src}: MenuItemIconProps) {
+    const styles = useThemeStyles();
+
+    return (
+        <BaseMenuItemIcon
+            src={src}
+            style={[styles.popoverMenuIcon, styles.wAuto]}
+        />
+    );
+}
+
+export default MenuItemIconNarrow;

src/components/MenuItem/leaves/leading/base/BaseMenuItemIcon.tsx

@@ -0,0 +1,48 @@
+import Icon from '@components/Icon';
+import {useMenuItemConfig, useMenuItemInteraction} from '@components/MenuItem/MenuItemContext';
+
+import useStyleUtils from '@hooks/useStyleUtils';
+
+import getButtonState from '@libs/getButtonState';
+
+import type IconAsset from '@src/types/utils/IconAsset';
+
+import type {StyleProp, ViewStyle} from 'react-native';
+
+import React from 'react';
+
+type MenuItemIconProps = {
+    src: IconAsset;
+};
+
+type BaseMenuItemIconProps = MenuItemIconProps & {
+    /** The slot the glyph sits in, which is all the icon leaves differ by */
+    style: StyleProp<ViewStyle>;
+};
+
+/** Base of the icon leaves: a glyph filled from the row's interaction state */
+function BaseMenuItemIcon({src, style}: BaseMenuItemIconProps) {
+    const StyleUtils = useStyleUtils();
+    const {isDisabled, isInteractive} = useMenuItemConfig();
+    const {isHovered, isPressed} = useMenuItemInteraction();
+
+    const iconFill = StyleUtils.getIconFillColor({
+        buttonState: getButtonState({isActive: isHovered, isPressed, isDisabled, isInteractive}),
+        isMenuIcon: true,
+        isPane: true,
+    });
+
+    return (
+        <Icon
+            contentFit="cover"
+            hovered={isHovered}
+            pressed={isPressed}
+            src={src}
+            fill={iconFill}
+            additionalStyles={style}
+        />
+    );
+}
+
+export default BaseMenuItemIcon;
+export type {MenuItemIconProps};