From 2a089311277e3f36818dc5efec7f1f64a1965bec Mon Sep 17 00:00:00 2001 From: dan Date: Sun, 28 Apr 2024 21:29:43 +0100 Subject: [PATCH] Fix dropdown immediately closing on Enter (#3745) * Move dropdown content into separate component * Fix dropdown with keyboard * No-op is sufficient --- src/components/Menu/index.web.tsx | 37 +++-- .../com/util/forms/NativeDropdown.web.tsx | 150 ++++++++++-------- 2 files changed, 111 insertions(+), 76 deletions(-) diff --git a/src/components/Menu/index.web.tsx b/src/components/Menu/index.web.tsx index 60b23420..031250dd 100644 --- a/src/components/Menu/index.web.tsx +++ b/src/components/Menu/index.web.tsx @@ -1,27 +1,26 @@ /* eslint-disable react/prop-types */ import React from 'react' -import {View, Pressable, ViewStyle, StyleProp} from 'react-native' -import * as DropdownMenu from '@radix-ui/react-dropdown-menu' +import {Pressable, StyleProp, View, ViewStyle} from 'react-native' import {msg} from '@lingui/macro' import {useLingui} from '@lingui/react' +import * as DropdownMenu from '@radix-ui/react-dropdown-menu' +import {atoms as a, flatten, useTheme, web} from '#/alf' import * as Dialog from '#/components/Dialog' import {useInteractionState} from '#/components/hooks/useInteractionState' -import {atoms as a, useTheme, flatten, web} from '#/alf' -import {Text} from '#/components/Typography' - +import {Context} from '#/components/Menu/context' import { ContextType, - TriggerProps, - ItemProps, GroupProps, - ItemTextProps, ItemIconProps, + ItemProps, + ItemTextProps, RadixPassThroughTriggerProps, + TriggerProps, } from '#/components/Menu/types' -import {Context} from '#/components/Menu/context' import {Portal} from '#/components/Portal' +import {Text} from '#/components/Typography' export function useMenuControl(): Dialog.DialogControlProps { const id = React.useId() @@ -135,10 +134,22 @@ export function Trigger({children, label}: TriggerProps) { }, props: { ...props, - // disable on web, use `onPress` - onPointerDown: () => false, - onPress: () => - control.isOpen ? control.close() : control.open(), + // No-op override to prevent false positive that interprets mobile scroll as a tap. + // This requires the custom onPress handler below to compensate. + // https://github.com/radix-ui/primitives/issues/1912 + onPointerDown: undefined, + onPress: () => { + if (window.event instanceof KeyboardEvent) { + // The onPointerDown hack above is not relevant to this press, so don't do anything. + return + } + // Compensate for the disabled onPointerDown above by triggering it manually. + if (control.isOpen) { + control.close() + } else { + control.open() + } + }, onFocus: onFocus, onBlur: onBlur, onMouseEnter, diff --git a/src/view/com/util/forms/NativeDropdown.web.tsx b/src/view/com/util/forms/NativeDropdown.web.tsx index 94591d39..6668ac21 100644 --- a/src/view/com/util/forms/NativeDropdown.web.tsx +++ b/src/view/com/util/forms/NativeDropdown.web.tsx @@ -1,12 +1,13 @@ import React from 'react' +import {Pressable, StyleSheet, Text, View, ViewStyle} from 'react-native' +import {IconProp} from '@fortawesome/fontawesome-svg-core' import {FontAwesomeIcon} from '@fortawesome/react-native-fontawesome' import * as DropdownMenu from '@radix-ui/react-dropdown-menu' -import {Pressable, StyleSheet, View, Text, ViewStyle} from 'react-native' -import {IconProp} from '@fortawesome/fontawesome-svg-core' import {MenuItemCommonProps} from 'zeego/lib/typescript/menu' + +import {HITSLOP_10} from 'lib/constants' import {usePalette} from 'lib/hooks/usePalette' import {useTheme} from 'lib/ThemeContext' -import {HITSLOP_10} from 'lib/constants' // Custom Dropdown Menu Components // == @@ -64,15 +65,9 @@ export function NativeDropdown({ accessibilityHint, triggerStyle, }: React.PropsWithChildren) { - const pal = usePalette('default') - const theme = useTheme() - const dropDownBackgroundColor = - theme.colorScheme === 'dark' ? pal.btn : pal.view const [open, setOpen] = React.useState(false) const buttonRef = React.useRef(null) const menuRef = React.useRef(null) - const {borderColor: separatorColor} = - theme.colorScheme === 'dark' ? pal.borderDark : pal.border React.useEffect(() => { function clickHandler(e: MouseEvent) { @@ -114,14 +109,27 @@ export function NativeDropdown({ return ( setOpen(o)}> - e.preventDefault()}> + } testID={testID} accessibilityRole="button" accessibilityLabel={accessibilityLabel} accessibilityHint={accessibilityHint} - onPress={() => setOpen(o => !o)} + onPointerDown={e => { + // Prevent false positive that interpret mobile scroll as a tap. + // This requires the custom onPress handler below to compensate. + // https://github.com/radix-ui/primitives/issues/1912 + e.preventDefault() + }} + onPress={() => { + if (window.event instanceof KeyboardEvent) { + // The onPointerDown hack above is not relevant to this press, so don't do anything. + return + } + // Compensate for the disabled onPointerDown above by triggering it manually. + setOpen(o => !o) + }} hitSlop={HITSLOP_10} style={triggerStyle}> {children} @@ -129,53 +137,53 @@ export function NativeDropdown({ - - {items.map((item, index) => { - if (item.label === 'separator') { - return ( - - ) - } - if (index > 1 && items[index - 1].label === 'separator') { - return ( - - - - {item.label} - - {item.icon && ( - - )} - - - ) - } - return ( + + + + ) +} + +function DropdownContent({ + items, + menuRef, +}: { + items: DropdownItem[] + menuRef: React.RefObject +}) { + const pal = usePalette('default') + const theme = useTheme() + const dropDownBackgroundColor = + theme.colorScheme === 'dark' ? pal.btn : pal.view + const {borderColor: separatorColor} = + theme.colorScheme === 'dark' ? pal.borderDark : pal.border + + return ( + + {items.map((item, index) => { + if (item.label === 'separator') { + return ( + + ) + } + if (index > 1 && items[index - 1].label === 'separator') { + return ( + @@ -190,11 +198,27 @@ export function NativeDropdown({ /> )} - ) - })} - - - + + ) + } + return ( + + + {item.label} + + {item.icon && ( + + )} + + ) + })} + ) }