From 9de7b5f8ae823e357b7d61688b31addb2fe8c555 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 12 May 2023 16:28:06 -0700 Subject: [PATCH 01/12] Create useArrowKeyFocusManager hook --- src/hooks/useArrowKeyFocusManager.js | 53 ++++++++++++++++++++++++++++ src/hooks/useKeyboardShortcut.js | 25 +++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 src/hooks/useArrowKeyFocusManager.js create mode 100644 src/hooks/useKeyboardShortcut.js diff --git a/src/hooks/useArrowKeyFocusManager.js b/src/hooks/useArrowKeyFocusManager.js new file mode 100644 index 000000000000..f0921f3b206b --- /dev/null +++ b/src/hooks/useArrowKeyFocusManager.js @@ -0,0 +1,53 @@ +import useKeyboardShortcut from './useKeyboardShortcut'; +import CONST from '../CONST'; + +export default function useArrowKeyFocusManager({focusedIndex, maxIndex, onFocusedIndexChange, disabledIndexes = [], shouldExcludeTextAreaNodes = true}) { + useKeyboardShortcut( + CONST.KEYBOARD_SHORTCUTS.ARROW_UP, + () => { + if (maxIndex < 0) { + return; + } + + const currentFocusedIndex = focusedIndex > 0 ? focusedIndex - 1 : maxIndex; + let newFocusedIndex = currentFocusedIndex; + + while (disabledIndexes.includes(newFocusedIndex)) { + newFocusedIndex = newFocusedIndex > 0 ? newFocusedIndex - 1 : maxIndex; + if (newFocusedIndex === currentFocusedIndex) { + // all indexes are disabled + return; // no-op + } + } + + onFocusedIndexChange(newFocusedIndex); + }, + { + excludedNodes: shouldExcludeTextAreaNodes ? ['TEXTAREA'] : [], + }, + ); + useKeyboardShortcut( + CONST.KEYBOARD_SHORTCUTS.ARROW_DOWN, + () => { + if (maxIndex < 0) { + return; + } + + const currentFocusedIndex = focusedIndex < maxIndex ? focusedIndex + 1 : 0; + let newFocusedIndex = currentFocusedIndex; + + while (disabledIndexes.includes(newFocusedIndex)) { + newFocusedIndex = newFocusedIndex < maxIndex ? newFocusedIndex + 1 : 0; + if (newFocusedIndex === currentFocusedIndex) { + // all indexes are disabled + return; // no-op + } + } + + onFocusedIndexChange(newFocusedIndex); + }, + { + excludedNodes: shouldExcludeTextAreaNodes ? ['TEXTAREA'] : [], + }, + ); +} diff --git a/src/hooks/useKeyboardShortcut.js b/src/hooks/useKeyboardShortcut.js new file mode 100644 index 000000000000..90a8bb952603 --- /dev/null +++ b/src/hooks/useKeyboardShortcut.js @@ -0,0 +1,25 @@ +import {useEffect, useRef} from 'react'; +import KeyboardShortcut from '../libs/KeyboardShortcut'; + +export default function useKeyboardShortcut(shortcut, callback, config = {}) { + const {captureOnInputs = true, shouldBubble = false, priority = 0, shouldPreventDefault = true, excludedNodes = []} = config; + const unsubscribe = useRef(null); + useEffect(() => { + if (unsubscribe.current) { + unsubscribe.current(); + } + unsubscribe.current = KeyboardShortcut.subscribe( + shortcut.shortcutKey, + callback, + shortcut.descriptionKey, + shortcut.modifiers, + captureOnInputs, + shouldBubble, + priority, + shouldPreventDefault, + excludedNodes, + // eslint-disable-next-line react-hooks/exhaustive-deps + ); + return unsubscribe.current; + }, [callback, captureOnInputs, excludedNodes, priority, shortcut.descriptionKey, shortcut.modifiers, shortcut.shortcutKey, shouldBubble, shouldPreventDefault]); +} From c35e14dcd9276d7be676efd59d66b9336f3723d2 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 12 May 2023 16:39:29 -0700 Subject: [PATCH 02/12] Make useArrowKeyFocusManager more powerful and easy to use --- src/hooks/useArrowKeyFocusManager.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/hooks/useArrowKeyFocusManager.js b/src/hooks/useArrowKeyFocusManager.js index f0921f3b206b..b22b51f33caa 100644 --- a/src/hooks/useArrowKeyFocusManager.js +++ b/src/hooks/useArrowKeyFocusManager.js @@ -1,7 +1,11 @@ +import {useState, useEffect} from 'react'; import useKeyboardShortcut from './useKeyboardShortcut'; import CONST from '../CONST'; -export default function useArrowKeyFocusManager({focusedIndex, maxIndex, onFocusedIndexChange, disabledIndexes = [], shouldExcludeTextAreaNodes = true}) { +export default function useArrowKeyFocusManager({maxIndex, onFocusedIndexChange, initialFocusedIndex = 0, disabledIndexes = [], shouldExcludeTextAreaNodes = true}) { + const [focusedIndex, setFocusedIndex] = useState(initialFocusedIndex); + useEffect(() => onFocusedIndexChange(focusedIndex), [focusedIndex, onFocusedIndexChange]); + useKeyboardShortcut( CONST.KEYBOARD_SHORTCUTS.ARROW_UP, () => { @@ -20,12 +24,13 @@ export default function useArrowKeyFocusManager({focusedIndex, maxIndex, onFocus } } - onFocusedIndexChange(newFocusedIndex); + setFocusedIndex(newFocusedIndex); }, { excludedNodes: shouldExcludeTextAreaNodes ? ['TEXTAREA'] : [], }, ); + useKeyboardShortcut( CONST.KEYBOARD_SHORTCUTS.ARROW_DOWN, () => { @@ -44,10 +49,12 @@ export default function useArrowKeyFocusManager({focusedIndex, maxIndex, onFocus } } - onFocusedIndexChange(newFocusedIndex); + setFocusedIndex(newFocusedIndex); }, { excludedNodes: shouldExcludeTextAreaNodes ? ['TEXTAREA'] : [], }, ); + + return focusedIndex; } From 887a89c5efc4af0501c50efc56731c45d0ef23fe Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 12 May 2023 17:56:58 -0700 Subject: [PATCH 03/12] Add isActive prop to useKeyboardShortcut --- src/hooks/useArrowKeyFocusManager.js | 4 +-- src/hooks/useKeyboardShortcut.js | 45 +++++++++++++++------------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/hooks/useArrowKeyFocusManager.js b/src/hooks/useArrowKeyFocusManager.js index b22b51f33caa..42759bc16bf3 100644 --- a/src/hooks/useArrowKeyFocusManager.js +++ b/src/hooks/useArrowKeyFocusManager.js @@ -2,7 +2,7 @@ import {useState, useEffect} from 'react'; import useKeyboardShortcut from './useKeyboardShortcut'; import CONST from '../CONST'; -export default function useArrowKeyFocusManager({maxIndex, onFocusedIndexChange, initialFocusedIndex = 0, disabledIndexes = [], shouldExcludeTextAreaNodes = true}) { +export default function useArrowKeyFocusManager({maxIndex, onFocusedIndexChange = () => {}, initialFocusedIndex = 0, disabledIndexes = [], shouldExcludeTextAreaNodes = true}) { const [focusedIndex, setFocusedIndex] = useState(initialFocusedIndex); useEffect(() => onFocusedIndexChange(focusedIndex), [focusedIndex, onFocusedIndexChange]); @@ -56,5 +56,5 @@ export default function useArrowKeyFocusManager({maxIndex, onFocusedIndexChange, }, ); - return focusedIndex; + return [focusedIndex, setFocusedIndex]; } diff --git a/src/hooks/useKeyboardShortcut.js b/src/hooks/useKeyboardShortcut.js index 90a8bb952603..2227824e4828 100644 --- a/src/hooks/useKeyboardShortcut.js +++ b/src/hooks/useKeyboardShortcut.js @@ -1,25 +1,30 @@ -import {useEffect, useRef} from 'react'; +import {useEffect, useRef, useCallback} from 'react'; import KeyboardShortcut from '../libs/KeyboardShortcut'; export default function useKeyboardShortcut(shortcut, callback, config = {}) { - const {captureOnInputs = true, shouldBubble = false, priority = 0, shouldPreventDefault = true, excludedNodes = []} = config; - const unsubscribe = useRef(null); + const {captureOnInputs = true, shouldBubble = false, priority = 0, shouldPreventDefault = true, excludedNodes = [], isActive = true} = config; + + const subscription = useRef(null); + const subscribe = useCallback( + () => + KeyboardShortcut.subscribe( + shortcut.shortcutKey, + callback, + shortcut.descriptionKey, + shortcut.modifiers, + captureOnInputs, + shouldBubble, + priority, + shouldPreventDefault, + excludedNodes, + ), + [callback, captureOnInputs, excludedNodes, priority, shortcut.descriptionKey, shortcut.modifiers, shortcut.shortcutKey, shouldBubble, shouldPreventDefault], + ); + useEffect(() => { - if (unsubscribe.current) { - unsubscribe.current(); - } - unsubscribe.current = KeyboardShortcut.subscribe( - shortcut.shortcutKey, - callback, - shortcut.descriptionKey, - shortcut.modifiers, - captureOnInputs, - shouldBubble, - priority, - shouldPreventDefault, - excludedNodes, - // eslint-disable-next-line react-hooks/exhaustive-deps - ); - return unsubscribe.current; - }, [callback, captureOnInputs, excludedNodes, priority, shortcut.descriptionKey, shortcut.modifiers, shortcut.shortcutKey, shouldBubble, shouldPreventDefault]); + const unsubscribe = subscription.current || (() => {}); + unsubscribe(); + subscription.current = isActive ? subscribe() : null; + return unsubscribe; + }, [isActive, subscribe]); } From 43b07b224e5b356b6b1f776f080cb913ce22a7d4 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 12 May 2023 18:06:47 -0700 Subject: [PATCH 04/12] Create useWindowDimensions hook --- src/hooks/useWindowDimensions.js | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 src/hooks/useWindowDimensions.js diff --git a/src/hooks/useWindowDimensions.js b/src/hooks/useWindowDimensions.js new file mode 100644 index 000000000000..ea46be38246a --- /dev/null +++ b/src/hooks/useWindowDimensions.js @@ -0,0 +1,20 @@ +import {useWindowDimensions} from 'react-native'; +import variables from '../styles/variables'; + +/** + * A convenience wrapper around React Native's useWindowDimensions hook that also provides booleans for our breakpoints. + * @returns {Object} + */ +export default function () { + const {width: windowWidth, height: windowHeight} = useWindowDimensions(); + const isSmallScreenWidth = windowWidth <= variables.mobileResponsiveWidthBreakpoint; + const isMediumScreenWidth = windowWidth > variables.mobileResponsiveWidthBreakpoint && windowWidth <= variables.tabletResponsiveWidthBreakpoint; + const isLargeScreenWidth = windowWidth > variables.tabletResponsiveWidthBreakpoint; + return { + windowWidth, + windowHeight, + isSmallScreenWidth, + isMediumScreenWidth, + isLargeScreenWidth, + }; +} From a64b767be4159dce549b87f68c8326564b2bb620 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 12 May 2023 18:54:22 -0700 Subject: [PATCH 05/12] Get popover displaying in the correct place --- src/components/ButtonWithDropdownMenu.js | 19 +- src/components/PopoverMenu/index.js | 194 ++++++++----------- src/components/PopoverWithMeasuredContent.js | 10 +- 3 files changed, 94 insertions(+), 129 deletions(-) diff --git a/src/components/ButtonWithDropdownMenu.js b/src/components/ButtonWithDropdownMenu.js index ff2198e63ca6..fc5a488a4ac4 100644 --- a/src/components/ButtonWithDropdownMenu.js +++ b/src/components/ButtonWithDropdownMenu.js @@ -8,6 +8,7 @@ import PopoverMenu from './PopoverMenu'; import Icon from './Icon'; import * as Expensicons from './Icon/Expensicons'; import themeColors from '../styles/themes/default'; +import CONST from '../CONST'; const propTypes = { /** Text to display for the menu header */ @@ -46,20 +47,19 @@ const ButtonWithDropdownMenu = (props) => { const [selectedItemIndex, setSelectedItemIndex] = useState(0); const [isMenuVisible, setIsMenuVisible] = useState(false); const [popoverAnchorPosition, setPopoverAnchorPosition] = useState(null); - const [popoverDimensions, setPopoverDimensions] = useState({}); - const {width, height} = useWindowDimensions(); + const {width: windowWidth, height: windowHeight} = useWindowDimensions(); const caretButton = useRef(null); useEffect(() => { - if (!caretButton.current || !popoverDimensions.nativeEvent) { + if (!caretButton.current) { return; } caretButton.current.measureInWindow((x, y, w) => { setPopoverAnchorPosition({ - left: x - popoverDimensions.nativeEvent.layout.width, - top: y + w + 10, + horizontal: x + w, + vertical: y - 16, }); }); - }, [width, height, popoverDimensions.nativeEvent]); + }, [windowWidth, windowHeight]); const selectedItem = props.options[selectedItemIndex]; return ( @@ -104,12 +104,16 @@ const ButtonWithDropdownMenu = (props) => { pressOnEnter /> )} - {props.options.length > 1 && ( + {props.options.length > 1 && !_.isEmpty(popoverAnchorPosition) && ( setIsMenuVisible(false)} onItemSelected={() => setIsMenuVisible(false)} anchorPosition={popoverAnchorPosition} + anchorOrigin={{ + horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.RIGHT, + vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM, + }} headerText={props.menuHeaderText} menuItems={_.map(props.options, (item, index) => ({ ...item, @@ -117,7 +121,6 @@ const ButtonWithDropdownMenu = (props) => { setSelectedItemIndex(index); }, }))} - onLayout={setPopoverDimensions} /> )} diff --git a/src/components/PopoverMenu/index.js b/src/components/PopoverMenu/index.js index fab9051f38b2..713361e28c87 100644 --- a/src/components/PopoverMenu/index.js +++ b/src/components/PopoverMenu/index.js @@ -1,136 +1,102 @@ import _ from 'underscore'; -import React, {PureComponent} from 'react'; +import React from 'react'; +import PropTypes from 'prop-types'; import {View} from 'react-native'; -import Popover from '../Popover'; +import PopoverWithMeasuredContent from '../PopoverWithMeasuredContent'; import styles from '../../styles/styles'; import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDimensions'; import MenuItem from '../MenuItem'; -import {propTypes as createMenuPropTypes, defaultProps} from './popoverMenuPropTypes'; -import ArrowKeyFocusManager from '../ArrowKeyFocusManager'; +import {propTypes as createMenuPropTypes, defaultProps as createMenuDefaultProps} from './popoverMenuPropTypes'; import Text from '../Text'; -import KeyboardShortcut from '../../libs/KeyboardShortcut'; import CONST from '../../CONST'; +import useArrowKeyFocusManager from '../../hooks/useArrowKeyFocusManager'; +import useKeyboardShortcut from '../../hooks/useKeyboardShortcut'; +import useWindowDimensions from '../../hooks/useWindowDimensions'; +// TODO: DRY props const propTypes = { ...createMenuPropTypes, ...windowDimensionsPropTypes, -}; - -class PopoverMenu extends PureComponent { - constructor(props) { - super(props); - this.state = { - focusedIndex: -1, - }; - this.resetFocusAndHideModal = this.resetFocusAndHideModal.bind(this); - this.removeKeyboardListener = this.removeKeyboardListener.bind(this); - this.attachKeyboardListener = this.attachKeyboardListener.bind(this); - this.selectedItem = null; - } - - componentDidUpdate(prevProps) { - if (this.props.isVisible === prevProps.isVisible) { - return; - } - if (this.props.isVisible) { - this.attachKeyboardListener(); - } else { - this.removeKeyboardListener(); - } - } + /** The horizontal and vertical anchors points for the popover */ + anchorPosition: PropTypes.shape({ + horizontal: PropTypes.number.isRequired, + vertical: PropTypes.number.isRequired, + }).isRequired, - componentWillUnmount() { - this.removeKeyboardListener(); - } + /** Where the popover should be positioned relative to the anchor points. */ + anchorOrigin: PropTypes.shape({ + horizontal: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL)), + vertical: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_VERTICAL)), + }), +}; - /** - * Set item to local variable to fire `onSelected` action after the menu popup closes - * @param {Object} item - */ - selectItem(item) { - this.selectedItem = item; - this.props.onItemSelected(item); - } +const defaultProps = { + ...createMenuDefaultProps, + anchorOrigin: { + horizontal: CONST.MODAL.ANCHOR_ORIGIN_HORIZONTAL.LEFT, + vertical: CONST.MODAL.ANCHOR_ORIGIN_VERTICAL.BOTTOM, + }, +}; - attachKeyboardListener() { - const shortcutConfig = CONST.KEYBOARD_SHORTCUTS.ENTER; - this.unsubscribeEnterKey = KeyboardShortcut.subscribe( - shortcutConfig.shortcutKey, - () => { - if (this.state.focusedIndex === -1) { - return; - } - this.selectItem(this.props.menuItems[this.state.focusedIndex]); - this.setState({focusedIndex: -1}); // Reset the focusedIndex on selecting any menu - }, - shortcutConfig.descriptionKey, - shortcutConfig.modifiers, - true, - ); - } +const PopoverMenu = (props) => { + const {isSmallScreenWidth} = useWindowDimensions(); - removeKeyboardListener() { - if (!this.unsubscribeEnterKey) { - return; - } - this.unsubscribeEnterKey(); - } + const selectItem = (index) => { + const selectedItem = props.menuItems[index]; + props.onItemSelected(selectedItem); + }; - resetFocusAndHideModal() { - this.setState({focusedIndex: -1}); // Reset the focusedIndex on modal hide - this.removeKeyboardListener(); - if (this.selectedItem) { - this.selectedItem.onSelected(); - this.selectedItem = null; - } - } + const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({initialFocusedIndex: -1, maxIndex: props.menuItems.length - 1}); + useKeyboardShortcut( + CONST.KEYBOARD_SHORTCUTS.ENTER, + () => { + if (focusedIndex === -1) { + return; + } + selectItem(focusedIndex); + setFocusedIndex(-1); // Reset the focusedIndex on selecting any menu + }, + {isActive: props.isVisible}, + ); - render() { - return ( - - - {!_.isEmpty(this.props.headerText) && ( - - {this.props.headerText} - - )} - this.setState({focusedIndex: index})} - > - {_.map(this.props.menuItems, (item, menuIndex) => ( - this.selectItem(item)} - focused={this.state.focusedIndex === menuIndex} - /> - ))} - - - - ); - } -} + return ( + setFocusedIndex(-1)} + animationIn={props.animationIn} + animationOut={props.animationOut} + animationInTiming={props.animationInTiming} + // TODO: rename to `shouldDisableAnimation` + disableAnimation={props.disableAnimation} + // TODO: fuck this prop + fromSidebarMediumScreen={props.fromSidebarMediumScreen} + > + + {!_.isEmpty(props.headerText) && {props.headerText}} + {_.map(props.menuItems, (item, menuIndex) => ( + selectItem(item)} + // TODO: rename to isFocused + focused={focusedIndex === menuIndex} + /> + ))} + + + ); +}; PopoverMenu.propTypes = propTypes; PopoverMenu.defaultProps = defaultProps; +PopoverMenu.displayName = 'PopoverMenu'; -export default withWindowDimensions(PopoverMenu); +export default React.memo(withWindowDimensions(PopoverMenu)); diff --git a/src/components/PopoverWithMeasuredContent.js b/src/components/PopoverWithMeasuredContent.js index 7452738c355c..6c5fe09d4d23 100644 --- a/src/components/PopoverWithMeasuredContent.js +++ b/src/components/PopoverWithMeasuredContent.js @@ -27,10 +27,6 @@ const propTypes = { vertical: PropTypes.oneOf(_.values(CONST.MODAL.ANCHOR_ORIGIN_VERTICAL)), }), - /** A function with content to measure. This component will use this.props.children by default, - but in the case the children are not displayed, the measurement will not work. */ - measureContent: PropTypes.func.isRequired, - /** Static dimensions for the popover. * Note: When passed, it will skip dimensions measuring of the popover, and provided dimensions will be used to calculate the anchor position. */ @@ -164,7 +160,7 @@ class PopoverWithMeasuredContent extends Component { const adjustedAnchorPosition = this.calculateAdjustedAnchorPosition(); const horizontalShift = computeHorizontalShift(adjustedAnchorPosition.left, this.popoverWidth, this.props.windowWidth); const verticalShift = computeVerticalShift(adjustedAnchorPosition.top, this.popoverHeight, this.props.windowHeight); - const shifedAnchorPosition = { + const shiftedAnchorPosition = { left: adjustedAnchorPosition.left + horizontalShift, top: adjustedAnchorPosition.top + verticalShift, }; @@ -172,9 +168,9 @@ class PopoverWithMeasuredContent extends Component { - {this.props.measureContent()} + {this.props.children} ) : ( /* From 25d53a758aab29caa617debe6ccd70f8b13ee445 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 12 May 2023 18:58:05 -0700 Subject: [PATCH 06/12] fix useKeyboardShortcut cleanup --- src/hooks/useKeyboardShortcut.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useKeyboardShortcut.js b/src/hooks/useKeyboardShortcut.js index 2227824e4828..3983a1d48cbc 100644 --- a/src/hooks/useKeyboardShortcut.js +++ b/src/hooks/useKeyboardShortcut.js @@ -25,6 +25,6 @@ export default function useKeyboardShortcut(shortcut, callback, config = {}) { const unsubscribe = subscription.current || (() => {}); unsubscribe(); subscription.current = isActive ? subscribe() : null; - return unsubscribe; + return isActive ? subscription.current : () => {}; }, [isActive, subscribe]); } From 659d88865f9a8aaa41ca07746a6b2ba5e11df61d Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 12 May 2023 19:03:25 -0700 Subject: [PATCH 07/12] Remove unnecessary measureContent prop from PopoverWithMeasuredContent --- src/components/EmojiPicker/EmojiPicker.js | 16 ------------- .../PopoverReportActionContextMenu.js | 23 ------------------- .../ReactionList/PopoverReactionList.js | 22 ------------------ 3 files changed, 61 deletions(-) diff --git a/src/components/EmojiPicker/EmojiPicker.js b/src/components/EmojiPicker/EmojiPicker.js index ae9578f68a4b..2bdaabc324ce 100644 --- a/src/components/EmojiPicker/EmojiPicker.js +++ b/src/components/EmojiPicker/EmojiPicker.js @@ -20,7 +20,6 @@ class EmojiPicker extends React.Component { this.measureEmojiPopoverAnchorPosition = this.measureEmojiPopoverAnchorPosition.bind(this); this.measureEmojiPopoverAnchorPositionAndUpdateState = this.measureEmojiPopoverAnchorPositionAndUpdateState.bind(this); this.focusEmojiSearchInput = this.focusEmojiSearchInput.bind(this); - this.measureContent = this.measureContent.bind(this); this.onModalHide = () => {}; this.onEmojiSelected = () => {}; @@ -131,20 +130,6 @@ class EmojiPicker extends React.Component { }); } - /** - * Used to calculate the EmojiPicker Dimensions - * - * @returns {JSX} - */ - measureContent() { - return ( - (this.emojiSearchInput = el)} - /> - ); - } - /** * Focus the search input in the emoji picker. */ @@ -177,7 +162,6 @@ class EmojiPicker extends React.Component { height: CONST.EMOJI_PICKER_SIZE.HEIGHT, }} anchorOrigin={this.state.emojiPopoverAnchorOrigin} - measureContent={this.measureContent} > - ); - } - /** * Run the callback and return a noop function to reset it * @param {Function} callback @@ -296,7 +274,6 @@ class PopoverReportActionContextMenu extends React.Component { animationIn="fadeIn" disableAnimation={false} animationOutTiming={1} - measureContent={this.measureContent} shouldSetModalVisibility={false} fullscreen > diff --git a/src/pages/home/report/ReactionList/PopoverReactionList.js b/src/pages/home/report/ReactionList/PopoverReactionList.js index 3642cce69e8f..8a413163056d 100644 --- a/src/pages/home/report/ReactionList/PopoverReactionList.js +++ b/src/pages/home/report/ReactionList/PopoverReactionList.js @@ -39,7 +39,6 @@ class PopoverReactionList extends React.Component { this.showReactionList = this.showReactionList.bind(this); this.hideReactionList = this.hideReactionList.bind(this); - this.measureContent = this.measureContent.bind(this); this.measureReactionListPosition = this.measureReactionListPosition.bind(this); this.getReactionListMeasuredLocation = this.getReactionListMeasuredLocation.bind(this); @@ -147,26 +146,6 @@ class PopoverReactionList extends React.Component { }); } - /** - * Used to calculate the PopoverReactionList Dimensions - * - * @returns {JSX} - */ - measureContent() { - return ( - - ); - } - render() { return ( <> @@ -177,7 +156,6 @@ class PopoverReactionList extends React.Component { animationIn="fadeIn" disableAnimation={false} animationOutTiming={1} - measureContent={this.measureContent} shouldSetModalVisibility={false} fullscreen > From 630a35873a2e6292bfc0ff43ad7aad0100121f38 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 12 May 2023 19:08:08 -0700 Subject: [PATCH 08/12] Fix comment indentation --- src/components/PopoverWithMeasuredContent.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/PopoverWithMeasuredContent.js b/src/components/PopoverWithMeasuredContent.js index 6c5fe09d4d23..c0f4ea3113b1 100644 --- a/src/components/PopoverWithMeasuredContent.js +++ b/src/components/PopoverWithMeasuredContent.js @@ -174,11 +174,11 @@ class PopoverWithMeasuredContent extends Component { ) : ( /* - This is an invisible view used to measure the size of the popover, - before it ever needs to be displayed. - We do this because we need to know its dimensions in order to correctly animate the popover, - but we can't measure its dimensions without first rendering it. - */ + This is an invisible view used to measure the size of the popover, + before it ever needs to be displayed. + We do this because we need to know its dimensions in order to correctly animate the popover, + but we can't measure its dimensions without first rendering it. + */ Date: Fri, 12 May 2023 19:15:21 -0700 Subject: [PATCH 09/12] Add JSdocs in useArrowKeyFocusManager --- src/hooks/useArrowKeyFocusManager.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/hooks/useArrowKeyFocusManager.js b/src/hooks/useArrowKeyFocusManager.js index 42759bc16bf3..09c3f03d488b 100644 --- a/src/hooks/useArrowKeyFocusManager.js +++ b/src/hooks/useArrowKeyFocusManager.js @@ -2,6 +2,17 @@ import {useState, useEffect} from 'react'; import useKeyboardShortcut from './useKeyboardShortcut'; import CONST from '../CONST'; +/** + * A hook that makes it easy to use the arrow keys to manage focus of items in a list + * + * @param {Object} config + * @param {Number} config.maxIndex – typically the number of items in your list + * @param {Function} [config.onFocusedIndexChange] – optional callback to execute when focusedIndex changes + * @param {Number} [config.initialFocusedIndex] – where to start in the list + * @param {Array} [config.disabledIndexes] – An array of indexes to disable + skip over + * @param {Boolean} [config.shouldExcludeTextAreaNodes] – Whether arrow keys should have any effect when a TextArea node is focused + * @returns {Array} + */ export default function useArrowKeyFocusManager({maxIndex, onFocusedIndexChange = () => {}, initialFocusedIndex = 0, disabledIndexes = [], shouldExcludeTextAreaNodes = true}) { const [focusedIndex, setFocusedIndex] = useState(initialFocusedIndex); useEffect(() => onFocusedIndexChange(focusedIndex), [focusedIndex, onFocusedIndexChange]); @@ -56,5 +67,6 @@ export default function useArrowKeyFocusManager({maxIndex, onFocusedIndexChange }, ); + // 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]; } From 83d4d950604b5d23c064a8e98db03f9f66dae635 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 12 May 2023 19:17:03 -0700 Subject: [PATCH 10/12] Add JSDocs for useKeyboardShortcut --- src/hooks/useKeyboardShortcut.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/hooks/useKeyboardShortcut.js b/src/hooks/useKeyboardShortcut.js index 3983a1d48cbc..434daeda1921 100644 --- a/src/hooks/useKeyboardShortcut.js +++ b/src/hooks/useKeyboardShortcut.js @@ -1,6 +1,13 @@ import {useEffect, useRef, useCallback} from 'react'; import KeyboardShortcut from '../libs/KeyboardShortcut'; +/** + * Register a keyboard shortcut handler. + * + * @param {Object} shortcut + * @param {Function} callback + * @param {Object} [config] + */ export default function useKeyboardShortcut(shortcut, callback, config = {}) { const {captureOnInputs = true, shouldBubble = false, priority = 0, shouldPreventDefault = true, excludedNodes = [], isActive = true} = config; From bbda68bd75a5790f33e20e54c7b9ee5d04f83420 Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 12 May 2023 19:37:28 -0700 Subject: [PATCH 11/12] Fix BasePaymentsPage --- src/components/AddPaymentMethodMenu.js | 12 +++++++++--- src/components/ReportActionItem/IOUPreview.js | 1 + src/libs/actions/IOU.js | 1 - .../ContextMenu/PopoverReportActionContextMenu.js | 1 + .../Payments/PaymentsPage/BasePaymentsPage.js | 11 +++++++---- 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/components/AddPaymentMethodMenu.js b/src/components/AddPaymentMethodMenu.js index 61927aefb868..3be364bca7b5 100644 --- a/src/components/AddPaymentMethodMenu.js +++ b/src/components/AddPaymentMethodMenu.js @@ -1,6 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import {withOnyx} from 'react-native-onyx'; +import _ from 'underscore'; import * as Expensicons from './Icon/Expensicons'; import withLocalize, {withLocalizePropTypes} from './withLocalize'; import compose from '../libs/compose'; @@ -12,11 +13,16 @@ import PopoverMenu from './PopoverMenu'; import paypalMeDataPropTypes from './paypalMeDataPropTypes'; const propTypes = { + /** Should the component be visible? */ isVisible: PropTypes.bool.isRequired, + + /** Callback to execute when the component closes. */ onClose: PropTypes.func.isRequired, + + /** Anchor position for the AddPaymentMenu. */ anchorPosition: PropTypes.shape({ - top: PropTypes.number, - left: PropTypes.number, + horizontal: PropTypes.number, + vertical: PropTypes.number, }), /** Account details for PayPal.Me */ @@ -43,7 +49,7 @@ const AddPaymentMethodMenu = (props) => ( isVisible={props.isVisible} onClose={props.onClose} anchorPosition={props.anchorPosition} - onItemSelected={() => props.onClose()} + onItemSelected={props.onClose} menuItems={[ { text: props.translate('common.bankAccount'), diff --git a/src/components/ReportActionItem/IOUPreview.js b/src/components/ReportActionItem/IOUPreview.js index 02c24aec77ff..2a196b654ec8 100644 --- a/src/components/ReportActionItem/IOUPreview.js +++ b/src/components/ReportActionItem/IOUPreview.js @@ -26,6 +26,7 @@ import {showContextMenuForReport} from '../ShowContextMenuContext'; import * as OptionsListUtils from '../../libs/OptionsListUtils'; import * as CurrencyUtils from '../../libs/CurrencyUtils'; import * as ReportUtils from '../../libs/ReportUtils'; +import Button from '../Button'; const propTypes = { /** The active IOUReport, used for Onyx subscription */ diff --git a/src/libs/actions/IOU.js b/src/libs/actions/IOU.js index 799ed3f3e40b..c17d752cea6c 100644 --- a/src/libs/actions/IOU.js +++ b/src/libs/actions/IOU.js @@ -1128,7 +1128,6 @@ function sendMoneyViaPaypal(report, amount, currency, comment, managerEmail, rec * @param {Object} chatReport * @param {Object} iouReport * @param {String} reimbursementBankAccountState - * @param {String} reimbursementBankAccountState */ function payMoneyRequest(paymentType, chatReport, iouReport, reimbursementBankAccountState) { if ( diff --git a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js index 612ddbf421ba..49b9e34876c2 100644 --- a/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js +++ b/src/pages/home/report/ContextMenu/PopoverReportActionContextMenu.js @@ -283,6 +283,7 @@ class PopoverReportActionContextMenu extends React.Component { reportID={this.state.reportID} reportAction={this.state.reportAction} draftMessage={this.state.reportActionDraftMessage} + selection={this.state.selection} isArchivedRoom={this.state.isArchivedRoom} isChronosReport={this.state.isChronosReport} anchor={this.contextMenuTargetNode} diff --git a/src/pages/settings/Payments/PaymentsPage/BasePaymentsPage.js b/src/pages/settings/Payments/PaymentsPage/BasePaymentsPage.js index 3f68ef4dee32..588b75bd906b 100644 --- a/src/pages/settings/Payments/PaymentsPage/BasePaymentsPage.js +++ b/src/pages/settings/Payments/PaymentsPage/BasePaymentsPage.js @@ -48,8 +48,9 @@ class BasePaymentsPage extends React.Component { title: '', }, selectedPaymentMethodType: null, + anchorPositionHorizontal: 0, + anchorPositionVertical: 0, anchorPositionTop: 0, - anchorPositionBottom: 0, anchorPositionRight: 0, addPaymentMethodButton: null, methodID: null, @@ -151,10 +152,12 @@ class BasePaymentsPage extends React.Component { setPositionAddPaymentMenu(position) { this.setState({ anchorPositionTop: position.top + position.height, - anchorPositionBottom: this.props.windowHeight - position.top, // We want the position to be 13px to the right of the left border anchorPositionRight: this.props.windowWidth - position.right + 13, + + anchorPositionHorizontal: position.x, + anchorPositionVertical: position.y, }); } @@ -416,8 +419,8 @@ class BasePaymentsPage extends React.Component { isVisible={this.state.shouldShowAddPaymentMenu} onClose={this.hideAddPaymentMenu} anchorPosition={{ - bottom: this.state.anchorPositionBottom, - right: this.state.anchorPositionRight - 10, + horizontal: this.state.anchorPositionHorizontal, + vertical: this.state.anchorPositionVertical - 10, }} onItemSelected={(method) => this.addPaymentMethodTypePressed(method)} /> From 911168b6ea78ec5608a4abd2c930e6de341ed41c Mon Sep 17 00:00:00 2001 From: rory Date: Fri, 12 May 2023 19:39:15 -0700 Subject: [PATCH 12/12] add TODO --- src/components/KYCWall/BaseKYCWall.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/KYCWall/BaseKYCWall.js b/src/components/KYCWall/BaseKYCWall.js index e1579fd38e73..781f9f019207 100644 --- a/src/components/KYCWall/BaseKYCWall.js +++ b/src/components/KYCWall/BaseKYCWall.js @@ -126,6 +126,7 @@ class KYCWall extends React.Component { this.setState({shouldShowAddPaymentMenu: false})} + // TODO: Fix this anchor position anchorPosition={{ top: this.state.anchorPositionTop, left: this.state.anchorPositionLeft,