diff --git a/components/SaveFileButton.tsx b/components/SaveFileButton.tsx index 20a67f943..fcd449f5b 100644 --- a/components/SaveFileButton.tsx +++ b/components/SaveFileButton.tsx @@ -14,20 +14,9 @@ interface SaveFileButtonProps extends TouchableOpacityProps { style?: StyleProp; afterOnPress?: () => void; beforeOnPress?: (() => Promise) | (() => void); - onMenuWillHide?: () => void; - onMenuWillShow?: () => void; } -const SaveFileButton: React.FC = ({ - fileName, - fileContent, - children, - style, - beforeOnPress, - afterOnPress, - onMenuWillHide, - onMenuWillShow, -}) => { +const SaveFileButton: React.FC = ({ fileName, fileContent, children, style, beforeOnPress, afterOnPress }) => { const handlePressMenuItem = useCallback( async (actionId: string) => { if (beforeOnPress) { @@ -50,14 +39,11 @@ const SaveFileButton: React.FC = ({ return ( {children} diff --git a/components/TooltipMenu.helpers.ts b/components/TooltipMenu.helpers.ts new file mode 100644 index 000000000..be8bb7063 --- /dev/null +++ b/components/TooltipMenu.helpers.ts @@ -0,0 +1,115 @@ +import { ContextMenuAction } from 'react-native-context-menu-view'; +import { Action } from './types'; + +export type Platform = 'ios' | 'android'; + +// Mirrors the structure of the items we hand to the native ContextMenu, with +// the original Action.id at every node. We walk this in lockstep with the +// indexPath the native side returns on press, so two actions sharing the same +// visible text (think localized "Copy") cannot collide. +export type IdNode = { + id?: string; + children?: IdNode[]; +}; + +export const normalizeMenuState = (menuState?: Action['menuState']): boolean | undefined => { + if (menuState === undefined) return undefined; + return menuState === 'mixed' ? true : Boolean(menuState); +}; + +const mapLeaf = (action: Action, platform: Platform): { item: ContextMenuAction; idNode: IdNode } | null => { + if (!action?.id || action.hidden) return null; + + const subResults = (action.subactions ?? []) + .map(sub => mapLeaf(sub, platform)) + .filter((r): r is { item: ContextMenuAction; idNode: IdNode } => r !== null); + + const item: ContextMenuAction = { + title: action.text, + subtitle: action.subtitle, + systemIcon: platform === 'ios' ? (action.icon?.iconValue ?? action.image) : undefined, + icon: platform === 'android' ? (action.icon?.iconValue ?? action.image) : undefined, + iconColor: typeof action.imageColor === 'string' ? action.imageColor : undefined, + destructive: Boolean(action.destructive), + disabled: Boolean(action.disabled), + inlineChildren: platform === 'ios' ? action.displayInline : undefined, + }; + + const selected = normalizeMenuState(action.menuState); + if (selected !== undefined) item.selected = selected; + if (subResults.length > 0) item.actions = subResults.map(r => r.item); + + const idNode: IdNode = { id: String(action.id) }; + if (subResults.length > 0) idNode.children = subResults.map(r => r.idNode); + + return { item, idNode }; +}; + +/** + * Build the items array passed to the native ContextMenu and a parallel + * id-tree of the exact same shape. Returning both in a single pass guarantees + * they cannot drift apart. + * + * iOS preserves grouping (with synthetic inline parents); Android flattens + * because its native menu doesn't render inline groups. + */ +export const buildMenu = (actions: Action[] | Action[][], platform: Platform): { items: ContextMenuAction[]; ids: IdNode[] } => { + const items: ContextMenuAction[] = []; + const ids: IdNode[] = []; + + if (platform === 'ios') { + for (const group of actions) { + if (Array.isArray(group)) { + const inline = group.map(a => mapLeaf(a, platform)).filter((r): r is { item: ContextMenuAction; idNode: IdNode } => r !== null); + if (inline.length === 0) continue; + items.push({ + title: '', + actions: inline.map(r => r.item), + inlineChildren: true, + } as ContextMenuAction); + // Synthetic inline group: no id of its own, only carries children. + ids.push({ children: inline.map(r => r.idNode) }); + } else { + const r = mapLeaf(group, platform); + if (r) { + items.push(r.item); + ids.push(r.idNode); + } + } + } + return { items, ids }; + } + + for (const action of actions.flat()) { + const r = mapLeaf(action, platform); + if (r) { + items.push(r.item); + ids.push(r.idNode); + } + } + return { items, ids }; +}; + +/** + * Resolve the original Action.id for a press event by walking the id-tree + * with the path delivered by the native side. + * + * Path semantics: + * - iOS: indexPath is always populated, full path from root. + * - Android top-level: only `index` is set; we synthesize `[index]`. + * - Android submenu: indexPath is `[parentIndex, childIndex]`. + */ +export const lookupId = (ids: IdNode[], path: readonly number[]): string | undefined => { + if (path.length === 0) return undefined; + + let nodes: IdNode[] | undefined = ids; + let node: IdNode | undefined; + + for (const i of path) { + if (!nodes || i < 0 || i >= nodes.length) return undefined; + node = nodes[i]; + nodes = node.children; + } + + return node?.id; +}; diff --git a/components/TooltipMenu.tsx b/components/TooltipMenu.tsx index 3fd9c013a..245d4ea39 100644 --- a/components/TooltipMenu.tsx +++ b/components/TooltipMenu.tsx @@ -1,8 +1,9 @@ -import React, { useCallback, useMemo, useRef } from 'react'; -import { NativeSyntheticEvent, Platform, Pressable, StyleSheet } from 'react-native'; -import ContextMenu, { ContextMenuAction, ContextMenuOnPressNativeEvent } from 'react-native-context-menu-view'; -import { ToolTipMenuProps, Action } from './types'; +import React, { useCallback, useMemo } from 'react'; +import { NativeSyntheticEvent, Platform, Pressable, StyleProp, StyleSheet, View, ViewStyle } from 'react-native'; +import ContextMenu, { ContextMenuOnPressNativeEvent } from 'react-native-context-menu-view'; +import { ToolTipMenuProps } from './types'; import { useSettings } from '../hooks/context/useSettings'; +import { buildMenu, lookupId } from './TooltipMenu.helpers'; const ToolTipMenu = (props: ToolTipMenuProps) => { const { @@ -21,205 +22,108 @@ const ToolTipMenu = (props: ToolTipMenuProps) => { accessibilityState, testID, style, - onMenuWillShow, - onMenuWillHide, enableAndroidRipple = true, - enableIOSPressOpacity = false, } = props; const { language } = useSettings(); - const openedRef = useRef(false); - const normalizeMenuState = useCallback((menuState?: Action['menuState']): boolean | undefined => { - if (menuState === undefined) { - return undefined; - } - if (menuState === 'mixed') { - return true; - } - return Boolean(menuState); - }, []); + const { items, ids } = useMemo(() => buildMenu(actions, Platform.OS as 'ios' | 'android'), [actions]); - const mapMenuItemForMenuView = useCallback( - (action: Action): ContextMenuAction | null => { - if (!action?.id || action.hidden) return null; - - const mappedSubactions = (action.subactions || []) - .map(subaction => mapMenuItemForMenuView(subaction)) - .filter((item): item is ContextMenuAction => item !== null); - - const menuItem: ContextMenuAction = { - title: action.text, - subtitle: action.subtitle, - systemIcon: Platform.OS === 'ios' ? (action.icon?.iconValue ?? action.image) : undefined, - icon: Platform.OS === 'android' ? (action.icon?.iconValue ?? action.image) : undefined, - iconColor: typeof action.imageColor === 'string' ? action.imageColor : undefined, - destructive: Boolean(action.destructive), - disabled: Boolean(action.disabled), - inlineChildren: Platform.OS === 'ios' ? action.displayInline : undefined, - }; - - const selected = normalizeMenuState(action.menuState); - if (selected !== undefined) { - menuItem.selected = selected; - } - - if (mappedSubactions.length > 0) { - menuItem.actions = mappedSubactions; - } - - return menuItem; + const handlePressMenuItem = useCallback( + (e: NativeSyntheticEvent) => { + const { name, indexPath, index } = e.nativeEvent; + const path = indexPath?.length ? indexPath : typeof index === 'number' ? [index] : []; + const id = lookupId(ids, path); + if (id !== undefined) onPressMenuItem(id); + else if (name) onPressMenuItem(name); // last-resort fallback }, - [normalizeMenuState], + [ids, onPressMenuItem], ); - const menuViewItemsIOS = useMemo(() => { - return actions - .map(actionGroup => { - if (Array.isArray(actionGroup) && actionGroup.length > 0) { - const inlineActions = actionGroup.map(mapMenuItemForMenuView).filter((item): item is ContextMenuAction => item !== null); - if (inlineActions.length === 0) return null; - const group: ContextMenuAction = { - title: '', - actions: inlineActions, - inlineChildren: true, - }; - return group; - } + if (disabled || actions.length === 0) return null; - if (!Array.isArray(actionGroup)) { - return mapMenuItemForMenuView(actionGroup); - } + // The native ContextMenu is the single source of truth for opening the menu: + // - Android: ContextMenuView's GestureDetector handles tap (dropdown mode) + // and long-press, then opens the popup itself. + // - iOS: UIContextMenuInteraction is attached to the first React child, with + // `showsMenuAsPrimaryAction` for tap-to-open in dropdown mode. + // + // We wrap in a Pressable ONLY when the caller wants a separate `onPress` + // (a short-tap action that does something OTHER than open the menu). Adding + // any extra Pressable handler is unnecessary and on Android races with the + // native gesture detector — usePressability always returns true from + // onStartShouldSetResponder, so the JS responder system claims the touch + // and dispatches ACTION_CANCEL to the child native view, leaving the menu + // unopened. There is no escape hatch for that — Pressable cannot be + // configured to skip responder claiming. + // + // Trade-off: dropdown buttons without onPress (HeaderMenuButton et al.) + // get no Android ripple. The menu opening (≈100ms) is the feedback. We + // accept this rather than reintroduce the gesture-cancel race. + const wrapInPressable = Boolean(onPress); - return null; - }) - .filter((item): item is ContextMenuAction => item !== null); - }, [actions, mapMenuItemForMenuView]); + const buttonShellStyle: StyleProp = isButton ? styles.button : undefined; + const visibleStyle = StyleSheet.flatten([buttonShellStyle, style, buttonStyle]); - const menuViewItemsAndroid = useMemo(() => { - const mergedActions = actions.flat().filter(action => action.id && !action.hidden); - return mergedActions.map(mapMenuItemForMenuView).filter((item): item is ContextMenuAction => item !== null); - }, [actions, mapMenuItemForMenuView]); - - // Map each action's display text to its stable id so the native press event - // (which only carries the action title) can be resolved back to the original id. - const titleToId = useMemo(() => { - const map = new Map(); - const registerAction = (action: Action) => { - if (action.id && action.text && !action.hidden) { - map.set(action.text, String(action.id)); - } - if (action.subactions) { - action.subactions.forEach(registerAction); - } - }; - actions.flat().forEach(registerAction); - return map; - }, [actions]); - - const handleMenuWillShow = useCallback(() => { - if (openedRef.current) { - return; - } - const visibleItems = Platform.OS === 'ios' ? menuViewItemsIOS : menuViewItemsAndroid; - if (visibleItems.length === 0) { - return; - } - openedRef.current = true; - onMenuWillShow?.(); - }, [onMenuWillShow, menuViewItemsIOS, menuViewItemsAndroid]); - - const handlePressMenuItemForMenuView = (e: NativeSyntheticEvent) => { - const { name } = e.nativeEvent; - if (name) { - const id = titleToId.get(name) ?? name; - onPressMenuItem(id); - } - openedRef.current = false; - onMenuWillHide?.(); - }; - - const renderMenuView = () => { - if (disabled) { - return null; - } - - // Keep wrapper behavior opt-in for button-like call sites. - // Non-button usages should not gain extra press/long-press handlers. - const shouldWrapWithPressable = isButton || Boolean(onPress); - - if (!shouldWrapWithPressable) { - return ( - { - if (!openedRef.current) { - return; - } - openedRef.current = false; - onMenuWillHide?.(); - }} - actions={Platform.OS === 'ios' ? menuViewItemsIOS : menuViewItemsAndroid} - dropdownMenuMode={!shouldOpenOnLongPress} - disabled={disabled} - style={style} - > - {children} - - ); - } + const menu = ( + + {children} + + ); + if (!wrapInPressable) { + // Wrap the native ContextMenu in a plain View that carries `testID` and the + // accessibility props. On iOS, react-native-context-menu-view propagates + // the accessibility identifier across multiple descendants of its native + // host, so attaching `testID` directly to ContextMenu makes Detox match + // multiple views (`Multiple elements found for "MATCHER(id == ...)"`). + // A plain View gives Detox a single, deterministic match and—unlike + // Pressable—never claims the JS responder, so it does not reintroduce the + // Android gesture-cancel race documented above. return ( - { - const shouldApplyPressedStyle = - pressed && ((Platform.OS === 'android' && enableAndroidRipple) || (Platform.OS === 'ios' && enableIOSPressOpacity)); - return StyleSheet.flatten([styles.pressable, style, buttonStyle, shouldApplyPressedStyle && styles.pressed]); - }} - disabled={disabled} - onPress={onPress} - onPressIn={!shouldOpenOnLongPress ? handleMenuWillShow : undefined} - onLongPress={shouldOpenOnLongPress ? handleMenuWillShow : undefined} + - { - if (!openedRef.current) { - return; - } - openedRef.current = false; - onMenuWillHide?.(); - }} - actions={Platform.OS === 'ios' ? menuViewItemsIOS : menuViewItemsAndroid} - dropdownMenuMode={!shouldOpenOnLongPress} - disabled={disabled} - style={buttonStyle ? styles.menuViewFlex : undefined} - > - {children} - - + {menu} + ); - }; + } - return actions.length > 0 ? renderMenuView() : null; + return ( + + StyleSheet.flatten([visibleStyle, pressed && enableAndroidRipple && Platform.OS === 'android' ? styles.pressed : null]) + } + accessibilityLabel={accessibilityLabel} + accessibilityHint={accessibilityHint} + accessibilityRole={accessibilityRole} + accessibilityState={accessibilityState} + accessibilityLanguage={language} + testID={testID} + hitSlop={8} + > + {menu} + + ); }; export default ToolTipMenu; const styles = StyleSheet.create({ - menuViewFlex: { flex: 1 }, - pressable: { alignSelf: 'center' }, + button: { alignSelf: 'center' }, + menuFlex: { flex: 1 }, pressed: { opacity: 0.6 }, }); diff --git a/components/addresses/AddressItem.tsx b/components/addresses/AddressItem.tsx index 45eca47d6..b3ae707b7 100644 --- a/components/addresses/AddressItem.tsx +++ b/components/addresses/AddressItem.tsx @@ -8,7 +8,6 @@ import { unlockWithBiometrics, useBiometrics } from '../../hooks/useBiometrics'; import loc, { formatBalance } from '../../loc'; import { BitcoinUnit } from '../../models/bitcoinUnits'; import presentAlert from '../Alert'; -import QRCodeComponent from '../QRCodeComponent'; import { useTheme } from '../themes'; import { AddressTypeBadge } from './AddressTypeBadge'; import { NativeStackNavigationProp } from '@react-navigation/native-stack'; @@ -170,8 +169,6 @@ const AddressItem = ({ [handleCopyPress, handleSharePress, navigateToSignVerify, handleCopyPrivkeyPress, isBiometricUseCapableAndEnabled], ); - const renderPreview = useCallback(() => , [item.address]); - // Render address with highlighting if a search query is provided const renderAddressContent = () => { if (searchQuery && searchQuery.length > 0) { @@ -201,8 +198,6 @@ const AddressItem = ({ title={item.address} actions={menuActions} onPressMenuItem={onToolTipPress} - // Revisit once RNMenu has renderPreview prop - renderPreview={renderPreview} onPress={navigateToReceive} isButton buttonStyle={styles.tooltipButton} diff --git a/components/types.ts b/components/types.ts index 0d3624e50..3544355cd 100644 --- a/components/types.ts +++ b/components/types.ts @@ -22,15 +22,17 @@ export interface ToolTipMenuProps { actions: Action[] | Action[][]; children: React.ReactNode; enableAndroidRipple?: boolean; - enableIOSPressOpacity?: boolean; - dismissMenu?: () => void; onPressMenuItem: (id: string) => void; title?: string; + // When true (default) the menu opens on long-press; when false it opens + // on a single tap (dropdown mode). shouldOpenOnLongPress?: boolean; + // Hint that the trigger should be styled like a button (e.g. center it). isButton?: boolean; - renderPreview?: () => React.ReactNode; + // Optional short-tap action. When provided, the trigger is wrapped in a + // Pressable so that a quick tap fires this callback while a long-press + // (or single tap in dropdown mode) still opens the native menu. onPress?: (event: GestureResponderEvent) => void; - previewValue?: string; accessibilityRole?: AccessibilityRole; disabled?: boolean; testID?: string; @@ -39,8 +41,6 @@ export interface ToolTipMenuProps { accessibilityHint?: string; accessibilityState?: object; buttonStyle?: ViewStyle | ViewStyle[]; - onMenuWillShow?: () => void; - onMenuWillHide?: () => void; } export enum HandOffActivityType { diff --git a/tests/unit/tooltip-menu-helpers.test.ts b/tests/unit/tooltip-menu-helpers.test.ts new file mode 100644 index 000000000..a9bcd7ccb --- /dev/null +++ b/tests/unit/tooltip-menu-helpers.test.ts @@ -0,0 +1,133 @@ +import { buildMenu, lookupId, normalizeMenuState } from '../../components/TooltipMenu.helpers'; +import type { Action } from '../../components/types'; + +describe('TooltipMenu.helpers', () => { + describe('normalizeMenuState', () => { + it('returns undefined when state is undefined', () => { + expect(normalizeMenuState(undefined)).toBeUndefined(); + }); + + it('coerces "mixed" to true', () => { + expect(normalizeMenuState('mixed')).toBe(true); + }); + + it('passes booleans through', () => { + expect(normalizeMenuState(true)).toBe(true); + expect(normalizeMenuState(false)).toBe(false); + }); + }); + + describe('buildMenu', () => { + const copyAddress: Action = { id: 'copy-address', text: 'Copy' }; + const share: Action = { id: 'share', text: 'Share' }; + const remove: Action = { id: 'delete', text: 'Delete', destructive: true }; + + it('preserves grouping on iOS with synthetic inline parents', () => { + const { items, ids } = buildMenu([[share, remove], [copyAddress]], 'ios'); + + expect(items).toHaveLength(2); + expect(items[0]).toMatchObject({ title: '', inlineChildren: true }); + expect(items[0].actions).toHaveLength(2); + expect(items[1]).toMatchObject({ title: '', inlineChildren: true }); + expect(items[1].actions).toHaveLength(1); + + // id-tree mirrors the items tree exactly. + expect(ids).toHaveLength(2); + expect(ids[0].id).toBeUndefined(); // synthetic group has no id + expect(ids[0].children).toHaveLength(2); + expect(ids[0].children?.[0].id).toBe('share'); + expect(ids[0].children?.[1].id).toBe('delete'); + expect(ids[1].children?.[0].id).toBe('copy-address'); + }); + + it('flattens groups on Android', () => { + const { items, ids } = buildMenu([[share, remove], [copyAddress]], 'android'); + + expect(items).toHaveLength(3); + expect(items.map(i => i.title)).toEqual(['Share', 'Delete', 'Copy']); + expect(ids.map(n => n.id)).toEqual(['share', 'delete', 'copy-address']); + }); + + it('skips hidden actions and actions without an id', () => { + const hidden: Action = { id: 'hidden', text: 'Hidden', hidden: true }; + const noId = { text: 'Bad' } as unknown as Action; + + const { items, ids } = buildMenu([share, hidden, noId, remove], 'android'); + + expect(items.map(i => i.title)).toEqual(['Share', 'Delete']); + expect(ids.map(n => n.id)).toEqual(['share', 'delete']); + }); + + it('maps subactions recursively and reflects them in the id-tree', () => { + const parent: Action = { + id: 'export', + text: 'Export', + subactions: [ + { id: 'export-share', text: 'Share' }, + { id: 'export-save', text: 'Save' }, + ], + }; + const { items, ids } = buildMenu([parent], 'ios'); + + expect(items[0].actions).toHaveLength(2); + expect(ids[0].id).toBe('export'); + expect(ids[0].children?.map(n => n.id)).toEqual(['export-share', 'export-save']); + }); + + it('assigns icons to the platform-appropriate field', () => { + const withIcon: Action = { id: 'x', text: 'X', icon: { iconValue: 'star' } }; + expect(buildMenu([withIcon], 'ios').items[0]).toMatchObject({ systemIcon: 'star', icon: undefined }); + expect(buildMenu([withIcon], 'android').items[0]).toMatchObject({ icon: 'star', systemIcon: undefined }); + }); + }); + + describe('lookupId', () => { + // Two actions with identical visible text would have collided in the + // old title-based map. The id-tree resolves them by position instead. + const collisionFixture = (() => { + const copyA: Action = { id: 'copy-a', text: 'Copy' }; + const copyB: Action = { id: 'copy-b', text: 'Copy' }; + return buildMenu([copyA, copyB], 'android').ids; + })(); + + it('returns the correct id for each twin even when visible text matches', () => { + expect(lookupId(collisionFixture, [0])).toBe('copy-a'); + expect(lookupId(collisionFixture, [1])).toBe('copy-b'); + }); + + it('walks into nested submenus on iOS', () => { + const parent: Action = { + id: 'export', + text: 'Export', + subactions: [ + { id: 'export-share', text: 'Share' }, + { id: 'export-save', text: 'Save' }, + ], + }; + const { ids } = buildMenu([parent], 'ios'); + + expect(lookupId(ids, [0])).toBe('export'); + expect(lookupId(ids, [0, 0])).toBe('export-share'); + expect(lookupId(ids, [0, 1])).toBe('export-save'); + }); + + it('walks through synthetic iOS inline groups', () => { + const a: Action = { id: 'a', text: 'A' }; + const b: Action = { id: 'b', text: 'B' }; + const { ids } = buildMenu([[a, b]], 'ios'); + + // path[0] selects the synthetic inline group; path[1] picks the leaf. + expect(lookupId(ids, [0, 0])).toBe('a'); + expect(lookupId(ids, [0, 1])).toBe('b'); + // The synthetic group itself has no id and cannot be pressed. + expect(lookupId(ids, [0])).toBeUndefined(); + }); + + it('returns undefined for empty path or out-of-range indices', () => { + const { ids } = buildMenu([{ id: 'only', text: 'Only' }], 'android'); + expect(lookupId(ids, [])).toBeUndefined(); + expect(lookupId(ids, [5])).toBeUndefined(); + expect(lookupId(ids, [-1])).toBeUndefined(); + }); + }); +});