Skip to content

Commit

Permalink
Merge pull request #39201 from software-mansion-labs/fix/sync-arrows-…
Browse files Browse the repository at this point in the history
…and-tab-navigation

Sync keyboard navigation in SelectionList and PopoverMenu
  • Loading branch information
roryabraham authored Apr 12, 2024
2 parents 4330e41 + 9bf097d commit 8d371c5
Show file tree
Hide file tree
Showing 14 changed files with 232 additions and 145 deletions.
9 changes: 7 additions & 2 deletions src/components/ContextMenuItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import useWindowDimensions from '@hooks/useWindowDimensions';
import getButtonState from '@libs/getButtonState';
import type IconAsset from '@src/types/utils/IconAsset';
import BaseMiniContextMenuItem from './BaseMiniContextMenuItem';
import FocusableMenuItem from './FocusableMenuItem';
import Icon from './Icon';
import MenuItem from './MenuItem';

type ContextMenuItemProps = {
/** Icon Component */
Expand Down Expand Up @@ -49,6 +49,9 @@ type ContextMenuItemProps = {

/** The ref of mini context menu item */
buttonRef?: React.RefObject<View>;

/** Handles what to do when the item is focused */
onFocus?: () => void;
};

type ContextMenuItemHandle = {
Expand All @@ -70,6 +73,7 @@ function ContextMenuItem(
wrapperStyle,
shouldPreventDefaultFocusOnPress = true,
buttonRef = {current: null},
onFocus = () => {},
}: ContextMenuItemProps,
ref: ForwardedRef<ContextMenuItemHandle>,
) {
Expand Down Expand Up @@ -113,7 +117,7 @@ function ContextMenuItem(
)}
</BaseMiniContextMenuItem>
) : (
<MenuItem
<FocusableMenuItem
title={itemText}
icon={itemIcon}
onPress={triggerPressAndUpdateSuccess}
Expand All @@ -125,6 +129,7 @@ function ContextMenuItem(
isAnonymousAction={isAnonymousAction}
focused={isFocused}
interactive={isThrottledButtonActive}
onFocus={onFocus}
/>
);
}
Expand Down
24 changes: 24 additions & 0 deletions src/components/FocusableMenuItem.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React, {useRef} from 'react';
import type {View} from 'react-native';
import useSyncFocus from '@hooks/useSyncFocus';
import type {MenuItemProps} from './MenuItem';
import MenuItem from './MenuItem';

function FocusableMenuItem(props: MenuItemProps) {
const ref = useRef<HTMLDivElement | View>(null);

// Sync focus on an item
useSyncFocus(ref, Boolean(props.focused));

return (
<MenuItem
// eslint-disable-next-line react/jsx-props-no-spreading
{...props}
ref={ref}
/>
);
}

FocusableMenuItem.displayName = 'FocusableMenuItem';

export default FocusableMenuItem;
4 changes: 3 additions & 1 deletion src/components/Hoverable/index.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type {Ref} from 'react';
import React, {cloneElement, forwardRef} from 'react';
import {hasHoverSupport} from '@libs/DeviceCapabilities';
import mergeRefs from '@libs/mergeRefs';
import {getReturnValue} from '@libs/ValueUtils';
import ActiveHoverable from './ActiveHoverable';
import type HoverableProps from './types';
Expand All @@ -14,7 +15,8 @@ function Hoverable({isDisabled, ...props}: HoverableProps, ref: Ref<HTMLElement>
// If Hoverable is disabled, just render the child without additional logic or event listeners.
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
if (isDisabled || !hasHoverSupport()) {
return cloneElement(getReturnValue(props.children, false), {ref});
const child = getReturnValue(props.children, false);
return cloneElement(child, {ref: mergeRefs(ref, child.ref)});
}

return (
Expand Down
12 changes: 9 additions & 3 deletions src/components/MenuItem.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import ExpensiMark from 'expensify-common/lib/ExpensiMark';
import type {ImageContentFit} from 'expo-image';
import type {ForwardedRef, ReactNode} from 'react';
import type {ReactNode} from 'react';
import React, {forwardRef, useContext, useMemo} from 'react';
import type {GestureResponderEvent, StyleProp, TextStyle, ViewStyle} from 'react-native';
import {View} from 'react-native';
Expand Down Expand Up @@ -32,6 +32,7 @@ import * as Expensicons from './Icon/Expensicons';
import * as defaultWorkspaceAvatars from './Icon/WorkspaceDefaultAvatars';
import {MenuItemGroupContext} from './MenuItemGroup';
import MultipleAvatars from './MultipleAvatars';
import type {PressableRef} from './Pressable/GenericPressable/types';
import PressableWithSecondaryInteraction from './PressableWithSecondaryInteraction';
import RenderHTML from './RenderHTML';
import SelectCircle from './SelectCircle';
Expand Down Expand Up @@ -249,6 +250,9 @@ type MenuItemBaseProps = {

/** Adds padding to the left of the text when there is no icon. */
shouldPutLeftPaddingWhenNoIcon?: boolean;

/** Handles what to do when the item is focused */
onFocus?: () => void;
};

type MenuItemProps = (IconProps | AvatarProps | NoIcon) & MenuItemBaseProps;
Expand Down Expand Up @@ -321,8 +325,9 @@ function MenuItem(
contentFit = 'cover',
isPaneMenu = false,
shouldPutLeftPaddingWhenNoIcon = false,
onFocus,
}: MenuItemProps,
ref: ForwardedRef<View>,
ref: PressableRef,
) {
const theme = useTheme();
const styles = useThemeStyles();
Expand Down Expand Up @@ -449,6 +454,7 @@ function MenuItem(
role={CONST.ROLE.MENUITEM}
accessibilityLabel={title ? title.toString() : ''}
accessible
onFocus={onFocus}
>
{({pressed}) => (
<>
Expand Down Expand Up @@ -678,5 +684,5 @@ function MenuItem(

MenuItem.displayName = 'MenuItem';

export type {IconProps, AvatarProps, NoIcon, MenuItemBaseProps, MenuItemProps};
export type {AvatarProps, IconProps, MenuItemBaseProps, MenuItemProps, NoIcon};
export default forwardRef(MenuItem);
4 changes: 3 additions & 1 deletion src/components/PopoverMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import useWindowDimensions from '@hooks/useWindowDimensions';
import CONST from '@src/CONST';
import type {AnchorPosition} from '@src/styles';
import type AnchorAlignment from '@src/types/utils/AnchorAlignment';
import FocusableMenuItem from './FocusableMenuItem';
import * as Expensicons from './Icon/Expensicons';
import type {MenuItemProps} from './MenuItem';
import MenuItem from './MenuItem';
Expand Down Expand Up @@ -193,7 +194,7 @@ function PopoverMenu({
{!!headerText && <Text style={[styles.createMenuHeaderText, styles.ml3]}>{headerText}</Text>}
{enteredSubMenuIndexes.length > 0 && renderBackButtonItem()}
{currentMenuItems.map((item, menuIndex) => (
<MenuItem
<FocusableMenuItem
key={item.text}
icon={item.icon}
iconWidth={item.iconWidth}
Expand All @@ -216,6 +217,7 @@ function PopoverMenu({
floatRightAvatarSize={item.floatRightAvatarSize}
shouldShowSubscriptRightAvatar={item.shouldShowSubscriptRightAvatar}
disabled={item.disabled}
onFocus={() => setFocusedIndex(menuIndex)}
/>
))}
</View>
Expand Down
12 changes: 11 additions & 1 deletion src/components/SelectionList/BaseListItem.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React from 'react';
import React, {useRef} from 'react';
import {View} from 'react-native';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import OfflineWithFeedback from '@components/OfflineWithFeedback';
import PressableWithFeedback from '@components/Pressable/PressableWithFeedback';
import useHover from '@hooks/useHover';
import useSyncFocus from '@hooks/useSyncFocus';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import CONST from '@src/CONST';
Expand All @@ -26,11 +27,18 @@ function BaseListItem<TItem extends ListItem>({
pendingAction,
FooterComponent,
children,
isFocused,
onFocus = () => {},
}: BaseListItemProps<TItem>) {
const theme = useTheme();
const styles = useThemeStyles();
const {hovered, bind} = useHover();

const pressableRef = useRef<View | HTMLDivElement>(null);

// Sync focus on an item
useSyncFocus(pressableRef, Boolean(isFocused));

const rightHandSideComponentRender = () => {
if (canSelectMultiple || !rightHandSideComponent) {
return null;
Expand All @@ -54,6 +62,7 @@ function BaseListItem<TItem extends ListItem>({
<PressableWithFeedback
// eslint-disable-next-line react/jsx-props-no-spreading
{...bind}
ref={pressableRef}
onPress={() => onSelectRow(item)}
disabled={isDisabled}
accessibilityLabel={item.text ?? ''}
Expand All @@ -64,6 +73,7 @@ function BaseListItem<TItem extends ListItem>({
onMouseDown={shouldPreventDefaultFocusOnSelectRow ? (e) => e.preventDefault() : undefined}
nativeID={keyForList ?? ''}
style={pressableStyle}
onFocus={onFocus}
>
<View style={wrapperStyle}>
{typeof children === 'function' ? children(hovered) : children}
Expand Down
Loading

0 comments on commit 8d371c5

Please sign in to comment.