Merge pull request #18955 from 0xmiroslav/fix/18744-auto-suggester

fix stale focusedIndex when maxIndex changed in ArrowKeyFocusManager
Expensify · May 15, 2023 · 2266274 · 2266274
2 parents aec9916 + 02bce91
commit 2266274
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 48 deletions.
diff --git a/src/components/ArrowKeyFocusManager.js b/src/components/ArrowKeyFocusManager.js
@@ -33,55 +33,16 @@ class ArrowKeyFocusManager extends Component {
         const arrowUpConfig = CONST.KEYBOARD_SHORTCUTS.ARROW_UP;
         const arrowDownConfig = CONST.KEYBOARD_SHORTCUTS.ARROW_DOWN;
 
-        this.unsubscribeArrowUpKey = KeyboardShortcut.subscribe(
-            arrowUpConfig.shortcutKey,
-            () => {
-                if (this.props.maxIndex < 0) {
-                    return;
-                }
-
-                const currentFocusedIndex = this.props.focusedIndex > 0 ? this.props.focusedIndex - 1 : this.props.maxIndex;
-                let newFocusedIndex = currentFocusedIndex;
-
-                while (this.props.disabledIndexes.includes(newFocusedIndex)) {
-                    newFocusedIndex = newFocusedIndex > 0 ? newFocusedIndex - 1 : this.props.maxIndex;
-                    if (newFocusedIndex === currentFocusedIndex) {
-                        // all indexes are disabled
-                        return; // no-op
-                    }
-                }
-
-                this.props.onFocusedIndexChanged(newFocusedIndex);
-            },
-            arrowUpConfig.descriptionKey,
-            arrowUpConfig.modifiers,
-            true,
-            false,
-            0,
-            true,
-            [this.props.shouldExcludeTextAreaNodes && 'TEXTAREA'],
-        );
+        this.onArrowUpKey = this.onArrowUpKey.bind(this);
+        this.onArrowDownKey = this.onArrowDownKey.bind(this);
+
+        this.unsubscribeArrowUpKey = KeyboardShortcut.subscribe(arrowUpConfig.shortcutKey, this.onArrowUpKey, arrowUpConfig.descriptionKey, arrowUpConfig.modifiers, true, false, 0, true, [
+            this.props.shouldExcludeTextAreaNodes && 'TEXTAREA',
+        ]);
 
         this.unsubscribeArrowDownKey = KeyboardShortcut.subscribe(
             arrowDownConfig.shortcutKey,
-            () => {
-                if (this.props.maxIndex < 0) {
-                    return;
-                }
-
-                const currentFocusedIndex = this.props.focusedIndex < this.props.maxIndex ? this.props.focusedIndex + 1 : 0;
-                let newFocusedIndex = currentFocusedIndex;
-
-                while (this.props.disabledIndexes.includes(newFocusedIndex)) {
-                    newFocusedIndex = newFocusedIndex < this.props.maxIndex ? newFocusedIndex + 1 : 0;
-                    if (newFocusedIndex === currentFocusedIndex) {
-                        // all indexes are disabled
-                        return; // no-op
-                    }
-                }
-
-                this.props.onFocusedIndexChanged(newFocusedIndex);
-            },
+            this.onArrowDownKey,
             arrowDownConfig.descriptionKey,
             arrowDownConfig.modifiers,
             true,
@@ -92,6 +53,15 @@ class ArrowKeyFocusManager extends Component {
         );
     }
 
+    componentDidUpdate(prevProps) {
+        if (prevProps.maxIndex === this.props.maxIndex) {
+            return;
+        }
+        if (this.props.focusedIndex > this.props.maxIndex) {
+            this.onArrowDownKey();
+        }
+    }
+
     componentWillUnmount() {
         if (this.unsubscribeArrowUpKey) {
             this.unsubscribeArrowUpKey();
@@ -102,6 +72,44 @@ class ArrowKeyFocusManager extends Component {
         }
     }
 
+    onArrowUpKey() {
+        if (this.props.maxIndex < 0) {
+            return;
+        }
+
+        const currentFocusedIndex = this.props.focusedIndex > 0 ? this.props.focusedIndex - 1 : this.props.maxIndex;
+        let newFocusedIndex = currentFocusedIndex;
+
+        while (this.props.disabledIndexes.includes(newFocusedIndex)) {
+            newFocusedIndex = newFocusedIndex > 0 ? newFocusedIndex - 1 : this.props.maxIndex;
+            if (newFocusedIndex === currentFocusedIndex) {
+                // all indexes are disabled
+                return; // no-op
+            }
+        }
+
+        this.props.onFocusedIndexChanged(newFocusedIndex);
+    }
+
+    onArrowDownKey() {
+        if (this.props.maxIndex < 0) {
+            return;
+        }
+
+        const currentFocusedIndex = this.props.focusedIndex < this.props.maxIndex ? this.props.focusedIndex + 1 : 0;
+        let newFocusedIndex = currentFocusedIndex;
+
+        while (this.props.disabledIndexes.includes(newFocusedIndex)) {
+            newFocusedIndex = newFocusedIndex < this.props.maxIndex ? newFocusedIndex + 1 : 0;
+            if (newFocusedIndex === currentFocusedIndex) {
+                // all indexes are disabled
+                return; // no-op
+            }
+        }
+
+        this.props.onFocusedIndexChanged(newFocusedIndex);
+    }
+
     render() {
         return this.props.children;
     }

diff --git a/src/pages/home/report/ReportActionCompose.js b/src/pages/home/report/ReportActionCompose.js
@@ -152,8 +152,8 @@ const defaultProps = {
 const getMaxArrowIndex = (numRows, isAutoSuggestionPickerLarge) => {
     // EmojiRowCount is number of emoji suggestions. For small screen we can fit 3 items and for large we show up to 5 items
     const emojiRowCount = isAutoSuggestionPickerLarge
-        ? Math.max(numRows, CONST.AUTO_COMPLETE_SUGGESTER.MAX_AMOUNT_OF_ITEMS)
-        : Math.max(numRows, CONST.AUTO_COMPLETE_SUGGESTER.MIN_AMOUNT_OF_ITEMS);
+        ? Math.min(numRows, CONST.AUTO_COMPLETE_SUGGESTER.MAX_AMOUNT_OF_ITEMS)
+        : Math.min(numRows, CONST.AUTO_COMPLETE_SUGGESTER.MIN_AMOUNT_OF_ITEMS);
 
     // -1 because we start at 0
     return emojiRowCount - 1;