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.

Sign up for GitHub

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 can't use right and left arrow key when a mention suggestion is shown #38383

Merged
merged 4 commits into from
Mar 25, 2024
Merged

Fix can't use right and left arrow key when a mention suggestion is shown #38383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fbbfb3e
rework the horizontal keys prop and set it as false by default
bernhardoj Mar 15, 2024
c5faa66
lint and cleanup
bernhardoj Mar 15, 2024
e1464ed
Merge branch 'main' into fix/38076-horizontal-arrow-key-doesnt-work
bernhardoj Mar 19, 2024
296fb5f
update jsdoc
bernhardoj Mar 19, 2024
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
2 changes: 1 addition & 1 deletion src/components/EmojiPicker/EmojiPickerMenu/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected, activeEmoji}) {
initialFocusedIndex: -1,
disableCyclicTraversal: true,
onFocusedIndexChange,
disableHorizontalKeys: isFocused,
allowHorizontalArrowKeys: !isFocused,
// We pass true without checking visibility of the component because if the popover is not visible this picker won't be mounted
isActive: true,
allowNegativeIndexes: true,
Expand Down
36 changes: 13 additions & 23 deletions src/hooks/useArrowKeyFocusManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type Config = {
isActive?: boolean;
itemsPerRow?: number;
disableCyclicTraversal?: boolean;
disableHorizontalKeys?: boolean;
allowHorizontalArrowKeys?: boolean;
allowNegativeIndexes?: boolean;
};

Expand All @@ -30,7 +30,7 @@ type UseArrowKeyFocusManager = [number, (index: number) => void];
* @param [config.isActive] – Whether the component is ready and should subscribe to KeyboardShortcut
* @param [config.itemsPerRow] – The number of items per row. If provided, the arrow keys will move focus horizontally as well as vertically
* @param [config.disableCyclicTraversal] – Whether to disable cyclic traversal of the list. If true, the arrow keys will have no effect when the first or last item is focused
* @param [config.disableHorizontalKeys] – Whether to disable the right/left keys
* @param [config.allowHorizontalArrowKeys] – Whether to enable the right/left keys
*/
export default function useArrowKeyFocusManager({
maxIndex,
Expand All @@ -44,10 +44,9 @@ export default function useArrowKeyFocusManager({
isActive,
itemsPerRow,
disableCyclicTraversal = false,
disableHorizontalKeys = false,
allowHorizontalArrowKeys = false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update the comment of @param tag as well.

allowNegativeIndexes = false,
}: Config): UseArrowKeyFocusManager {
const allowHorizontalArrowKeys = !!itemsPerRow;
const [focusedIndex, setFocusedIndex] = useState(initialFocusedIndex);
const arrowConfig = useMemo(
() => ({
Expand All @@ -60,9 +59,9 @@ export default function useArrowKeyFocusManager({
const horizontalArrowConfig = useMemo(
() => ({
excludedNodes: shouldExcludeTextAreaNodes ? ['TEXTAREA'] : [],
isActive: isActive && !disableHorizontalKeys,
isActive: isActive && allowHorizontalArrowKeys,
}),
[isActive, shouldExcludeTextAreaNodes, disableHorizontalKeys],
[isActive, shouldExcludeTextAreaNodes, allowHorizontalArrowKeys],
);

// eslint-disable-next-line react-hooks/exhaustive-deps
Expand All @@ -75,16 +74,11 @@ export default function useArrowKeyFocusManager({
const nextIndex = disableCyclicTraversal ? -1 : maxIndex;

setFocusedIndex((actualIndex) => {
let currentFocusedIndex = -1;
if (allowHorizontalArrowKeys) {
currentFocusedIndex = actualIndex > 0 ? actualIndex - itemsPerRow : nextIndex;
} else {
currentFocusedIndex = actualIndex > 0 ? actualIndex - 1 : nextIndex;
}
const currentFocusedIndex = actualIndex > 0 ? actualIndex - (itemsPerRow ?? 1) : nextIndex;
let newFocusedIndex = currentFocusedIndex;

while (disabledIndexes.includes(newFocusedIndex)) {
newFocusedIndex -= allowHorizontalArrowKeys ? itemsPerRow : 1;
newFocusedIndex -= itemsPerRow ?? 1;
if (newFocusedIndex < 0) {
if (disableCyclicTraversal) {
if (!allowNegativeIndexes) {
Expand All @@ -101,7 +95,7 @@ export default function useArrowKeyFocusManager({
}
return newFocusedIndex;
});
}, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, itemsPerRow, maxIndex, allowNegativeIndexes]);
}, [disableCyclicTraversal, disabledIndexes, itemsPerRow, maxIndex, allowNegativeIndexes]);

useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_UP, arrowUpCallback, arrowConfig);

Expand All @@ -117,10 +111,8 @@ export default function useArrowKeyFocusManager({

if (actualIndex === -1) {
currentFocusedIndex = 0;
} else if (allowHorizontalArrowKeys) {
currentFocusedIndex = actualIndex < maxIndex ? actualIndex + itemsPerRow : nextIndex;
} else {
currentFocusedIndex = actualIndex < maxIndex ? actualIndex + 1 : nextIndex;
currentFocusedIndex = actualIndex < maxIndex ? actualIndex + (itemsPerRow ?? 1) : nextIndex;
}

if (disableCyclicTraversal && currentFocusedIndex > maxIndex) {
Expand All @@ -132,7 +124,7 @@ export default function useArrowKeyFocusManager({
if (actualIndex < 0) {
newFocusedIndex += 1;
} else {
newFocusedIndex += allowHorizontalArrowKeys ? itemsPerRow : 1;
newFocusedIndex += itemsPerRow ?? 1;
}

if (newFocusedIndex > maxIndex) {
Expand All @@ -148,7 +140,7 @@ export default function useArrowKeyFocusManager({
}
return newFocusedIndex;
});
}, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, itemsPerRow, maxIndex]);
}, [disableCyclicTraversal, disabledIndexes, itemsPerRow, maxIndex]);

useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_DOWN, arrowDownCallback, arrowConfig);

Expand All @@ -160,8 +152,7 @@ export default function useArrowKeyFocusManager({
const nextIndex = disableCyclicTraversal ? -1 : maxIndex;

setFocusedIndex((actualIndex) => {
let currentFocusedIndex = -1;
currentFocusedIndex = actualIndex > 0 ? actualIndex - 1 : nextIndex;
const currentFocusedIndex = actualIndex > 0 ? actualIndex - 1 : nextIndex;

let newFocusedIndex = currentFocusedIndex;

Expand All @@ -187,8 +178,7 @@ export default function useArrowKeyFocusManager({
const nextIndex = disableCyclicTraversal ? maxIndex : 0;

setFocusedIndex((actualIndex) => {
let currentFocusedIndex = -1;
currentFocusedIndex = actualIndex < maxIndex ? actualIndex + 1 : nextIndex;
const currentFocusedIndex = actualIndex < maxIndex ? actualIndex + 1 : nextIndex;

let newFocusedIndex = currentFocusedIndex;

Expand Down
Loading