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

fix: prevent parent view scroll when press space bar and arrow #56435

Open
wants to merge 14 commits into
base: main
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
9 changes: 9 additions & 0 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const keyInputUpArrow = KeyCommand?.constants?.keyInputUpArrow ?? 'keyInputUpArr
const keyInputDownArrow = KeyCommand?.constants?.keyInputDownArrow ?? 'keyInputDownArrow';
const keyInputLeftArrow = KeyCommand?.constants?.keyInputLeftArrow ?? 'keyInputLeftArrow';
const keyInputRightArrow = KeyCommand?.constants?.keyInputRightArrow ?? 'keyInputRightArrow';
const keyInputSpace = ' ';

// describes if a shortcut key can cause navigation
const KEYBOARD_SHORTCUT_NAVIGATION_TYPE = 'NAVIGATION_SHORTCUT';
Expand Down Expand Up @@ -928,6 +929,14 @@ const CONST = {
shortcutKey: 'Backspace',
modifiers: [],
},
SPACE: {
descriptionKey: null,
shortcutKey: 'Space',
modifiers: [],
trigger: {
DEFAULT: {input: keyInputSpace},
},
},
},
KEYBOARD_SHORTCUTS_TYPES: {
NAVIGATION_SHORTCUT: KEYBOARD_SHORTCUT_NAVIGATION_TYPE,
Expand Down
31 changes: 24 additions & 7 deletions src/components/PopoverMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/* eslint-disable react/jsx-props-no-spreading */
import lodashIsEqual from 'lodash/isEqual';
import type {ReactNode, RefObject} from 'react';
import React, {useLayoutEffect, useState} from 'react';
import React, {useCallback, useLayoutEffect, useState} from 'react';
import {StyleSheet, View} from 'react-native';
import type {StyleProp, TextStyle, ViewStyle} from 'react-native';
import type {GestureResponderEvent, StyleProp, TextStyle, ViewStyle} from 'react-native';
import type {ModalProps} from 'react-native-modal';
import useArrowKeyFocusManager from '@hooks/useArrowKeyFocusManager';
import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
Expand All @@ -12,8 +12,9 @@ import useStyleUtils from '@hooks/useStyleUtils';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import useWindowDimensions from '@hooks/useWindowDimensions';
import * as Browser from '@libs/Browser';
import * as Modal from '@userActions/Modal';
import {isSafari} from '@libs/Browser';
import getPlatform from '@libs/getPlatform';
import {close as closeModal} from '@userActions/Modal';
import CONST from '@src/CONST';
import type {AnchorPosition} from '@src/styles';
import type {PendingAction} from '@src/types/onyx/OnyxCommon';
Expand Down Expand Up @@ -189,7 +190,8 @@ function PopoverMenu({
const currentMenuItemsFocusedIndex = getSelectedItemIndex(currentMenuItems);
const [enteredSubMenuIndexes, setEnteredSubMenuIndexes] = useState<readonly number[]>(CONST.EMPTY_ARRAY);
const {windowHeight} = useWindowDimensions();

const platform = getPlatform();
const isWebOrDesktop = platform === CONST.PLATFORM.WEB || platform === CONST.PLATFORM.DESKTOP;
const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({initialFocusedIndex: currentMenuItemsFocusedIndex, maxIndex: currentMenuItems.length - 1, isActive: isVisible});

const selectItem = (index: number) => {
Expand All @@ -202,9 +204,9 @@ function PopoverMenu({
setEnteredSubMenuIndexes([...enteredSubMenuIndexes, index]);
const selectedSubMenuItemIndex = selectedItem?.subMenuItems.findIndex((option) => option.isSelected);
setFocusedIndex(selectedSubMenuItemIndex);
} else if (selectedItem.shouldCallAfterModalHide && !Browser.isSafari()) {
} else if (selectedItem.shouldCallAfterModalHide && !isSafari()) {
onItemSelected?.(selectedItem, index);
Modal.close(
closeModal(
() => {
selectedItem.onSelected?.();
},
Expand Down Expand Up @@ -312,6 +314,21 @@ function PopoverMenu({
{isActive: isVisible},
);

const keyboardShortcutSpaceCallback = useCallback(
(e?: GestureResponderEvent | KeyboardEvent) => {
if (shouldUseScrollView) {
return;
}

e?.preventDefault();
},
[shouldUseScrollView],
);

// On web and desktop, pressing the space bar after interacting with the parent view
// can cause the parent view to scroll when the space bar is pressed.
useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.SPACE, keyboardShortcutSpaceCallback, {isActive: isWebOrDesktop && isVisible, shouldPreventDefault: false});

const onModalHide = () => {
setFocusedIndex(currentMenuItemsFocusedIndex);
};
Expand Down
3 changes: 3 additions & 0 deletions src/components/SelectionList/BaseSelectionList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import useKeyboardShortcut from '@hooks/useKeyboardShortcut';
import useKeyboardState from '@hooks/useKeyboardState';
import useLocalize from '@hooks/useLocalize';
import usePrevious from '@hooks/usePrevious';
import useScrollEnabled from '@hooks/useScrollEnabled';
import useSingleExecution from '@hooks/useSingleExecution';
import useStyledSafeAreaInsets from '@hooks/useStyledSafeAreaInsets';
import useThemeStyles from '@hooks/useThemeStyles';
Expand Down Expand Up @@ -137,6 +138,7 @@ function BaseSelectionList<TItem extends ListItem>(
const shouldShowSelectAll = !!onSelectAll;
const activeElementRole = useActiveElementRole();
const isFocused = useIsFocused();
const scrollEnabled = useScrollEnabled();
const [maxToRenderPerBatch, setMaxToRenderPerBatch] = useState(shouldUseDynamicMaxToRenderPerBatch ? 0 : CONST.MAX_TO_RENDER_PER_BATCH.DEFAULT);
const [isInitialSectionListRender, setIsInitialSectionListRender] = useState(true);
const {isKeyboardShown} = useKeyboardState();
Expand Down Expand Up @@ -876,6 +878,7 @@ function BaseSelectionList<TItem extends ListItem>(
listHeaderContent
)
}
scrollEnabled={scrollEnabled}
ListFooterComponent={listFooterContent ?? ShowMoreButtonInstance}
onEndReached={onEndReached}
onEndReachedThreshold={onEndReachedThreshold}
Expand Down
5 changes: 5 additions & 0 deletions src/hooks/useScrollEnabled/index.native.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import type UseScrollEnabled from './types';

const useScrollEnabled: UseScrollEnabled = () => undefined;

export default useScrollEnabled;
8 changes: 8 additions & 0 deletions src/hooks/useScrollEnabled/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import {useIsFocused} from '@react-navigation/native';
import type UseScrollEnabled from './types';

const useScrollEnabled: UseScrollEnabled = () => {
const isFocused = useIsFocused();
return isFocused;
};
export default useScrollEnabled;
3 changes: 3 additions & 0 deletions src/hooks/useScrollEnabled/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
type UseScrollEnabled = () => boolean | undefined;

export default UseScrollEnabled;
4 changes: 4 additions & 0 deletions src/libs/KeyboardShortcut/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const keyInputUpArrow = KeyCommand?.constants?.keyInputUpArrow?.toString() ?? 'k
const keyInputDownArrow = KeyCommand?.constants?.keyInputDownArrow?.toString() ?? 'keyInputDownArrow';
const keyInputLeftArrow = KeyCommand?.constants?.keyInputLeftArrow?.toString() ?? 'keyInputLeftArrow';
const keyInputRightArrow = KeyCommand?.constants?.keyInputRightArrow?.toString() ?? 'keyInputRightArrow';
const keyInputSpace = ' ';

/**
* Generates the normalized display name for keyboard shortcuts.
Expand All @@ -67,6 +68,9 @@ function getDisplayName(key: string, modifiers: string | string[]): string {
if (key.toLowerCase() === keyInputRightArrow.toLowerCase()) {
return ['ARROWRIGHT'];
}
if (key === keyInputSpace) {
return ['SPACE'];
}
return [key.toUpperCase()];
})();

Expand Down
4 changes: 3 additions & 1 deletion src/pages/settings/Profile/ProfilePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import Section from '@components/Section';
import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
import useLocalize from '@hooks/useLocalize';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useScrollEnabled from '@hooks/useScrollEnabled';
import useStyledSafeAreaInsets from '@hooks/useStyledSafeAreaInsets';
import useStyleUtils from '@hooks/useStyleUtils';
import useTheme from '@hooks/useTheme';
Expand All @@ -39,7 +40,7 @@ function ProfilePage() {
const {translate} = useLocalize();
const {shouldUseNarrowLayout} = useResponsiveLayout();
const {safeAreaPaddingBottomStyle} = useStyledSafeAreaInsets();

const scrollEnabled = useScrollEnabled();
const [loginList] = useOnyx(ONYXKEYS.LOGIN_LIST);
const [privatePersonalDetails] = useOnyx(ONYXKEYS.PRIVATE_PERSONAL_DETAILS);
const currentUserPersonalDetails = useCurrentUserPersonalDetails();
Expand Down Expand Up @@ -156,6 +157,7 @@ function ProfilePage() {
<ScrollView
style={styles.pt3}
contentContainerStyle={safeAreaPaddingBottomStyle}
scrollEnabled={scrollEnabled}
>
<MenuItemGroup>
<View style={[styles.flex1, shouldUseNarrowLayout ? styles.workspaceSectionMobile : styles.workspaceSection]}>
Expand Down
Loading