From b4622fe7331a4833174d8e09e4fe7dfbfcdf9ab1 Mon Sep 17 00:00:00 2001 From: Aldo Canepa Garay <87341702+aldo-expensify@users.noreply.github.com> Date: Fri, 2 Feb 2024 14:57:23 -0800 Subject: [PATCH] Revert "feat: implement useArrowKeyFocusManager in EmojiPickerMenu" --- .../EmojiPicker/EmojiPickerMenu/index.js | 214 ++++++++++++------ .../EmojiPickerMenu/index.native.js | 4 +- .../EmojiPickerMenu/useEmojiPickerMenu.js | 2 - src/hooks/useArrowKeyFocusManager.ts | 112 +-------- src/libs/EmojiUtils.ts | 13 -- 5 files changed, 156 insertions(+), 189 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPickerMenu/index.js b/src/components/EmojiPicker/EmojiPickerMenu/index.js index cbf55b31c74d..a723eed446a4 100755 --- a/src/components/EmojiPicker/EmojiPickerMenu/index.js +++ b/src/components/EmojiPicker/EmojiPickerMenu/index.js @@ -7,7 +7,6 @@ import _ from 'underscore'; import EmojiPickerMenuItem from '@components/EmojiPicker/EmojiPickerMenuItem'; import Text from '@components/Text'; import TextInput from '@components/TextInput'; -import useArrowKeyFocusManager from '@hooks/useArrowKeyFocusManager'; import useLocalize from '@hooks/useLocalize'; import useSingleExecution from '@hooks/useSingleExecution'; import useStyleUtils from '@hooks/useStyleUtils'; @@ -53,7 +52,6 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected}) { preferredSkinTone, listStyle, emojiListRef, - spacersIndexes, } = useEmojiPickerMenu(); // Ref for the emoji search input @@ -63,11 +61,22 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected}) { // prevent auto focus when open picker for mobile device const shouldFocusInputOnScreenFocus = canFocusInputOnScreenFocus(); + const [highlightedIndex, setHighlightedIndex] = useState(-1); const [arePointerEventsDisabled, setArePointerEventsDisabled] = useState(false); + const [selection, setSelection] = useState({start: 0, end: 0}); const [isFocused, setIsFocused] = useState(false); const [isUsingKeyboardMovement, setIsUsingKeyboardMovement] = useState(false); - const [highlightEmoji, setHighlightEmoji] = useState(false); const [highlightFirstEmoji, setHighlightFirstEmoji] = useState(false); + const firstNonHeaderIndex = useMemo(() => _.findIndex(filteredEmojis, (item) => !item.spacer && !item.header), [filteredEmojis]); + + /** + * On text input selection change + * + * @param {Event} event + */ + const onSelectionChange = useCallback((event) => { + setSelection(event.nativeEvent.selection); + }, []); const mouseMoveHandler = useCallback(() => { if (!arePointerEventsDisabled) { @@ -76,39 +85,15 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected}) { setArePointerEventsDisabled(false); }, [arePointerEventsDisabled]); - const onFocusedIndexChange = useCallback( - (newIndex) => { - if (filteredEmojis.length === 0) { - return; - } - - if (highlightFirstEmoji) { - setHighlightFirstEmoji(false); - } - - if (!isUsingKeyboardMovement) { - setIsUsingKeyboardMovement(true); - } - - // If the input is not focused and the new index is out of range, focus the input - if (newIndex < 0 && !searchInputRef.current.isFocused()) { - searchInputRef.current.focus(); - } - }, - [filteredEmojis.length, highlightFirstEmoji, isUsingKeyboardMovement], - ); - - const disabledIndexes = useMemo(() => (isListFiltered ? [] : [...headerIndices, ...spacersIndexes]), [headerIndices, isListFiltered, spacersIndexes]); - - const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({ - maxIndex: filteredEmojis.length - 1, - // Spacers indexes need to be disabled so that the arrow keys don't focus them. All headers are hidden when list is filtered - disabledIndexes, - itemsPerRow: CONST.EMOJI_NUM_PER_ROW, - initialFocusedIndex: -1, - disableCyclicTraversal: true, - onFocusedIndexChange, - }); + /** + * Focuses the search Input and has the text selected + */ + function focusInputWithTextSelect() { + if (!searchInputRef.current) { + return; + } + searchInputRef.current.focus(); + } const filterEmojis = _.throttle((searchTerm) => { const [normalizedSearchTerm, newFilteredEmojiList] = suggestEmojis(searchTerm); @@ -120,17 +105,119 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected}) { // There are no headers when searching, so we need to re-make them sticky when there is no search term setFilteredEmojis(allEmojis); setHeaderIndices(headerRowIndices); - setFocusedIndex(-1); - setHighlightEmoji(false); + setHighlightedIndex(-1); + setHighlightFirstEmoji(false); return; } // Remove sticky header indices. There are no headers while searching and we don't want to make emojis sticky setFilteredEmojis(newFilteredEmojiList); setHeaderIndices([]); + setHighlightedIndex(0); setHighlightFirstEmoji(true); - setIsUsingKeyboardMovement(false); }, throttleTime); + /** + * Highlights emojis adjacent to the currently highlighted emoji depending on the arrowKey + * @param {String} arrowKey + */ + const highlightAdjacentEmoji = useCallback( + (arrowKey) => { + if (filteredEmojis.length === 0) { + return; + } + + // Arrow Down and Arrow Right enable arrow navigation when search is focused + if (searchInputRef.current && searchInputRef.current.isFocused()) { + if (arrowKey !== 'ArrowDown' && arrowKey !== 'ArrowRight') { + return; + } + + if (arrowKey === 'ArrowRight' && !(searchInputRef.current.value.length === selection.start && selection.start === selection.end)) { + return; + } + + // Blur the input, change the highlight type to keyboard, and disable pointer events + searchInputRef.current.blur(); + setArePointerEventsDisabled(true); + setIsUsingKeyboardMovement(true); + setHighlightFirstEmoji(false); + + // We only want to hightlight the Emoji if none was highlighted already + // If we already have a highlighted Emoji, lets just skip the first navigation + if (highlightedIndex !== -1) { + return; + } + } + + // If nothing is highlighted and an arrow key is pressed + // select the first emoji, apply keyboard movement styles, and disable pointer events + if (highlightedIndex === -1) { + setHighlightedIndex(firstNonHeaderIndex); + setArePointerEventsDisabled(true); + setIsUsingKeyboardMovement(true); + return; + } + + let newIndex = highlightedIndex; + const move = (steps, boundsCheck, onBoundReached = () => {}) => { + if (boundsCheck()) { + onBoundReached(); + return; + } + + // Move in the prescribed direction until we reach an element that isn't a header + const isHeader = (e) => e.header || e.spacer; + do { + newIndex += steps; + if (newIndex < 0) { + break; + } + } while (isHeader(filteredEmojis[newIndex])); + }; + + switch (arrowKey) { + case 'ArrowDown': + move(CONST.EMOJI_NUM_PER_ROW, () => highlightedIndex + CONST.EMOJI_NUM_PER_ROW > filteredEmojis.length - 1); + break; + case 'ArrowLeft': + move( + -1, + () => highlightedIndex - 1 < firstNonHeaderIndex, + () => { + // Reaching start of the list, arrow left set the focus to searchInput. + focusInputWithTextSelect(); + newIndex = -1; + }, + ); + break; + case 'ArrowRight': + move(1, () => highlightedIndex + 1 > filteredEmojis.length - 1); + break; + case 'ArrowUp': + move( + -CONST.EMOJI_NUM_PER_ROW, + () => highlightedIndex - CONST.EMOJI_NUM_PER_ROW < firstNonHeaderIndex, + () => { + // Reaching start of the list, arrow up set the focus to searchInput. + focusInputWithTextSelect(); + newIndex = -1; + }, + ); + break; + default: + break; + } + + // Actually highlight the new emoji, apply keyboard movement styles, and disable pointer events + if (newIndex !== highlightedIndex) { + setHighlightedIndex(newIndex); + setArePointerEventsDisabled(true); + setIsUsingKeyboardMovement(true); + } + }, + [filteredEmojis, firstNonHeaderIndex, highlightedIndex, selection.end, selection.start], + ); + const keyDownHandler = useCallback( (keyBoardEvent) => { if (keyBoardEvent.key.startsWith('Arrow')) { @@ -138,17 +225,14 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected}) { keyBoardEvent.preventDefault(); } + // Move the highlight when arrow keys are pressed + highlightAdjacentEmoji(keyBoardEvent.key); return; } // Select the currently highlighted emoji if enter is pressed - if (!isEnterWhileComposition(keyBoardEvent) && keyBoardEvent.key === CONST.KEYBOARD_SHORTCUTS.ENTER.shortcutKey) { - let indexToSelect = focusedIndex; - if (highlightFirstEmoji) { - indexToSelect = 0; - } - - const item = filteredEmojis[indexToSelect]; + if (!isEnterWhileComposition(keyBoardEvent) && keyBoardEvent.key === CONST.KEYBOARD_SHORTCUTS.ENTER.shortcutKey && highlightedIndex !== -1) { + const item = filteredEmojis[highlightedIndex]; if (!item) { return; } @@ -166,6 +250,7 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected}) { // interfering with the input behaviour. if (keyBoardEvent.key === 'Tab' || keyBoardEvent.key === 'Enter' || (keyBoardEvent.key === 'Shift' && searchInputRef.current && !searchInputRef.current.isFocused())) { setIsUsingKeyboardMovement(true); + return; } // We allow typing in the search box if any key is pressed apart from Arrow keys. @@ -173,7 +258,7 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected}) { searchInputRef.current.focus(); } }, - [filteredEmojis, focusedIndex, highlightFirstEmoji, isFocused, onEmojiSelected, preferredSkinTone], + [filteredEmojis, highlightAdjacentEmoji, highlightedIndex, isFocused, onEmojiSelected, preferredSkinTone], ); /** @@ -258,15 +343,13 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected}) { const emojiCode = types && types[preferredSkinTone] ? types[preferredSkinTone] : code; - const isEmojiFocused = index === focusedIndex && isUsingKeyboardMovement; - const shouldEmojiBeHighlighted = index === focusedIndex && highlightEmoji; - const shouldFirstEmojiBeHighlighted = index === 0 && highlightFirstEmoji; + const isEmojiFocused = index === highlightedIndex && isUsingKeyboardMovement; + const shouldEmojiBeHighlighted = index === highlightedIndex && highlightFirstEmoji; return ( onEmojiSelected(emoji, item))} onHoverIn={() => { - setHighlightEmoji(false); setHighlightFirstEmoji(false); if (!isUsingKeyboardMovement) { return; @@ -274,26 +357,18 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected}) { setIsUsingKeyboardMovement(false); }} emoji={emojiCode} - onFocus={() => setFocusedIndex(index)} + onFocus={() => setHighlightedIndex(index)} + onBlur={() => + // Only clear the highlighted index if the highlighted index is the same, + // meaning that the focus changed to an element that is not an emoji item. + setHighlightedIndex((prevState) => (prevState === index ? -1 : prevState)) + } isFocused={isEmojiFocused} - isHighlighted={shouldFirstEmojiBeHighlighted || shouldEmojiBeHighlighted} + isHighlighted={shouldEmojiBeHighlighted} /> ); }, - [ - preferredSkinTone, - focusedIndex, - isUsingKeyboardMovement, - highlightEmoji, - highlightFirstEmoji, - singleExecution, - styles, - isSmallScreenWidth, - windowWidth, - translate, - onEmojiSelected, - setFocusedIndex, - ], + [preferredSkinTone, highlightedIndex, isUsingKeyboardMovement, highlightFirstEmoji, singleExecution, translate, onEmojiSelected, isSmallScreenWidth, windowWidth, styles], ); return ( @@ -314,8 +389,9 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected}) { defaultValue="" ref={searchInputRef} autoFocus={shouldFocusInputOnScreenFocus} + onSelectionChange={onSelectionChange} onFocus={() => { - setFocusedIndex(-1); + setHighlightedIndex(-1); setIsFocused(true); setIsUsingKeyboardMovement(false); }} @@ -337,7 +413,7 @@ function EmojiPickerMenu({forwardedRef, onEmojiSelected}) { ref={emojiListRef} data={filteredEmojis} renderItem={renderItem} - extraData={[focusedIndex, preferredSkinTone]} + extraData={[highlightedIndex, preferredSkinTone]} stickyHeaderIndices={headerIndices} /> diff --git a/src/components/EmojiPicker/EmojiPickerMenu/index.native.js b/src/components/EmojiPicker/EmojiPickerMenu/index.native.js index 27a49d360906..1463ce736699 100644 --- a/src/components/EmojiPicker/EmojiPickerMenu/index.native.js +++ b/src/components/EmojiPicker/EmojiPickerMenu/index.native.js @@ -21,7 +21,6 @@ function EmojiPickerMenu({onEmojiSelected}) { const styles = useThemeStyles(); const {windowWidth, isSmallScreenWidth} = useWindowDimensions(); const {translate} = useLocalize(); - const {singleExecution} = useSingleExecution(); const { allEmojis, headerEmojis, @@ -36,6 +35,7 @@ function EmojiPickerMenu({onEmojiSelected}) { listStyle, emojiListRef, } = useEmojiPickerMenu(); + const {singleExecution} = useSingleExecution(); const StyleUtils = useStyleUtils(); /** @@ -73,7 +73,7 @@ function EmojiPickerMenu({onEmojiSelected}) { /** * Given an emoji item object, render a component based on its type. * Items with the code "SPACER" return nothing and are used to fill rows up to 8 - * so that the sticky headers function properly. + * so that the sticky headers function properly * * @param {Object} item * @returns {*} diff --git a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js index 7caab5e6fb55..2d895193ec68 100644 --- a/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js +++ b/src/components/EmojiPicker/EmojiPickerMenu/useEmojiPickerMenu.js @@ -16,7 +16,6 @@ const useEmojiPickerMenu = () => { const allEmojis = useMemo(() => EmojiUtils.mergeEmojisWithFrequentlyUsedEmojis(emojis), [frequentlyUsedEmojis]); const headerEmojis = useMemo(() => EmojiUtils.getHeaderEmojis(allEmojis), [allEmojis]); const headerRowIndices = useMemo(() => _.map(headerEmojis, (headerEmoji) => headerEmoji.index), [headerEmojis]); - const spacersIndexes = useMemo(() => EmojiUtils.getSpacersIndexes(allEmojis), [allEmojis]); const [filteredEmojis, setFilteredEmojis] = useState(allEmojis); const [headerIndices, setHeaderIndices] = useState(headerRowIndices); const isListFiltered = allEmojis.length !== filteredEmojis.length; @@ -62,7 +61,6 @@ const useEmojiPickerMenu = () => { preferredSkinTone, listStyle, emojiListRef, - spacersIndexes, }; }; diff --git a/src/hooks/useArrowKeyFocusManager.ts b/src/hooks/useArrowKeyFocusManager.ts index 352734c92e8d..ecd494dfd9ea 100644 --- a/src/hooks/useArrowKeyFocusManager.ts +++ b/src/hooks/useArrowKeyFocusManager.ts @@ -9,8 +9,6 @@ type Config = { disabledIndexes?: readonly number[]; shouldExcludeTextAreaNodes?: boolean; isActive?: boolean; - itemsPerRow?: number; - disableCyclicTraversal?: boolean; }; type UseArrowKeyFocusManager = [number, (index: number) => void]; @@ -26,8 +24,6 @@ type UseArrowKeyFocusManager = [number, (index: number) => void]; * @param [config.disabledIndexes] – An array of indexes to disable + skip over * @param [config.shouldExcludeTextAreaNodes] – Whether arrow keys should have any effect when a TextArea node is focused * @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 */ export default function useArrowKeyFocusManager({ maxIndex, @@ -39,10 +35,7 @@ export default function useArrowKeyFocusManager({ disabledIndexes = CONST.EMPTY_ARRAY, shouldExcludeTextAreaNodes = true, isActive, - itemsPerRow, - disableCyclicTraversal = false, }: Config): UseArrowKeyFocusManager { - const allowHorizontalArrowKeys = !!itemsPerRow; const [focusedIndex, setFocusedIndex] = useState(initialFocusedIndex); const arrowConfig = useMemo( () => ({ @@ -52,29 +45,19 @@ export default function useArrowKeyFocusManager({ [isActive, shouldExcludeTextAreaNodes], ); - // eslint-disable-next-line react-hooks/exhaustive-deps - useEffect(() => onFocusedIndexChange(focusedIndex), [focusedIndex]); + useEffect(() => onFocusedIndexChange(focusedIndex), [focusedIndex, onFocusedIndexChange]); const arrowUpCallback = useCallback(() => { if (maxIndex < 0) { return; } - 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 - 1 : maxIndex; let newFocusedIndex = currentFocusedIndex; while (disabledIndexes.includes(newFocusedIndex)) { - newFocusedIndex -= allowHorizontalArrowKeys ? itemsPerRow : 1; - if (newFocusedIndex < 0) { - break; - } + newFocusedIndex = newFocusedIndex > 0 ? newFocusedIndex - 1 : maxIndex; if (newFocusedIndex === currentFocusedIndex) { // all indexes are disabled return actualIndex; // no-op @@ -82,8 +65,7 @@ export default function useArrowKeyFocusManager({ } return newFocusedIndex; }); - }, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, itemsPerRow, maxIndex]); - + }, [disabledIndexes, maxIndex]); useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_UP, arrowUpCallback, arrowConfig); const arrowDownCallback = useCallback(() => { @@ -91,98 +73,22 @@ export default function useArrowKeyFocusManager({ return; } - const nextIndex = disableCyclicTraversal ? maxIndex : 0; - setFocusedIndex((actualIndex) => { - let currentFocusedIndex = -1; - - if (actualIndex === -1) { - currentFocusedIndex = 0; - } else if (allowHorizontalArrowKeys) { - currentFocusedIndex = actualIndex < maxIndex ? actualIndex + itemsPerRow : nextIndex; - } else { - currentFocusedIndex = actualIndex < maxIndex ? actualIndex + 1 : nextIndex; - } - - if (disableCyclicTraversal && currentFocusedIndex > maxIndex) { - return actualIndex; - } - - let newFocusedIndex = currentFocusedIndex; - while (disabledIndexes.includes(newFocusedIndex)) { - if (actualIndex < 0) { - newFocusedIndex += 1; - } else { - newFocusedIndex += allowHorizontalArrowKeys ? itemsPerRow : 1; - } - - if (newFocusedIndex < 0) { - break; - } - if (newFocusedIndex === currentFocusedIndex) { - // all indexes are disabled - return actualIndex; - } - } - return newFocusedIndex; - }); - }, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, itemsPerRow, maxIndex]); - - useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_DOWN, arrowDownCallback, arrowConfig); - - const arrowLeftCallback = useCallback(() => { - if (maxIndex < 0 || !allowHorizontalArrowKeys) { - return; - } - - const nextIndex = disableCyclicTraversal ? -1 : maxIndex; - - setFocusedIndex((actualIndex) => { - let currentFocusedIndex = -1; - currentFocusedIndex = actualIndex > 0 ? actualIndex - 1 : nextIndex; - - let newFocusedIndex = currentFocusedIndex; - - while (disabledIndexes.includes(newFocusedIndex)) { - newFocusedIndex = newFocusedIndex > 0 ? newFocusedIndex - 1 : nextIndex; - - if (newFocusedIndex === currentFocusedIndex) { - // all indexes are disabled - return actualIndex; // no-op - } - } - return newFocusedIndex; - }); - }, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, maxIndex]); - - useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_LEFT, arrowLeftCallback, arrowConfig); - - const arrowRightCallback = useCallback(() => { - if (maxIndex < 0 || !allowHorizontalArrowKeys) { - return; - } - - const nextIndex = disableCyclicTraversal ? maxIndex : 0; - - setFocusedIndex((actualIndex) => { - let currentFocusedIndex = -1; - currentFocusedIndex = actualIndex < maxIndex ? actualIndex + 1 : nextIndex; - + const currentFocusedIndex = actualIndex < maxIndex ? actualIndex + 1 : 0; let newFocusedIndex = currentFocusedIndex; while (disabledIndexes.includes(newFocusedIndex)) { - newFocusedIndex = newFocusedIndex < maxIndex ? newFocusedIndex + 1 : nextIndex; - + newFocusedIndex = newFocusedIndex < maxIndex ? newFocusedIndex + 1 : 0; if (newFocusedIndex === currentFocusedIndex) { // all indexes are disabled return actualIndex; } } + return newFocusedIndex; }); - }, [allowHorizontalArrowKeys, disableCyclicTraversal, disabledIndexes, maxIndex]); - - useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_RIGHT, arrowRightCallback, arrowConfig); + }, [disabledIndexes, maxIndex]); + useKeyboardShortcut(CONST.KEYBOARD_SHORTCUTS.ARROW_DOWN, arrowDownCallback, arrowConfig); // Note: you don't need to manually manage focusedIndex in the parent. setFocusedIndex is only exposed in case you want to reset focusedIndex or focus a specific item return [focusedIndex, setFocusedIndex]; diff --git a/src/libs/EmojiUtils.ts b/src/libs/EmojiUtils.ts index fe79ea68a0b3..e34fa0b90fc6 100644 --- a/src/libs/EmojiUtils.ts +++ b/src/libs/EmojiUtils.ts @@ -548,18 +548,6 @@ const getEmojiReactionDetails = (emojiName: string, reaction: ReportActionReacti }; }; -function getSpacersIndexes(allEmojis: EmojiPickerList): number[] { - const spacersIndexes: number[] = []; - allEmojis.forEach((emoji, index) => { - if (!(CONST.EMOJI_PICKER_ITEM_TYPES.SPACER in emoji)) { - return; - } - - spacersIndexes.push(index); - }); - return spacersIndexes; -} - export { findEmojiByName, findEmojiByCode, @@ -582,5 +570,4 @@ export { getAddedEmojis, isFirstLetterEmoji, hasAccountIDEmojiReacted, - getSpacersIndexes, };