iOS: Accessibility: Fix to-do completion status can't be changed from the note list (#12101)

2025-08-10 22:11:50 +02:00 · 2025-04-14 09:28:35 -07:00
parent 62ca6cb70b
commit 9871717de4
5 changed files with 122 additions and 39 deletions
--- a/.eslintignore
+++ b/.eslintignore
@@ -711,6 +711,7 @@ packages/app-mobile/components/biometrics/biometricAuthenticate.js
 packages/app-mobile/components/biometrics/sensorInfo.js
 packages/app-mobile/components/buttons/FloatingActionButton.js
 packages/app-mobile/components/buttons/LabelledIconButton.js
+packages/app-mobile/components/buttons/MultiTouchableOpacity.js
 packages/app-mobile/components/buttons/TextButton.js
 packages/app-mobile/components/buttons/index.js
 packages/app-mobile/components/getResponsiveValue.test.js
--- a/.gitignore
+++ b/.gitignore
@@ -686,6 +686,7 @@ packages/app-mobile/components/biometrics/biometricAuthenticate.js
 packages/app-mobile/components/biometrics/sensorInfo.js
 packages/app-mobile/components/buttons/FloatingActionButton.js
 packages/app-mobile/components/buttons/LabelledIconButton.js
+packages/app-mobile/components/buttons/MultiTouchableOpacity.js
 packages/app-mobile/components/buttons/TextButton.js
 packages/app-mobile/components/buttons/index.js
 packages/app-mobile/components/getResponsiveValue.test.js
--- a/packages/app-mobile/components/Checkbox.tsx
+++ b/packages/app-mobile/components/Checkbox.tsx
@@ -1,11 +1,13 @@
 import * as React from 'react';
 import { useState, useEffect, useCallback, useMemo } from 'react';
-import { TouchableHighlight, StyleSheet, TextStyle } from 'react-native';
+import { TouchableHighlight, StyleSheet, TextStyle, AccessibilityInfo } from 'react-native';
 import Icon from './Icon';
+import { _ } from '@joplin/lib/locale';

 interface Props {
 	checked: boolean;
 	accessibilityLabel?: string;
+	accessibilityHint?: string;
 	onChange?: (checked: boolean)=> void;
 	style?: TextStyle;
 	iconStyle?: TextStyle;
@@ -40,6 +42,15 @@ const Checkbox: React.FC<Props> = props => {
 		setChecked(checked => {
 			const newChecked = !checked;
 			props.onChange?.(newChecked);
+
+			// An .announceForAccessibility is necessary here because VoiceOver doesn't announce this
+			// change on its own, even though we change [accessibilityState].
+			if (newChecked) {
+				AccessibilityInfo.announceForAccessibility(_('Checked'));
+			} else {
+				AccessibilityInfo.announceForAccessibility(_('Unchecked'));
+			}
+
 			return newChecked;
 		});
 	}, [props.onChange]);
@@ -58,6 +69,7 @@ const Checkbox: React.FC<Props> = props => {
 			accessibilityRole="checkbox"
 			accessibilityState={accessibilityState}
 			accessibilityLabel={props.accessibilityLabel ?? ''}
+			accessibilityHint={props.accessibilityHint}
 			// Web requires aria-checked
 			aria-checked={checked}
 		>
--- a/packages/app-mobile/components/NoteItem.tsx
+++ b/packages/app-mobile/components/NoteItem.tsx
@@ -1,7 +1,7 @@
 import * as React from 'react';
 import { memo, useCallback, useMemo } from 'react';
 import { connect } from 'react-redux';
-import { Text, View, StyleSheet, TextStyle, ViewStyle, AccessibilityInfo, TouchableOpacity } from 'react-native';
+import { Text, StyleSheet, TextStyle, ViewStyle, AccessibilityInfo } from 'react-native';
 import Checkbox from './Checkbox';
 import Note from '@joplin/lib/models/Note';
 import time from '@joplin/lib/time';
@@ -11,6 +11,7 @@ import { AppState } from '../utils/types';
 import { Dispatch } from 'redux';
 import { NoteEntity } from '@joplin/lib/services/database/types';
 import useOnLongPressProps from '../utils/hooks/useOnLongPressProps';
+import MultiTouchableOpacity from './buttons/MultiTouchableOpacity';

 interface Props {
 	dispatch: Dispatch;
@@ -31,18 +32,25 @@ const useStyles = (themeId: number) => {
 			borderBottomWidth: 1,
 			borderBottomColor: theme.dividerColor,
 			alignItems: 'flex-start',
+			// backgroundColor: theme.backgroundColor,
+		};
+
+		const listItemPressable: ViewStyle = {
+			flexGrow: 1,
+			alignSelf: 'stretch',
+		};
+		const listItemPressableWithCheckbox: ViewStyle = {
+			...listItemPressable,
+			paddingRight: theme.marginRight,
+		};
+		const listItemPressableWithoutCheckbox: ViewStyle = {
+			...listItemPressable,
 			paddingLeft: theme.marginLeft,
 			paddingRight: theme.marginRight,
 			paddingTop: theme.itemMarginTop,
 			paddingBottom: theme.itemMarginBottom,
-			// backgroundColor: theme.backgroundColor,
 		};

-		const listItemWithCheckbox = { ...listItem };
-		delete listItemWithCheckbox.paddingTop;
-		delete listItemWithCheckbox.paddingBottom;
-		delete listItemWithCheckbox.paddingLeft;
-
 		const listItemText: TextStyle = {
 			flex: 1,
 			color: theme.color,
@@ -62,7 +70,8 @@ const useStyles = (themeId: number) => {
 			listItem,
 			listItemText,
 			selectionWrapper,
-			listItemWithCheckbox,
+			listItemPressableWithoutCheckbox,
+			listItemPressableWithCheckbox,
 			listItemTextWithCheckbox,
 			selectionWrapperSelected,
 			checkboxStyle: {
@@ -132,7 +141,6 @@ const NoteItemComponent: React.FC<Props> = memo(props => {
 	const checkboxChecked = !!Number(note.todo_completed);

 	const checkboxStyle = styles.checkboxStyle;
-	const listItemStyle = isTodo ? styles.listItemWithCheckbox : styles.listItem;
 	const listItemTextStyle = isTodo ? styles.listItemTextWithCheckbox : styles.listItemText;
 	const opacityStyle = isTodo && checkboxChecked ? styles.checkedOpacityStyle : styles.uncheckedOpacityStyle;
 	const isSelected = props.noteSelectionEnabled && props.selectedNoteIds.includes(note.id);
@@ -140,41 +148,34 @@ const NoteItemComponent: React.FC<Props> = memo(props => {
 	const selectionWrapperStyle = isSelected ? styles.selectionWrapperSelected : styles.selectionWrapper;

 	const noteTitle = Note.displayTitle(note);
-
 	const selectDeselectLabel = isSelected ? _('Deselect') : _('Select');
 	const onLongPressProps = useOnLongPressProps({ onLongPress, actionDescription: selectDeselectLabel });

-	const contextMenuProps = {
-		// Web only.
-		onContextMenu: onLongPressProps.onContextMenu,
+	const todoCheckbox = isTodo ? <Checkbox
+		style={checkboxStyle}
+		checked={checkboxChecked}
+		onChange={todoCheckbox_change}
+		accessibilityLabel={_('to-do: %s', noteTitle)}
+	/> : null;
+
+	const pressableProps = {
+		style: isTodo ? styles.listItemPressableWithCheckbox : styles.listItemPressableWithoutCheckbox,
+		accessibilityHint: props.noteSelectionEnabled ? '' : _('Opens note'),
+		'aria-pressed': props.noteSelectionEnabled ? isSelected : undefined,
+		accessibilityState: { selected: isSelected },
+		...onLongPressProps,
 	};
 	return (
-		<View
-			// context menu listeners need to be added to a parent view of the
-			// TouchableOpacity -- on web, TouchableOpacity registers a custom
-			// onContextMenu handler that can't be overridden.
-			{...contextMenuProps}
+		<MultiTouchableOpacity
+			containerProps={{
+				style: [selectionWrapperStyle, opacityStyle, styles.listItem],
+			}}
+			pressableProps={pressableProps}
+			onPress={onPress}
+			beforePressable={todoCheckbox}
 		>
-			<TouchableOpacity
-				activeOpacity={0.5}
-				onPress={onPress}
-				accessibilityRole='button'
-				accessibilityHint={props.noteSelectionEnabled ? '' : _('Opens note')}
-				aria-pressed={props.noteSelectionEnabled ? isSelected : undefined}
-				accessibilityState={{ selected: isSelected }}
-				{...onLongPressProps}
-			>
-				<View style={[selectionWrapperStyle, opacityStyle, listItemStyle]}>
-					{isTodo ? <Checkbox
-						style={checkboxStyle}
-						checked={checkboxChecked}
-						onChange={todoCheckbox_change}
-						accessibilityLabel={_('to-do: %s', noteTitle)}
-					/> : null }
-					<Text style={listItemTextStyle}>{noteTitle}</Text>
-				</View>
-			</TouchableOpacity>
-		</View>
+			<Text style={listItemTextStyle}>{noteTitle}</Text>
+		</MultiTouchableOpacity>
 	);
 });

--- a/packages/app-mobile/components/buttons/MultiTouchableOpacity.tsx
+++ b/packages/app-mobile/components/buttons/MultiTouchableOpacity.tsx
@@ -0,0 +1,68 @@
+import * as React from 'react';
+import { useCallback, useMemo, useRef } from 'react';
+import { Animated, StyleSheet, Pressable, ViewProps, PressableProps } from 'react-native';
+
+interface Props {
+	// Nodes that need to change opacity but shouldn't be included in the main touchable
+	beforePressable: React.ReactNode;
+	// Children of the main pressable
+	children: React.ReactNode;
+	onPress: ()=> void;
+
+	pressableProps?: PressableProps;
+	containerProps?: ViewProps;
+}
+
+// A TouchableOpacity that can contain multiple pressable items still within the region that
+// changes opacity
+const MultiTouchableOpacity: React.FC<Props> = props => {
+	// See https://blog.logrocket.com/react-native-touchable-vs-pressable-components/
+	// for more about animating Pressable buttons.
+	const fadeAnim = useRef(new Animated.Value(1)).current;
+
+	const animationDuration = 100; // ms
+	const onPressIn = useCallback(() => {
+		// Fade out.
+		Animated.timing(fadeAnim, {
+			toValue: 0.5,
+			duration: animationDuration,
+			useNativeDriver: true,
+		}).start();
+	}, [fadeAnim]);
+	const onPressOut = useCallback(() => {
+		// Fade in.
+		Animated.timing(fadeAnim, {
+			toValue: 1,
+			duration: animationDuration,
+			useNativeDriver: true,
+		}).start();
+	}, [fadeAnim]);
+
+	const button = (
+		<Pressable
+			accessibilityRole='button'
+			{...props.pressableProps}
+			onPress={props.onPress}
+			onPressIn={onPressIn}
+			onPressOut={onPressOut}
+		>
+			{props.children}
+		</Pressable>
+	);
+
+	const styles = useMemo(() => {
+		return StyleSheet.create({
+			container: { opacity: fadeAnim },
+		});
+	}, [fadeAnim]);
+
+	const containerProps = props.containerProps ?? {};
+	return (
+		<Animated.View {...containerProps} style={[styles.container, props.containerProps.style]}>
+			{props.beforePressable}
+			{button}
+		</Animated.View>
+	);
+};
+
+export default MultiTouchableOpacity;