Mobile: Fixes #10253: Move accessibility focus to the first note action menu item on open (#11031)

2025-10-31 00:07:48 +02:00 · 2024-09-12 01:04:23 -07:00
parent ca5d35339f
commit ea61bfc498
5 changed files with 181 additions and 90 deletions
--- a/.eslintignore
+++ b/.eslintignore
@@ -612,6 +612,7 @@ packages/app-mobile/components/NoteList.js
 packages/app-mobile/components/ProfileSwitcher/ProfileEditor.js
 packages/app-mobile/components/ProfileSwitcher/ProfileSwitcher.js
 packages/app-mobile/components/ProfileSwitcher/useProfileConfig.js
+packages/app-mobile/components/ScreenHeader/Menu.js
 packages/app-mobile/components/ScreenHeader/WarningBanner.test.js
 packages/app-mobile/components/ScreenHeader/WarningBanner.js
 packages/app-mobile/components/ScreenHeader/WarningBox.js
--- a/.gitignore
+++ b/.gitignore
@@ -589,6 +589,7 @@ packages/app-mobile/components/NoteList.js
 packages/app-mobile/components/ProfileSwitcher/ProfileEditor.js
 packages/app-mobile/components/ProfileSwitcher/ProfileSwitcher.js
 packages/app-mobile/components/ProfileSwitcher/useProfileConfig.js
+packages/app-mobile/components/ScreenHeader/Menu.js
 packages/app-mobile/components/ScreenHeader/WarningBanner.test.js
 packages/app-mobile/components/ScreenHeader/WarningBanner.js
 packages/app-mobile/components/ScreenHeader/WarningBox.js
--- a/packages/app-mobile/components/ScreenHeader/Menu.tsx
+++ b/packages/app-mobile/components/ScreenHeader/Menu.tsx
@@ -0,0 +1,148 @@
+import * as React from 'react';
+import { useCallback, useMemo, useState } from 'react';
+import { StyleSheet, TextStyle, View, Text, ScrollView, useWindowDimensions } from 'react-native';
+import { themeStyle } from '../global-style';
+import { Menu, MenuOption as MenuOptionComponent, MenuOptions, MenuTrigger } from 'react-native-popup-menu';
+import AccessibleView from '../accessibility/AccessibleView';
+import debounce from '../../utils/debounce';
+
+interface MenuOptionDivider {
+	isDivider: true;
+}
+
+interface MenuOptionButton {
+	key?: string;
+	isDivider?: false|undefined;
+	disabled?: boolean;
+	onPress: ()=> void;
+	title: string;
+}
+
+export type MenuOptionType = MenuOptionDivider|MenuOptionButton;
+
+interface Props {
+	themeId: number;
+	options: MenuOptionType[];
+	children: React.ReactNode;
+}
+
+const useStyles = (themeId: number) => {
+	const { height: windowHeight } = useWindowDimensions();
+
+	return useMemo(() => {
+		const theme = themeStyle(themeId);
+
+		const contextMenuItemTextBase: TextStyle = {
+			flex: 1,
+			textAlignVertical: 'center',
+			paddingLeft: theme.marginLeft,
+			paddingRight: theme.marginRight,
+			paddingTop: theme.itemMarginTop,
+			paddingBottom: theme.itemMarginBottom,
+			color: theme.color,
+			backgroundColor: theme.backgroundColor,
+			fontSize: theme.fontSize,
+		};
+
+		return StyleSheet.create({
+			divider: {
+				borderBottomWidth: 1,
+				borderColor: theme.dividerColor,
+				backgroundColor: '#0000ff',
+			},
+			contextMenu: {
+				backgroundColor: theme.backgroundColor2,
+			},
+			contextMenuItem: {
+				backgroundColor: theme.backgroundColor,
+			},
+			contextMenuItemText: {
+				...contextMenuItemTextBase,
+			},
+			contextMenuItemTextDisabled: {
+				...contextMenuItemTextBase,
+				opacity: 0.5,
+			},
+			menuContentScroller: {
+				maxHeight: windowHeight - 50,
+			},
+			contextMenuButton: {
+				padding: 0,
+			},
+		});
+	}, [themeId, windowHeight]);
+};
+
+const MenuComponent: React.FC<Props> = props => {
+	const styles = useStyles(props.themeId);
+
+	const menuOptionComponents: React.ReactNode[] = [];
+
+	// When undefined/null: Don't auto-focus anything.
+	const [refocusCounter, setRefocusCounter] = useState<number|undefined>(undefined);
+
+	let key = 0;
+	let isFirst = true;
+	for (const option of props.options) {
+		if (option.isDivider === true) {
+			menuOptionComponents.push(
+				<View key={`menuOption_divider_${key++}`} style={styles.divider} />,
+			);
+		} else {
+			const canAutoFocus = isFirst;
+			menuOptionComponents.push(
+				<MenuOptionComponent value={option.onPress} key={`menuOption_${option.key ?? key++}`} style={styles.contextMenuItem} disabled={!!option.disabled}>
+					<AccessibleView refocusCounter={canAutoFocus ? refocusCounter : undefined}>
+						<Text
+							style={option.disabled ? styles.contextMenuItemTextDisabled : styles.contextMenuItemText}
+							disabled={!!option.disabled}
+						>{option.title}</Text>
+					</AccessibleView>
+				</MenuOptionComponent>,
+			);
+
+			isFirst = false;
+		}
+	}
+
+	const onMenuItemSelect = useCallback((value: unknown) => {
+		if (typeof value === 'function') {
+			value();
+		}
+		setRefocusCounter(undefined);
+	}, []);
+
+	// debounce: If the menu is focused during its transition animation, it briefly
+	// appears to be in the wrong place. As such, add a brief delay before focusing.
+	const onMenuOpen = useMemo(() => debounce(() => {
+		setRefocusCounter(counter => (counter ?? 0) + 1);
+	}, 200), []);
+
+	// Resetting the refocus counter to undefined causes the menu to not be focused immediately
+	// after opening.
+	const onMenuClose = useCallback(() => {
+		setRefocusCounter(undefined);
+	}, []);
+
+	return (
+		<Menu
+			onSelect={onMenuItemSelect}
+			onClose={onMenuClose}
+			onOpen={onMenuOpen}
+			style={styles.contextMenu}
+		>
+			<MenuTrigger style={styles.contextMenuButton} testID='screen-header-menu-trigger'>
+				{props.children}
+			</MenuTrigger>
+			<MenuOptions>
+				<ScrollView
+					style={styles.menuContentScroller}
+					aria-modal={true}
+					accessibilityViewIsModal={true}
+				>{menuOptionComponents}</ScrollView>
+			</MenuOptions>
+		</Menu>
+	);
+};
+
+export default MenuComponent;
--- a/packages/app-mobile/components/ScreenHeader/index.tsx
+++ b/packages/app-mobile/components/ScreenHeader/index.tsx
@@ -1,11 +1,10 @@
 import * as React from 'react';
 import { PureComponent, ReactElement } from 'react';
 import { connect } from 'react-redux';
-import { View, Text, StyleSheet, TouchableOpacity, Image, ScrollView, Dimensions, ViewStyle, Platform } from 'react-native';
+import { View, Text, StyleSheet, TouchableOpacity, Image, ViewStyle, Platform } from 'react-native';
 const Icon = require('react-native-vector-icons/Ionicons').default;
 const { BackButtonService } = require('../../services/back-button.js');
 import NavService from '@joplin/lib/services/NavService';
-import { Menu, MenuOptions, MenuOption, MenuTrigger } from 'react-native-popup-menu';
 import { _, _n } from '@joplin/lib/locale';
 import Note from '@joplin/lib/models/Note';
 import Folder from '@joplin/lib/models/Folder';
@@ -26,27 +25,17 @@ import { Dispatch } from 'redux';
 import WarningBanner from './WarningBanner';
 import WebBetaButton from './WebBetaButton';

+import Menu, { MenuOptionType } from './Menu';
+export { MenuOptionType };
+
 // Rather than applying a padding to the whole bar, it is applied to each
 // individual component (button, picker, etc.) so that the touchable areas
 // are widder and to give more room to the picker component which has a larger
 // default height.
 const PADDING_V = 10;

-type OnSelectCallbackType=()=> void;
 type OnPressCallback=()=> void;

-export type MenuOptionType = {
-	onPress: OnPressCallback;
-	isDivider?: boolean;
-	title: string;
-	disabled?: boolean;
-}|{
-	isDivider: true;
-	title?: undefined;
-	onPress?: undefined;
-	disabled?: false;
-};
-
 interface ScreenHeaderProps {
 	selectedNoteIds: string[];
 	selectedFolderId: string;
@@ -116,11 +105,6 @@ class ScreenHeaderComponent extends PureComponent<ScreenHeaderProps, ScreenHeade
 				shadowColor: '#000000',
 				elevation: 5,
 			},
-			divider: {
-				borderBottomWidth: 1,
-				borderColor: theme.dividerColor,
-				backgroundColor: '#0000ff',
-			},
 			sideMenuButton: {
 				flex: 1,
 				alignItems: 'center',
@@ -171,23 +155,6 @@ class ScreenHeaderComponent extends PureComponent<ScreenHeaderProps, ScreenHeade
 				color: theme.color2,
 				fontWeight: 'bold',
 			},
-			contextMenu: {
-				backgroundColor: theme.backgroundColor2,
-			},
-			contextMenuItem: {
-				backgroundColor: theme.backgroundColor,
-			},
-			contextMenuItemText: {
-				flex: 1,
-				textAlignVertical: 'center',
-				paddingLeft: theme.marginLeft,
-				paddingRight: theme.marginRight,
-				paddingTop: theme.itemMarginTop,
-				paddingBottom: theme.itemMarginBottom,
-				color: theme.color,
-				backgroundColor: theme.backgroundColor,
-				fontSize: theme.fontSize,
-			},
 			titleText: {
 				flex: 1,
 				textAlignVertical: 'center',
@@ -200,10 +167,6 @@ class ScreenHeaderComponent extends PureComponent<ScreenHeaderProps, ScreenHeade
 			},
 		};

-		styleObject.contextMenuItemTextDisabled = {
-			...styleObject.contextMenuItemText,
-			opacity: 0.5,
-		};

 		styleObject.topIcon = { ...theme.icon };
 		styleObject.topIcon.flex = 1;
@@ -289,12 +252,6 @@ class ScreenHeaderComponent extends PureComponent<ScreenHeaderProps, ScreenHeade
 		}
 	}

-	private menu_select(value: OnSelectCallbackType) {
-		if (typeof value === 'function') {
-			value();
-		}
-	}
-
 	public render() {
 		const themeId = this.props.themeId;
 		// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
@@ -537,45 +494,27 @@ class ScreenHeaderComponent extends PureComponent<ScreenHeaderProps, ScreenHeade
 			);
 		}

-		let key = 0;
-		const menuOptionComponents = [];
+		const menuOptions: MenuOptionType[] = [...this.props.menuOptions];

 		const selectedFolder = this.props.notesParentType === 'Folder' ? Folder.byId(this.props.folders, this.props.selectedFolderId) : null;
 		const selectedFolderInTrash = itemIsInTrash(selectedFolder);

 		if (!this.props.noteSelectionEnabled) {
-			for (let i = 0; i < this.props.menuOptions.length; i++) {
-				const o = this.props.menuOptions[i];
-
-				if (o.isDivider) {
-					menuOptionComponents.push(<View key={`menuOption_${key++}`} style={this.styles().divider} />);
-				} else {
-					menuOptionComponents.push(
-						<MenuOption value={o.onPress} key={`menuOption_${key++}`} style={this.styles().contextMenuItem} disabled={!!o.disabled}>
-							<Text
-								style={o.disabled ? this.styles().contextMenuItemTextDisabled : this.styles().contextMenuItemText}
-								disabled={!!o.disabled}
-							>{o.title}</Text>
-						</MenuOption>,
-					);
-				}
-			}
-
-			if (menuOptionComponents.length) {
-				menuOptionComponents.push(<View key={`menuOption_${key++}`} style={this.styles().divider} />);
+			if (menuOptions.length) {
+				menuOptions.push({ isDivider: true });
 			}
 		} else {
-			menuOptionComponents.push(
-				<MenuOption value={() => this.deleteButton_press()} key={'menuOption_delete'} style={this.styles().contextMenuItem}>
-					<Text style={this.styles().contextMenuItemText}>{_('Delete')}</Text>
-				</MenuOption>,
-			);
+			menuOptions.push({
+				key: 'delete',
+				title: _('Delete'),
+				onPress: this.deleteButton_press,
+			});

-			menuOptionComponents.push(
-				<MenuOption value={() => this.duplicateButton_press()} key={'menuOption_duplicate'} style={this.styles().contextMenuItem}>
-					<Text style={this.styles().contextMenuItemText}>{_('Duplicate')}</Text>
-				</MenuOption>,
-			);
+			menuOptions.push({
+				key: 'duplicate',
+				title: _('Duplicate'),
+				onPress: this.duplicateButton_press,
+			});
 		}

 		const createTitleComponent = (disabled: boolean, hideableAfterTitleComponents: ReactElement) => {
@@ -661,7 +600,6 @@ class ScreenHeaderComponent extends PureComponent<ScreenHeaderProps, ScreenHeade
 		</>;

 		const titleComp = createTitleComponent(headerItemDisabled, hideableRightComponents);
-		const windowHeight = Dimensions.get('window').height - 50;

 		const contextMenuStyle: ViewStyle = {
 			paddingTop: PADDING_V,
@@ -672,16 +610,11 @@ class ScreenHeaderComponent extends PureComponent<ScreenHeaderProps, ScreenHeade
 		if (this.props.noteSelectionEnabled) contextMenuStyle.width = 1;

 		const menuComp =
-			!menuOptionComponents.length || !showContextMenuButton ? null : (
-				<Menu onSelect={value => this.menu_select(value)} style={this.styles().contextMenu}>
-					<MenuTrigger style={contextMenuStyle} testID='screen-header-menu-trigger'>
-						<View accessibilityLabel={_('Actions')}>
-							<Icon name="ellipsis-vertical" style={this.styles().contextMenuTrigger} />
-						</View>
-					</MenuTrigger>
-					<MenuOptions>
-						<ScrollView style={{ maxHeight: windowHeight }}>{menuOptionComponents}</ScrollView>
-					</MenuOptions>
+			!menuOptions.length || !showContextMenuButton ? null : (
+				<Menu themeId={this.props.themeId} options={menuOptions}>
+					<View style={contextMenuStyle} accessibilityLabel={_('Actions')}>
+						<Icon name="ellipsis-vertical" style={this.styles().contextMenuTrigger} />
+					</View>
 				</Menu>
 			);

--- a/packages/app-mobile/components/accessibility/AccessibleView.tsx
+++ b/packages/app-mobile/components/accessibility/AccessibleView.tsx
@@ -1,8 +1,11 @@
 import { focus } from '@joplin/lib/utils/focusHandler';
+import Logger from '@joplin/utils/Logger';
 import * as React from 'react';
 import { useEffect, useState } from 'react';
 import { AccessibilityInfo, findNodeHandle, Platform, UIManager, View, ViewProps } from 'react-native';

+const logger = Logger.create('AccessibleView');
+
 interface Props extends ViewProps {
 	// Prevents a view from being interacted with by accessibility tools, the mouse, or the keyboard focus.
 	// See https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/inert.
@@ -31,6 +34,7 @@ const AccessibleView: React.FC<Props> = ({ inert, refocusCounter, children, ...v

 	useEffect(() => {
 		if ((refocusCounter ?? null) === null) return;
+		if (!containerRef) return;

 		const autoFocus = () => {
 			if (Platform.OS === 'web') {
@@ -48,7 +52,11 @@ const AccessibleView: React.FC<Props> = ({ inert, refocusCounter, children, ...v
 				UIManager.focus(containerRef);
 			} else {
 				const handle = findNodeHandle(containerRef as View);
-				AccessibilityInfo.setAccessibilityFocus(handle);
+				if (handle !== null) {
+					AccessibilityInfo.setAccessibilityFocus(handle);
+				} else {
+					logger.warn('Couldn\'t find a view to focus.');
+				}
 			}
 		};