Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mobile: Accessibility: Improve focus handling in the note actions menu and modal dialogs #11929

Open
wants to merge 15 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,14 @@ packages/app-mobile/components/SideMenuContentNote.js
packages/app-mobile/components/TextInput.js
packages/app-mobile/components/ToggleSpaceButton.js
packages/app-mobile/components/accessibility/AccessibleModalMenu.js
packages/app-mobile/components/accessibility/AccessibleView.test.js
packages/app-mobile/components/accessibility/AccessibleView.js
packages/app-mobile/components/accessibility/FocusControl/AutoFocusProvider.js
packages/app-mobile/components/accessibility/FocusControl/FocusControl.js
packages/app-mobile/components/accessibility/FocusControl/FocusControlProvider.js
packages/app-mobile/components/accessibility/FocusControl/MainAppContent.js
packages/app-mobile/components/accessibility/FocusControl/ModalWrapper.js
packages/app-mobile/components/accessibility/FocusControl/types.js
packages/app-mobile/components/app-nav.js
packages/app-mobile/components/base-screen.js
packages/app-mobile/components/biometrics/BiometricPopup.js
Expand Down
7 changes: 7 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,14 @@ packages/app-mobile/components/SideMenuContentNote.js
packages/app-mobile/components/TextInput.js
packages/app-mobile/components/ToggleSpaceButton.js
packages/app-mobile/components/accessibility/AccessibleModalMenu.js
packages/app-mobile/components/accessibility/AccessibleView.test.js
packages/app-mobile/components/accessibility/AccessibleView.js
packages/app-mobile/components/accessibility/FocusControl/AutoFocusProvider.js
packages/app-mobile/components/accessibility/FocusControl/FocusControl.js
packages/app-mobile/components/accessibility/FocusControl/FocusControlProvider.js
packages/app-mobile/components/accessibility/FocusControl/MainAppContent.js
packages/app-mobile/components/accessibility/FocusControl/ModalWrapper.js
packages/app-mobile/components/accessibility/FocusControl/types.js
packages/app-mobile/components/app-nav.js
packages/app-mobile/components/base-screen.js
packages/app-mobile/components/biometrics/BiometricPopup.js
Expand Down
75 changes: 58 additions & 17 deletions packages/app-mobile/components/Modal.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import * as React from 'react';
import { RefObject, useCallback, useMemo, useRef } from 'react';
import { GestureResponderEvent, Modal, ModalProps, ScrollView, StyleSheet, View, ViewStyle, useWindowDimensions } from 'react-native';
import { RefObject, useCallback, useMemo, useRef, useState } from 'react';
import { GestureResponderEvent, Modal, ModalProps, Platform, ScrollView, StyleSheet, View, ViewStyle, useWindowDimensions } from 'react-native';
import { hasNotch } from 'react-native-device-info';
import FocusControl from './accessibility/FocusControl/FocusControl';
import { msleep, Second } from '@joplin/utils/time';
import useAsyncEffect from '@joplin/lib/hooks/useAsyncEffect';
import { ModalState } from './accessibility/FocusControl/types';

interface ModalElementProps extends ModalProps {
children: React.ReactNode;
Expand Down Expand Up @@ -67,6 +71,36 @@ const useBackgroundTouchListeners = (onRequestClose: (event: GestureResponderEve
return { onShouldBackgroundCaptureTouch, onBackgroundTouchFinished };
};

const useModalStatus = (containerComponent: View|null, visible: boolean) => {
const contentMounted = !!containerComponent;
const [controlsFocus, setControlsFocus] = useState(false);
useAsyncEffect(async (event) => {
if (contentMounted) {
setControlsFocus(true);
} else {
// Accessibility: Work around Android's default focus-setting behavior.
// By default, React Native's Modal on Android sets focus about 0.8 seconds
// after the modal is dismissed. As a result, the Modal controls focus until
// roughly one second after the modal is dismissed.
if (Platform.OS === 'android') {
await msleep(Second);
}

if (!event.cancelled) {
setControlsFocus(false);
}
}
}, [contentMounted]);

let modalStatus = ModalState.Closed;
if (controlsFocus) {
modalStatus = visible ? ModalState.Open : ModalState.Closing;
} else if (visible) {
modalStatus = ModalState.Open;
}
return modalStatus;
};

const ModalElement: React.FC<ModalElementProps> = ({
children,
containerStyle,
Expand All @@ -84,29 +118,36 @@ const ModalElement: React.FC<ModalElementProps> = ({
</View>
);

const backgroundRef = useRef<View>();
const { onShouldBackgroundCaptureTouch, onBackgroundTouchFinished } = useBackgroundTouchListeners(modalProps.onRequestClose, backgroundRef);

const [containerComponent, setContainerComponent] = useState<View|null>(null);
const modalStatus = useModalStatus(containerComponent, modalProps.visible);

const containerRef = useRef<View|null>(null);
containerRef.current = containerComponent;
const { onShouldBackgroundCaptureTouch, onBackgroundTouchFinished } = useBackgroundTouchListeners(modalProps.onRequestClose, containerRef);

const contentAndBackdrop = <View
ref={backgroundRef}
ref={setContainerComponent}
style={styles.modalBackground}
onStartShouldSetResponder={onShouldBackgroundCaptureTouch}
onResponderRelease={onBackgroundTouchFinished}
>{content}</View>;

// supportedOrientations: On iOS, this allows the dialog to be shown in non-portrait orientations.
return (
<Modal
supportedOrientations={['portrait', 'portrait-upside-down', 'landscape', 'landscape-left', 'landscape-right']}
{...modalProps}
>
{scrollOverflow ? (
<ScrollView
style={styles.modalScrollView}
contentContainerStyle={styles.modalScrollViewContent}
>{contentAndBackdrop}</ScrollView>
) : contentAndBackdrop}
</Modal>
<FocusControl.ModalWrapper state={modalStatus}>
<Modal
// supportedOrientations: On iOS, this allows the dialog to be shown in non-portrait orientations.
supportedOrientations={['portrait', 'portrait-upside-down', 'landscape', 'landscape-left', 'landscape-right']}
{...modalProps}
>
{scrollOverflow ? (
<ScrollView
style={styles.modalScrollView}
contentContainerStyle={styles.modalScrollViewContent}
>{contentAndBackdrop}</ScrollView>
) : contentAndBackdrop}
</Modal>
</FocusControl.ModalWrapper>
);
};

Expand Down
36 changes: 22 additions & 14 deletions packages/app-mobile/components/ScreenHeader/Menu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ 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';
import FocusControl from '../accessibility/FocusControl/FocusControl';
import { ModalState } from '../accessibility/FocusControl/types';

interface MenuOptionDivider {
isDivider: true;
Expand Down Expand Up @@ -81,18 +83,19 @@ const MenuComponent: React.FC<Props> = props => {
// When undefined/null: Don't auto-focus anything.
const [refocusCounter, setRefocusCounter] = useState<number|undefined>(undefined);

let key = 0;
let keyCounter = 0;
let isFirst = true;
for (const option of props.options) {
if (option.isDivider === true) {
menuOptionComponents.push(
<View key={`menuOption_divider_${key++}`} style={styles.divider} />,
<View key={`menuOption_divider_${keyCounter++}`} style={styles.divider} />,
);
} else {
const canAutoFocus = isFirst;
const key = `menuOption_${option.key ?? keyCounter++}`;
menuOptionComponents.push(
<MenuOptionComponent value={option.onPress} key={`menuOption_${option.key ?? key++}`} style={styles.contextMenuItem} disabled={!!option.disabled}>
<AccessibleView refocusCounter={canAutoFocus ? refocusCounter : undefined}>
<MenuOptionComponent value={option.onPress} key={key} style={styles.contextMenuItem} disabled={!!option.disabled}>
<AccessibleView refocusCounter={canAutoFocus ? refocusCounter : undefined} testID={key}>
<Text
style={option.disabled ? styles.contextMenuItemTextDisabled : styles.contextMenuItemText}
disabled={!!option.disabled}
Expand All @@ -105,42 +108,47 @@ const MenuComponent: React.FC<Props> = props => {
}
}

const [open, setOpen] = useState(false);

const onMenuItemSelect = useCallback((value: unknown) => {
if (typeof value === 'function') {
value();
}
setRefocusCounter(undefined);
setOpen(false);
}, []);

// 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(() => {
const onMenuOpened = useMemo(() => debounce(() => {
setRefocusCounter(counter => (counter ?? 0) + 1);
setOpen(true);
}, 200), []);

// Resetting the refocus counter to undefined causes the menu to not be focused immediately
// after opening.
const onMenuClose = useCallback(() => {
const onMenuClosed = useCallback(() => {
setRefocusCounter(undefined);
setOpen(false);
}, []);

return (
<Menu
onSelect={onMenuItemSelect}
onClose={onMenuClose}
onOpen={onMenuOpen}
onClose={onMenuClosed}
onOpen={onMenuOpened}
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}
testID={`menu-content-${refocusCounter ? 'refocusing' : ''}`}
>{menuOptionComponents}</ScrollView>
<FocusControl.ModalWrapper state={open ? ModalState.Open : ModalState.Closed}>
<ScrollView
style={styles.menuContentScroller}
testID={`menu-content-${refocusCounter ? 'refocusing' : ''}`}
>{menuOptionComponents}</ScrollView>
</FocusControl.ModalWrapper>
</MenuOptions>
</Menu>
);
Expand Down
5 changes: 4 additions & 1 deletion packages/app-mobile/components/SideMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -271,12 +271,14 @@ const SideMenuComponent: React.FC<Props> = props => {
<AccessibleView
inert={!open}
style={styles.menuWrapper}
testID='menu-wrapper'
>
<AccessibleView
// Auto-focuses an empty view at the beginning of the sidemenu -- if we instead
// focus the container view, VoiceOver fails to focus to any components within
// the sidebar.
refocusCounter={!open ? 1 : undefined}
testID='sidemenu-menu-focus-region'
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, testID, if provided, is included in the focus log message. It may make sense to use a different prop for this (e.g. debugTag) and make it required if refocusCounter is specified.

/>

{props.menu}
Expand All @@ -287,8 +289,9 @@ const SideMenuComponent: React.FC<Props> = props => {
<AccessibleView
inert={open}
style={styles.contentWrapper}
testID='content-wrapper'
>
<AccessibleView refocusCounter={open ? 1 : undefined} />
<AccessibleView refocusCounter={open ? 1 : undefined} testID='sidemenu-content-focus-region' />
{props.children}
</AccessibleView>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import * as React from 'react';
import FocusControl from './FocusControl/FocusControl';
import { render } from '@testing-library/react-native';
import AccessibleView from './AccessibleView';
import { AccessibilityInfo } from 'react-native';
import ModalWrapper from './FocusControl/ModalWrapper';
import { ModalState } from './FocusControl/types';

interface TestContentWrapperProps {
mainContent: React.ReactNode;
dialogs: React.ReactNode;
}

const TestContentWrapper: React.FC<TestContentWrapperProps> = props => {
return <FocusControl.Provider>
{props.dialogs}
<FocusControl.MainAppContent>
{props.mainContent}
</FocusControl.MainAppContent>
</FocusControl.Provider>;
};

jest.mock('react-native', () => {
const ReactNative = jest.requireActual('react-native');
ReactNative.AccessibilityInfo.setAccessibilityFocus = jest.fn();
return ReactNative;
});

describe('AccessibleView', () => {
test('should wait for the currently-open dialog to dismiss before applying focus requests', () => {
const setFocusMock = AccessibilityInfo.setAccessibilityFocus as jest.Mock;
setFocusMock.mockClear();

interface TestContentOptions {
modalState: ModalState;
refocusCounter: undefined|number;
}
const renderTestContent = ({ modalState, refocusCounter }: TestContentOptions) => {
const mainContent = <AccessibleView refocusCounter={refocusCounter}/>;
const visibleDialog = <ModalWrapper state={modalState}>{null}</ModalWrapper>;
return <TestContentWrapper
mainContent={mainContent}
dialogs={visibleDialog}
/>;
};

render(renderTestContent({
refocusCounter: undefined,
modalState: ModalState.Open,
}));

// Increasing the refocusCounter for a background view while a dialog is visible
// should not try to focus the background view.
render(renderTestContent({
refocusCounter: 1,
modalState: ModalState.Open,
}));
expect(setFocusMock).not.toHaveBeenCalled();

// Focus should not be set until done closing
render(renderTestContent({
refocusCounter: 1,
modalState: ModalState.Closing,
}));
expect(setFocusMock).not.toHaveBeenCalled();

// Keeping the same refocus counter, but dismissing the dialog should focus
// the test view.
render(renderTestContent({
refocusCounter: 1,
modalState: ModalState.Closed,
}));
expect(setFocusMock).toHaveBeenCalled();
});
});
Loading
Loading