From 4e85d60bf5d976e2a5445a3036f781f25254ca24 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 14:38:24 +0200 Subject: [PATCH 01/21] migrate native AttachmentPicker to functional component --- .../AttachmentPicker/index.native.js | 376 +++++++++--------- 1 file changed, 177 insertions(+), 199 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index b4b7d0b04c4e..15723b08578c 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -1,25 +1,22 @@ -/** - * The react native image/document pickers work for iOS/Android, but we want to wrap them both within AttachmentPicker - */ import _ from 'underscore'; -import React, {Component} from 'react'; -import {Alert, Linking, View} from 'react-native'; -import {launchImageLibrary} from 'react-native-image-picker'; +import React, {useEffect, useState, useRef} from 'react'; +import {View, Alert, Linking} from 'react-native'; import RNDocumentPicker from 'react-native-document-picker'; import RNFetchBlob from 'react-native-blob-util'; +import {launchImageLibrary} from 'react-native-image-picker'; import {propTypes as basePropTypes, defaultProps} from './attachmentPickerPropTypes'; -import styles from '../../styles/styles'; -import Popover from '../Popover'; -import MenuItem from '../MenuItem'; -import * as Expensicons from '../Icon/Expensicons'; import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDimensions'; import withLocalize, {withLocalizePropTypes} from '../withLocalize'; import compose from '../../libs/compose'; -import launchCamera from './launchCamera'; import CONST from '../../CONST'; import * as FileUtils from '../../libs/fileDownload/FileUtils'; -import ArrowKeyFocusManager from '../ArrowKeyFocusManager'; +import * as Expensicons from '../Icon/Expensicons'; +import launchCamera from './launchCamera'; import KeyboardShortcut from '../../libs/KeyboardShortcut'; +import Popover from '../Popover'; +import MenuItem from '../MenuItem'; +import styles from '../../styles/styles'; +import ArrowKeyFocusManager from '../ArrowKeyFocusManager'; const propTypes = { ...basePropTypes, @@ -88,133 +85,26 @@ function getDataForUpload(fileData) { }); } -/** - * This component renders a function as a child and - * returns a "show attachment picker" method that takes - * a callback. This is the ios/android implementation - * opening a modal with attachment options - */ -class AttachmentPicker extends Component { - constructor(...args) { - super(...args); - - this.state = { - isVisible: false, - focusedIndex: -1, - }; - - this.menuItemData = [ - { - icon: Expensicons.Camera, - textTranslationKey: 'attachmentPicker.takePhoto', - pickAttachment: () => this.showImagePicker(launchCamera), - }, - { - icon: Expensicons.Gallery, - textTranslationKey: 'attachmentPicker.chooseFromGallery', - pickAttachment: () => this.showImagePicker(launchImageLibrary), - }, - ]; - - // When selecting an image on a native device, it would be redundant to have a second option for choosing a document, - // so it is excluded in this case. - if (this.props.type !== CONST.ATTACHMENT_PICKER_TYPE.IMAGE) { - this.menuItemData.push({ - icon: Expensicons.Paperclip, - textTranslationKey: 'attachmentPicker.chooseDocument', - pickAttachment: () => this.showDocumentPicker(), - }); - } - - this.close = this.close.bind(this); - this.pickAttachment = this.pickAttachment.bind(this); - this.removeKeyboardListener = this.removeKeyboardListener.bind(this); - this.attachKeyboardListener = this.attachKeyboardListener.bind(this); - } - - componentDidUpdate(prevState) { - if (this.state.isVisible === prevState.isVisible) { - return; - } - - if (this.state.isVisible) { - this.attachKeyboardListener(); - } else { - this.removeKeyboardListener(); - } - } - - componentWillUnmount() { - this.removeKeyboardListener(); - } - - attachKeyboardListener() { - const shortcutConfig = CONST.KEYBOARD_SHORTCUTS.ENTER; - this.unsubscribeEnterKey = KeyboardShortcut.subscribe( - shortcutConfig.shortcutKey, - () => { - if (this.state.focusedIndex === -1) { - return; - } - this.selectItem(this.menuItemData[this.state.focusedIndex]); - this.setState({focusedIndex: -1}); // Reset the focusedIndex on selecting any menu - }, - shortcutConfig.descriptionKey, - shortcutConfig.modifiers, - true, - ); - } - - removeKeyboardListener() { - if (!this.unsubscribeEnterKey) { - return; - } - this.unsubscribeEnterKey(); - } - - /** - * Handles the image/document picker result and - * sends the selected attachment to the caller (parent component) - * - * @param {Array} attachments - * @returns {Promise} - */ - pickAttachment(attachments = []) { - if (attachments.length === 0) { - return; - } - - const fileData = _.first(attachments); - - if (fileData.width === -1 || fileData.height === -1) { - this.showImageCorruptionAlert(); - return; - } - - return getDataForUpload(fileData) - .then((result) => { - this.completeAttachmentSelection(result); - }) - .catch((error) => { - this.showGeneralAlert(error.message); - throw error; - }); - } +function AttachmentPicker(props) { + // const onPicked = useRef(); + const completeAttachmentSelection = useRef(); + const onModalHide = useRef(); + const keyboardListener = useRef(); /** * Inform the users when they need to grant camera access and guide them to settings */ - showPermissionsAlert() { + function showPermissionsAlert() { Alert.alert( - this.props.translate('attachmentPicker.cameraPermissionRequired'), - this.props.translate('attachmentPicker.expensifyDoesntHaveAccessToCamera'), + props.translate('attachmentPicker.cameraPermissionRequired'), + props.translate('attachmentPicker.expensifyDoesntHaveAccessToCamera'), [ { - text: this.props.translate('common.cancel'), + text: props.translate('common.cancel'), style: 'cancel', }, { - text: this.props.translate('common.settings'), + text: props.translate('common.settings'), onPress: () => Linking.openSettings(), }, ], @@ -222,15 +112,23 @@ class AttachmentPicker extends Component { ); } + /** + * A generic handling when we don't know the exact reason for an error + * + */ + function showGeneralAlert() { + Alert.alert(props.translate('attachmentPicker.attachmentError'), props.translate('attachmentPicker.errorWhileSelectingAttachment')); + } + /** * Common image picker handling * * @param {function} imagePickerFunc - RNImagePicker.launchCamera or RNImagePicker.launchImageLibrary * @returns {Promise} */ - showImagePicker(imagePickerFunc) { + function showImagePicker(imagePickerFunc) { return new Promise((resolve, reject) => { - imagePickerFunc(getImagePickerOptions(this.props.type), (response) => { + imagePickerFunc(getImagePickerOptions(props.type), (response) => { if (response.didCancel) { // When the user cancelled resolve with no attachment return resolve(); @@ -238,10 +136,10 @@ class AttachmentPicker extends Component { if (response.errorCode) { switch (response.errorCode) { case 'permission': - this.showPermissionsAlert(); + showPermissionsAlert(); return resolve(); default: - this.showGeneralAlert(); + showGeneralAlert(); break; } @@ -253,19 +151,28 @@ class AttachmentPicker extends Component { }); } - /** - * A generic handling when we don't know the exact reason for an error - * - */ - showGeneralAlert() { - Alert.alert(this.props.translate('attachmentPicker.attachmentError'), this.props.translate('attachmentPicker.errorWhileSelectingAttachment')); - } + const [isVisible, setIsVisible] = useState(false); + const [focusedIndex, setFocusedIndex] = useState(-1); + const [menuItemData, setMenuItemData] = useState([ + { + icon: Expensicons.Camera, + textTranslationKey: 'attachmentPicker.takePhoto', + pickAttachment: () => showImagePicker(launchCamera), + }, + { + icon: Expensicons.Gallery, + textTranslationKey: 'attachmentPicker.chooseFromGallery', + pickAttachment: () => showImagePicker(launchImageLibrary), + }, + ]); + + // const [result, setResult] = useState() /** * An attachment error dialog when user selected malformed images */ - showImageCorruptionAlert() { - Alert.alert(this.props.translate('attachmentPicker.attachmentError'), this.props.translate('attachmentPicker.errorWhileSelectingCorruptedImage')); + function showImageCorruptionAlert() { + Alert.alert(props.translate('attachmentPicker.attachmentError'), props.translate('attachmentPicker.errorWhileSelectingCorruptedImage')); } /** @@ -273,43 +180,61 @@ class AttachmentPicker extends Component { * * @returns {Promise} */ - showDocumentPicker() { + function showDocumentPicker() { return RNDocumentPicker.pick(documentPickerOptions).catch((error) => { if (RNDocumentPicker.isCancel(error)) { return; } - this.showGeneralAlert(error.message); + showGeneralAlert(error.message); throw error; }); } /** - * Triggers the `onPicked` callback with the selected attachment + * Opens the attachment modal + * + * @param {function} onPickedHandler A callback that will be called with the selected attachment */ - completeAttachmentSelection() { - if (!this.state.result) { - return; - } - - this.state.onPicked(this.state.result); + function open(onPickedHandler) { + completeAttachmentSelection.current = onPickedHandler; + setIsVisible(true); } /** - * Opens the attachment modal - * - * @param {function} onPicked A callback that will be called with the selected attachment + * Closes the attachment modal */ - open(onPicked) { - this.completeAttachmentSelection = onPicked; - this.setState({isVisible: true}); + function close() { + setIsVisible(false); } /** - * Closes the attachment modal + * Handles the image/document picker result and + * sends the selected attachment to the caller (parent component) + * + * @param {Array} attachments + * @returns {Promise} */ - close() { - this.setState({isVisible: false}); + function pickAttachment(attachments = []) { + if (attachments.length === 0) { + return; + } + + const fileData = _.first(attachments); + + if (fileData.width === -1 || fileData.height === -1) { + showImageCorruptionAlert(); + return; + } + + return getDataForUpload(fileData) + .then((result) => { + completeAttachmentSelection.current(result); + }) + .catch((error) => { + showGeneralAlert(error.message); + throw error; + }); } /** @@ -318,65 +243,118 @@ class AttachmentPicker extends Component { * @param {Object} item - an item from this.menuItemData * @param {Function} item.pickAttachment */ - selectItem(item) { + function selectItem(item) { /* setTimeout delays execution to the frame after the modal closes * without this on iOS closing the modal closes the gallery/camera as well */ - this.onModalHide = () => + onModalHide.current = () => setTimeout( () => item .pickAttachment() - .then(this.pickAttachment) + .then(pickAttachment) .catch(console.error) - .finally(() => delete this.onModalHide), + .finally(() => delete onModalHide.current), 200, ); - this.close(); + close(); + } + + function attachKeyboardListener() { + const shortcutConfig = CONST.KEYBOARD_SHORTCUTS.ENTER; + keyboardListener.current = KeyboardShortcut.subscribe( + shortcutConfig.shortcutKey, + () => { + if (focusedIndex === -1) { + return; + } + selectItem(menuItemData[focusedIndex]); + setFocusedIndex(-1); // Reset the focusedIndex on selecting any menu + }, + shortcutConfig.descriptionKey, + shortcutConfig.modifiers, + true, + ); } + function removeKeyboardListener() { + if (!keyboardListener.current) { + return; + } + keyboardListener.current(); + } + + useEffect(() => { + // When selecting an image on a native device, it would be redundant to have a second option for choosing a document, + // so it is excluded in this case. + if (props.type === CONST.ATTACHMENT_PICKER_TYPE.IMAGE) { + return; + } + + setMenuItemData((oldMenuItemData) => [ + ...oldMenuItemData, + { + icon: Expensicons.Paperclip, + textTranslationKey: 'attachmentPicker.chooseDocument', + pickAttachment: () => showDocumentPicker(), + }, + ]); + + return () => { + removeKeyboardListener(); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + useEffect(() => { + if (isVisible) { + attachKeyboardListener(); + } else { + removeKeyboardListener(); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [isVisible]); + /** * Call the `children` renderProp with the interface defined in propTypes * * @returns {React.ReactNode} */ - renderChildren() { - return this.props.children({ - openPicker: ({onPicked}) => this.open(onPicked), + function renderChildren() { + return props.children({ + openPicker: ({onPicked}) => open(onPicked), }); } - render() { - return ( - <> - - - this.setState({focusedIndex: index})} - > - {_.map(this.menuItemData, (item, menuIndex) => ( - this.selectItem(item)} - focused={this.state.focusedIndex === menuIndex} - /> - ))} - - - - {this.renderChildren()} - - ); - } + return ( + <> + close()} + isVisible={isVisible} + anchorPosition={styles.createMenuPosition} + onModalHide={onModalHide.current} + > + + setFocusedIndex(index)} + > + {_.map(menuItemData, (item, menuIndex) => ( + selectItem(item)} + focused={focusedIndex === menuIndex} + /> + ))} + + + + {renderChildren()} + + ); } AttachmentPicker.propTypes = propTypes; From acfd749f79da4d2cce8217d5d9f6f060e14712af Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 14:46:33 +0200 Subject: [PATCH 02/21] remove unnecessary comment --- src/components/AttachmentPicker/index.native.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 15723b08578c..56c767da6c72 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -86,7 +86,6 @@ function getDataForUpload(fileData) { } function AttachmentPicker(props) { - // const onPicked = useRef(); const completeAttachmentSelection = useRef(); const onModalHide = useRef(); const keyboardListener = useRef(); From 1cd666a10f65d9778509a1d4a09d577b511caa1b Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:09:54 +0200 Subject: [PATCH 03/21] destructure props --- .../AttachmentPicker/index.native.js | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 56c767da6c72..80eebc9d857d 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -85,7 +85,7 @@ function getDataForUpload(fileData) { }); } -function AttachmentPicker(props) { +function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { const completeAttachmentSelection = useRef(); const onModalHide = useRef(); const keyboardListener = useRef(); @@ -95,15 +95,15 @@ function AttachmentPicker(props) { */ function showPermissionsAlert() { Alert.alert( - props.translate('attachmentPicker.cameraPermissionRequired'), - props.translate('attachmentPicker.expensifyDoesntHaveAccessToCamera'), + translate('attachmentPicker.cameraPermissionRequired'), + translate('attachmentPicker.expensifyDoesntHaveAccessToCamera'), [ { - text: props.translate('common.cancel'), + text: translate('common.cancel'), style: 'cancel', }, { - text: props.translate('common.settings'), + text: translate('common.settings'), onPress: () => Linking.openSettings(), }, ], @@ -116,7 +116,7 @@ function AttachmentPicker(props) { * */ function showGeneralAlert() { - Alert.alert(props.translate('attachmentPicker.attachmentError'), props.translate('attachmentPicker.errorWhileSelectingAttachment')); + Alert.alert(translate('attachmentPicker.attachmentError'), translate('attachmentPicker.errorWhileSelectingAttachment')); } /** @@ -127,7 +127,7 @@ function AttachmentPicker(props) { */ function showImagePicker(imagePickerFunc) { return new Promise((resolve, reject) => { - imagePickerFunc(getImagePickerOptions(props.type), (response) => { + imagePickerFunc(getImagePickerOptions(type), (response) => { if (response.didCancel) { // When the user cancelled resolve with no attachment return resolve(); @@ -171,7 +171,7 @@ function AttachmentPicker(props) { * An attachment error dialog when user selected malformed images */ function showImageCorruptionAlert() { - Alert.alert(props.translate('attachmentPicker.attachmentError'), props.translate('attachmentPicker.errorWhileSelectingCorruptedImage')); + Alert.alert(translate('attachmentPicker.attachmentError'), translate('attachmentPicker.errorWhileSelectingCorruptedImage')); } /** @@ -286,7 +286,7 @@ function AttachmentPicker(props) { useEffect(() => { // When selecting an image on a native device, it would be redundant to have a second option for choosing a document, // so it is excluded in this case. - if (props.type === CONST.ATTACHMENT_PICKER_TYPE.IMAGE) { + if (type === CONST.ATTACHMENT_PICKER_TYPE.IMAGE) { return; } @@ -320,7 +320,7 @@ function AttachmentPicker(props) { * @returns {React.ReactNode} */ function renderChildren() { - return props.children({ + return children({ openPicker: ({onPicked}) => open(onPicked), }); } @@ -333,7 +333,7 @@ function AttachmentPicker(props) { anchorPosition={styles.createMenuPosition} onModalHide={onModalHide.current} > - + selectItem(item)} focused={focusedIndex === menuIndex} /> From 2ad8277dcfcf2d6b96942ff8e6252571c021419c Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:12:22 +0200 Subject: [PATCH 04/21] refactor function declarations --- .../AttachmentPicker/index.native.js | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 80eebc9d857d..6303777f79b7 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -40,7 +40,7 @@ const imagePickerOptions = { * @param {String} type * @returns {Object} */ -function getImagePickerOptions(type) { +const getImagePickerOptions = (type) => { // mediaType property is one of the ImagePicker configuration to restrict types' const mediaType = type === CONST.ATTACHMENT_PICKER_TYPE.IMAGE ? 'photo' : 'mixed'; return { @@ -64,7 +64,7 @@ const documentPickerOptions = { * @param {Object} fileData * @return {Promise} */ -function getDataForUpload(fileData) { +const getDataForUpload = (fileData) => { const fileName = fileData.fileName || fileData.name || 'chat_attachment'; const fileResult = { name: FileUtils.cleanFileName(fileName), @@ -93,7 +93,7 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { /** * Inform the users when they need to grant camera access and guide them to settings */ - function showPermissionsAlert() { + const showPermissionsAlert = () => { Alert.alert( translate('attachmentPicker.cameraPermissionRequired'), translate('attachmentPicker.expensifyDoesntHaveAccessToCamera'), @@ -115,7 +115,7 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { * A generic handling when we don't know the exact reason for an error * */ - function showGeneralAlert() { + const showGeneralAlert = () => { Alert.alert(translate('attachmentPicker.attachmentError'), translate('attachmentPicker.errorWhileSelectingAttachment')); } @@ -125,8 +125,7 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { * @param {function} imagePickerFunc - RNImagePicker.launchCamera or RNImagePicker.launchImageLibrary * @returns {Promise} */ - function showImagePicker(imagePickerFunc) { - return new Promise((resolve, reject) => { + const showImagePicker = (imagePickerFunc) => new Promise((resolve, reject) => { imagePickerFunc(getImagePickerOptions(type), (response) => { if (response.didCancel) { // When the user cancelled resolve with no attachment @@ -147,8 +146,7 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { return resolve(response.assets); }); - }); - } + }) const [isVisible, setIsVisible] = useState(false); const [focusedIndex, setFocusedIndex] = useState(-1); @@ -170,7 +168,7 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { /** * An attachment error dialog when user selected malformed images */ - function showImageCorruptionAlert() { + const showImageCorruptionAlert = () => { Alert.alert(translate('attachmentPicker.attachmentError'), translate('attachmentPicker.errorWhileSelectingCorruptedImage')); } @@ -179,23 +177,21 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { * * @returns {Promise} */ - function showDocumentPicker() { - return RNDocumentPicker.pick(documentPickerOptions).catch((error) => { + const showDocumentPicker = () => RNDocumentPicker.pick(documentPickerOptions).catch((error) => { if (RNDocumentPicker.isCancel(error)) { return; } showGeneralAlert(error.message); throw error; - }); - } + }) /** * Opens the attachment modal * * @param {function} onPickedHandler A callback that will be called with the selected attachment */ - function open(onPickedHandler) { + const open = (onPickedHandler) => { completeAttachmentSelection.current = onPickedHandler; setIsVisible(true); } @@ -203,7 +199,7 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { /** * Closes the attachment modal */ - function close() { + const close = () => { setIsVisible(false); } @@ -214,7 +210,7 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { * @param {Array} attachments * @returns {Promise} */ - function pickAttachment(attachments = []) { + const pickAttachment = (attachments = []) => { if (attachments.length === 0) { return; } @@ -242,7 +238,7 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { * @param {Object} item - an item from this.menuItemData * @param {Function} item.pickAttachment */ - function selectItem(item) { + const selectItem = (item) => { /* setTimeout delays execution to the frame after the modal closes * without this on iOS closing the modal closes the gallery/camera as well */ onModalHide.current = () => @@ -259,7 +255,7 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { close(); } - function attachKeyboardListener() { + const attachKeyboardListener = () => { const shortcutConfig = CONST.KEYBOARD_SHORTCUTS.ENTER; keyboardListener.current = KeyboardShortcut.subscribe( shortcutConfig.shortcutKey, @@ -276,7 +272,7 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { ); } - function removeKeyboardListener() { + const removeKeyboardListener = () => { if (!keyboardListener.current) { return; } @@ -319,11 +315,9 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { * * @returns {React.ReactNode} */ - function renderChildren() { - return children({ + const renderChildren = () => children({ openPicker: ({onPicked}) => open(onPicked), - }); - } + }) return ( <> From d90c027632485514cab1a24f33f7571de64de942 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:13:08 +0200 Subject: [PATCH 05/21] remove unnecessary newline --- src/components/AttachmentPicker/index.native.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 6303777f79b7..a7ca787c4e39 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -113,7 +113,6 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { /** * A generic handling when we don't know the exact reason for an error - * */ const showGeneralAlert = () => { Alert.alert(translate('attachmentPicker.attachmentError'), translate('attachmentPicker.errorWhileSelectingAttachment')); From 491fb1294d11651727e17c3abf765a58f2018e12 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:21:31 +0200 Subject: [PATCH 06/21] simplify style conditional expression --- src/components/AttachmentPicker/index.native.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index a7ca787c4e39..151f1bec9868 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -326,7 +326,7 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { anchorPosition={styles.createMenuPosition} onModalHide={onModalHide.current} > - + Date: Thu, 10 Aug 2023 15:26:47 +0200 Subject: [PATCH 07/21] replace withLocalize with useLocalize hook --- src/components/AttachmentPicker/index.native.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 151f1bec9868..1f5c1db1dbd6 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -6,8 +6,6 @@ import RNFetchBlob from 'react-native-blob-util'; import {launchImageLibrary} from 'react-native-image-picker'; import {propTypes as basePropTypes, defaultProps} from './attachmentPickerPropTypes'; import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDimensions'; -import withLocalize, {withLocalizePropTypes} from '../withLocalize'; -import compose from '../../libs/compose'; import CONST from '../../CONST'; import * as FileUtils from '../../libs/fileDownload/FileUtils'; import * as Expensicons from '../Icon/Expensicons'; @@ -17,11 +15,11 @@ import Popover from '../Popover'; import MenuItem from '../MenuItem'; import styles from '../../styles/styles'; import ArrowKeyFocusManager from '../ArrowKeyFocusManager'; +import useLocalize from '../../hooks/useLocalize'; const propTypes = { ...basePropTypes, ...windowDimensionsPropTypes, - ...withLocalizePropTypes, }; /** @@ -85,10 +83,12 @@ const getDataForUpload = (fileData) => { }); } -function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { +function AttachmentPicker({ type, children, isSmallScreenWidth}) { const completeAttachmentSelection = useRef(); const onModalHide = useRef(); const keyboardListener = useRef(); + + const {translate} = useLocalize() /** * Inform the users when they need to grant camera access and guide them to settings @@ -352,4 +352,4 @@ function AttachmentPicker({translate, type, children, isSmallScreenWidth}) { AttachmentPicker.propTypes = propTypes; AttachmentPicker.defaultProps = defaultProps; -export default compose(withWindowDimensions, withLocalize)(AttachmentPicker); +export default withWindowDimensions(AttachmentPicker); From 05aad64c47f10fe76e9d9f211cbeb43068137a7a Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:27:42 +0200 Subject: [PATCH 08/21] simplify passing function as a prop --- src/components/AttachmentPicker/index.native.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 1f5c1db1dbd6..19c4ac36228f 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -321,7 +321,7 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { return ( <> close()} + onClose={close} isVisible={isVisible} anchorPosition={styles.createMenuPosition} onModalHide={onModalHide.current} From 29010871f010ee7e5611b74af2bdaa9dd1945942 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:31:49 +0200 Subject: [PATCH 09/21] wrap necessary functions with useCallback --- .../AttachmentPicker/index.native.js | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 19c4ac36228f..97ff1eea419b 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -1,5 +1,5 @@ import _ from 'underscore'; -import React, {useEffect, useState, useRef} from 'react'; +import React, {useEffect, useState, useRef, useCallback} from 'react'; import {View, Alert, Linking} from 'react-native'; import RNDocumentPicker from 'react-native-document-picker'; import RNFetchBlob from 'react-native-blob-util'; @@ -114,9 +114,9 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { /** * A generic handling when we don't know the exact reason for an error */ - const showGeneralAlert = () => { + const showGeneralAlert = useCallback(() => { Alert.alert(translate('attachmentPicker.attachmentError'), translate('attachmentPicker.errorWhileSelectingAttachment')); - } + }, [translate]) /** * Common image picker handling @@ -167,9 +167,9 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { /** * An attachment error dialog when user selected malformed images */ - const showImageCorruptionAlert = () => { + const showImageCorruptionAlert = useCallback(() => { Alert.alert(translate('attachmentPicker.attachmentError'), translate('attachmentPicker.errorWhileSelectingCorruptedImage')); - } + }, [translate]) /** * Launch the DocumentPicker. Results are in the same format as ImagePicker @@ -209,7 +209,7 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { * @param {Array} attachments * @returns {Promise} */ - const pickAttachment = (attachments = []) => { + const pickAttachment = useCallback((attachments = []) => { if (attachments.length === 0) { return; } @@ -229,7 +229,7 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { showGeneralAlert(error.message); throw error; }); - } + }, [showGeneralAlert, showImageCorruptionAlert]) /** * Setup native attachment selection to start after this popover closes @@ -237,7 +237,7 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { * @param {Object} item - an item from this.menuItemData * @param {Function} item.pickAttachment */ - const selectItem = (item) => { + const selectItem = useCallback((item) => { /* setTimeout delays execution to the frame after the modal closes * without this on iOS closing the modal closes the gallery/camera as well */ onModalHide.current = () => @@ -252,9 +252,9 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { ); close(); - } + }, [pickAttachment]) - const attachKeyboardListener = () => { + const attachKeyboardListener = useCallback(() => { const shortcutConfig = CONST.KEYBOARD_SHORTCUTS.ENTER; keyboardListener.current = KeyboardShortcut.subscribe( shortcutConfig.shortcutKey, @@ -269,7 +269,7 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { shortcutConfig.modifiers, true, ); - } + }, [focusedIndex, menuItemData, selectItem]) const removeKeyboardListener = () => { if (!keyboardListener.current) { @@ -306,8 +306,7 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { } else { removeKeyboardListener(); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isVisible]); + }, [isVisible, attachKeyboardListener]); /** * Call the `children` renderProp with the interface defined in propTypes From e2af866f5720131c18db1adbd5368f03fe5031b3 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:34:01 +0200 Subject: [PATCH 10/21] simplify showDocumentPicker function call --- src/components/AttachmentPicker/index.native.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 97ff1eea419b..96ee4a5b4527 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -290,7 +290,7 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { { icon: Expensicons.Paperclip, textTranslationKey: 'attachmentPicker.chooseDocument', - pickAttachment: () => showDocumentPicker(), + pickAttachment: showDocumentPicker, }, ]); From bf143c9f8c16d95214833d4605baddba02453c0e Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:35:11 +0200 Subject: [PATCH 11/21] add displayName --- src/components/AttachmentPicker/index.native.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 96ee4a5b4527..a903f125c43b 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -350,5 +350,6 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { AttachmentPicker.propTypes = propTypes; AttachmentPicker.defaultProps = defaultProps; +AttachmentPicker.displayName = 'AttachmentPicker'; export default withWindowDimensions(AttachmentPicker); From e1a30e4119f0f547431d853aa99f0de1878b41bc Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:35:35 +0200 Subject: [PATCH 12/21] remove unnecessary comment --- src/components/AttachmentPicker/index.native.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index a903f125c43b..ca0e9279c27a 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -162,8 +162,6 @@ function AttachmentPicker({ type, children, isSmallScreenWidth}) { }, ]); - // const [result, setResult] = useState() - /** * An attachment error dialog when user selected malformed images */ From fdd4db74e8c1df8cecdd39af28fb397cf28b9d6c Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:45:37 +0200 Subject: [PATCH 13/21] replace withWindowDimensions with useWindowDimensions hook --- src/components/AttachmentPicker/index.native.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index ca0e9279c27a..0c1dc1765bcb 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -5,7 +5,6 @@ import RNDocumentPicker from 'react-native-document-picker'; import RNFetchBlob from 'react-native-blob-util'; import {launchImageLibrary} from 'react-native-image-picker'; import {propTypes as basePropTypes, defaultProps} from './attachmentPickerPropTypes'; -import withWindowDimensions, {windowDimensionsPropTypes} from '../withWindowDimensions'; import CONST from '../../CONST'; import * as FileUtils from '../../libs/fileDownload/FileUtils'; import * as Expensicons from '../Icon/Expensicons'; @@ -16,10 +15,10 @@ import MenuItem from '../MenuItem'; import styles from '../../styles/styles'; import ArrowKeyFocusManager from '../ArrowKeyFocusManager'; import useLocalize from '../../hooks/useLocalize'; +import useWindowDimensions from '../../hooks/useWindowDimensions'; const propTypes = { ...basePropTypes, - ...windowDimensionsPropTypes, }; /** @@ -83,12 +82,13 @@ const getDataForUpload = (fileData) => { }); } -function AttachmentPicker({ type, children, isSmallScreenWidth}) { +function AttachmentPicker({ type, children}) { const completeAttachmentSelection = useRef(); const onModalHide = useRef(); const keyboardListener = useRef(); const {translate} = useLocalize() + const {isSmallScreenWidth} = useWindowDimensions() /** * Inform the users when they need to grant camera access and guide them to settings @@ -350,4 +350,4 @@ AttachmentPicker.propTypes = propTypes; AttachmentPicker.defaultProps = defaultProps; AttachmentPicker.displayName = 'AttachmentPicker'; -export default withWindowDimensions(AttachmentPicker); +export default AttachmentPicker; From 98ae4a5cabab86dbd11a7c968106696579345b83 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:47:40 +0200 Subject: [PATCH 14/21] move useState to the top --- src/components/AttachmentPicker/index.native.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 0c1dc1765bcb..bd39e83ea3eb 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -83,6 +83,9 @@ const getDataForUpload = (fileData) => { } function AttachmentPicker({ type, children}) { + const [isVisible, setIsVisible] = useState(false); + const [focusedIndex, setFocusedIndex] = useState(-1); + const completeAttachmentSelection = useRef(); const onModalHide = useRef(); const keyboardListener = useRef(); @@ -147,8 +150,6 @@ function AttachmentPicker({ type, children}) { }); }) - const [isVisible, setIsVisible] = useState(false); - const [focusedIndex, setFocusedIndex] = useState(-1); const [menuItemData, setMenuItemData] = useState([ { icon: Expensicons.Camera, From 845470ffc6277483595253e09eb660053f5e667f Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Thu, 10 Aug 2023 15:57:00 +0200 Subject: [PATCH 15/21] add explaination to eslint disable rule --- src/components/AttachmentPicker/index.native.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index bd39e83ea3eb..41317b78a7e3 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -296,7 +296,7 @@ function AttachmentPicker({ type, children}) { return () => { removeKeyboardListener(); }; - // eslint-disable-next-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-hooks/exhaustive-deps -- we only want this effect to run on initial render }, []); useEffect(() => { From cc36b2afd5beea64e29f2c0cd1317623eb93ddda Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Fri, 11 Aug 2023 08:40:17 +0200 Subject: [PATCH 16/21] convert menuItemData to useState --- .../AttachmentPicker/index.native.js | 178 ++++++++++-------- 1 file changed, 95 insertions(+), 83 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 41317b78a7e3..492c6d4e23c2 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -1,5 +1,5 @@ import _ from 'underscore'; -import React, {useEffect, useState, useRef, useCallback} from 'react'; +import React, {useEffect, useState, useRef, useCallback, useMemo} from 'react'; import {View, Alert, Linking} from 'react-native'; import RNDocumentPicker from 'react-native-document-picker'; import RNFetchBlob from 'react-native-blob-util'; @@ -44,7 +44,7 @@ const getImagePickerOptions = (type) => { mediaType, ...imagePickerOptions, }; -} +}; /** * See https://github.com/rnmods/react-native-document-picker#options for DocumentPicker configuration options @@ -80,23 +80,23 @@ const getDataForUpload = (fileData) => { fileResult.size = stats.size; return fileResult; }); -} +}; -function AttachmentPicker({ type, children}) { +function AttachmentPicker({type, children}) { const [isVisible, setIsVisible] = useState(false); const [focusedIndex, setFocusedIndex] = useState(-1); const completeAttachmentSelection = useRef(); const onModalHide = useRef(); const keyboardListener = useRef(); - - const {translate} = useLocalize() - const {isSmallScreenWidth} = useWindowDimensions() + + const {translate} = useLocalize(); + const {isSmallScreenWidth} = useWindowDimensions(); /** * Inform the users when they need to grant camera access and guide them to settings */ - const showPermissionsAlert = () => { + const showPermissionsAlert = useCallback(() => { Alert.alert( translate('attachmentPicker.cameraPermissionRequired'), translate('attachmentPicker.expensifyDoesntHaveAccessToCamera'), @@ -112,14 +112,14 @@ function AttachmentPicker({ type, children}) { ], {cancelable: false}, ); - } + }, [translate]); /** * A generic handling when we don't know the exact reason for an error */ const showGeneralAlert = useCallback(() => { Alert.alert(translate('attachmentPicker.attachmentError'), translate('attachmentPicker.errorWhileSelectingAttachment')); - }, [translate]) + }, [translate]); /** * Common image picker handling @@ -127,7 +127,8 @@ function AttachmentPicker({ type, children}) { * @param {function} imagePickerFunc - RNImagePicker.launchCamera or RNImagePicker.launchImageLibrary * @returns {Promise} */ - const showImagePicker = (imagePickerFunc) => new Promise((resolve, reject) => { + const showImagePicker = useCallback((imagePickerFunc) => + new Promise((resolve, reject) => { imagePickerFunc(getImagePickerOptions(type), (response) => { if (response.didCancel) { // When the user cancelled resolve with no attachment @@ -148,41 +149,54 @@ function AttachmentPicker({ type, children}) { return resolve(response.assets); }); - }) - - const [menuItemData, setMenuItemData] = useState([ - { - icon: Expensicons.Camera, - textTranslationKey: 'attachmentPicker.takePhoto', - pickAttachment: () => showImagePicker(launchCamera), - }, - { - icon: Expensicons.Gallery, - textTranslationKey: 'attachmentPicker.chooseFromGallery', - pickAttachment: () => showImagePicker(launchImageLibrary), - }, - ]); - - /** - * An attachment error dialog when user selected malformed images - */ - const showImageCorruptionAlert = useCallback(() => { - Alert.alert(translate('attachmentPicker.attachmentError'), translate('attachmentPicker.errorWhileSelectingCorruptedImage')); - }, [translate]) + }), [showGeneralAlert, showPermissionsAlert, type]); /** * Launch the DocumentPicker. Results are in the same format as ImagePicker * * @returns {Promise} */ - const showDocumentPicker = () => RNDocumentPicker.pick(documentPickerOptions).catch((error) => { + const showDocumentPicker = useCallback(() => + RNDocumentPicker.pick(documentPickerOptions).catch((error) => { if (RNDocumentPicker.isCancel(error)) { return; } showGeneralAlert(error.message); throw error; - }) + }), [showGeneralAlert]); + + const menuItemData = useMemo(() => { + const data = [ + { + icon: Expensicons.Camera, + textTranslationKey: 'attachmentPicker.takePhoto', + pickAttachment: () => showImagePicker(launchCamera), + }, + { + icon: Expensicons.Gallery, + textTranslationKey: 'attachmentPicker.chooseFromGallery', + pickAttachment: () => showImagePicker(launchImageLibrary), + }, + ]; + + if (type !== CONST.ATTACHMENT_PICKER_TYPE.IMAGE) { + data.push({ + icon: Expensicons.Paperclip, + textTranslationKey: 'attachmentPicker.chooseDocument', + pickAttachment: showDocumentPicker, + }); + } + + return data; + }, [showDocumentPicker, showImagePicker, type]); + + /** + * An attachment error dialog when user selected malformed images + */ + const showImageCorruptionAlert = useCallback(() => { + Alert.alert(translate('attachmentPicker.attachmentError'), translate('attachmentPicker.errorWhileSelectingCorruptedImage')); + }, [translate]); /** * Opens the attachment modal @@ -192,14 +206,14 @@ function AttachmentPicker({ type, children}) { const open = (onPickedHandler) => { completeAttachmentSelection.current = onPickedHandler; setIsVisible(true); - } + }; /** * Closes the attachment modal */ const close = () => { setIsVisible(false); - } + }; /** * Handles the image/document picker result and @@ -208,27 +222,30 @@ function AttachmentPicker({ type, children}) { * @param {Array} attachments * @returns {Promise} */ - const pickAttachment = useCallback((attachments = []) => { - if (attachments.length === 0) { - return; - } + const pickAttachment = useCallback( + (attachments = []) => { + if (attachments.length === 0) { + return; + } - const fileData = _.first(attachments); + const fileData = _.first(attachments); - if (fileData.width === -1 || fileData.height === -1) { - showImageCorruptionAlert(); - return; - } + if (fileData.width === -1 || fileData.height === -1) { + showImageCorruptionAlert(); + return; + } - return getDataForUpload(fileData) - .then((result) => { - completeAttachmentSelection.current(result); - }) - .catch((error) => { - showGeneralAlert(error.message); - throw error; - }); - }, [showGeneralAlert, showImageCorruptionAlert]) + return getDataForUpload(fileData) + .then((result) => { + completeAttachmentSelection.current(result); + }) + .catch((error) => { + showGeneralAlert(error.message); + throw error; + }); + }, + [showGeneralAlert, showImageCorruptionAlert], + ); /** * Setup native attachment selection to start after this popover closes @@ -236,22 +253,25 @@ function AttachmentPicker({ type, children}) { * @param {Object} item - an item from this.menuItemData * @param {Function} item.pickAttachment */ - const selectItem = useCallback((item) => { - /* setTimeout delays execution to the frame after the modal closes - * without this on iOS closing the modal closes the gallery/camera as well */ - onModalHide.current = () => - setTimeout( - () => - item - .pickAttachment() - .then(pickAttachment) - .catch(console.error) - .finally(() => delete onModalHide.current), - 200, - ); - - close(); - }, [pickAttachment]) + const selectItem = useCallback( + (item) => { + /* setTimeout delays execution to the frame after the modal closes + * without this on iOS closing the modal closes the gallery/camera as well */ + onModalHide.current = () => + setTimeout( + () => + item + .pickAttachment() + .then(pickAttachment) + .catch(console.error) + .finally(() => delete onModalHide.current), + 200, + ); + + close(); + }, + [pickAttachment], + ); const attachKeyboardListener = useCallback(() => { const shortcutConfig = CONST.KEYBOARD_SHORTCUTS.ENTER; @@ -268,14 +288,14 @@ function AttachmentPicker({ type, children}) { shortcutConfig.modifiers, true, ); - }, [focusedIndex, menuItemData, selectItem]) + }, [focusedIndex, menuItemData, selectItem]); const removeKeyboardListener = () => { if (!keyboardListener.current) { return; } keyboardListener.current(); - } + }; useEffect(() => { // When selecting an image on a native device, it would be redundant to have a second option for choosing a document, @@ -284,15 +304,6 @@ function AttachmentPicker({ type, children}) { return; } - setMenuItemData((oldMenuItemData) => [ - ...oldMenuItemData, - { - icon: Expensicons.Paperclip, - textTranslationKey: 'attachmentPicker.chooseDocument', - pickAttachment: showDocumentPicker, - }, - ]); - return () => { removeKeyboardListener(); }; @@ -312,9 +323,10 @@ function AttachmentPicker({ type, children}) { * * @returns {React.ReactNode} */ - const renderChildren = () => children({ + const renderChildren = () => + children({ openPicker: ({onPicked}) => open(onPicked), - }) + }); return ( <> From 374b292a844d74cdd6d80253278a982d09fb6055 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Fri, 11 Aug 2023 12:03:42 +0200 Subject: [PATCH 17/21] replace empty return statement with Promise.resolve() --- src/components/AttachmentPicker/index.native.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 492c6d4e23c2..3381d6486bd1 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -225,14 +225,14 @@ function AttachmentPicker({type, children}) { const pickAttachment = useCallback( (attachments = []) => { if (attachments.length === 0) { - return; + return Promise.resolve(); } const fileData = _.first(attachments); if (fileData.width === -1 || fileData.height === -1) { showImageCorruptionAlert(); - return; + return Promise.resolve(); } return getDataForUpload(fileData) From f1089dfde0ca670e60132015afc827f592b74df2 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Fri, 11 Aug 2023 13:34:39 +0200 Subject: [PATCH 18/21] take advantage of useKeyboardShortcut hook --- .../AttachmentPicker/index.native.js | 126 +++++++----------- 1 file changed, 50 insertions(+), 76 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 3381d6486bd1..11495f348a9c 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -1,5 +1,5 @@ import _ from 'underscore'; -import React, {useEffect, useState, useRef, useCallback, useMemo} from 'react'; +import React, {useState, useRef, useCallback, useMemo} from 'react'; import {View, Alert, Linking} from 'react-native'; import RNDocumentPicker from 'react-native-document-picker'; import RNFetchBlob from 'react-native-blob-util'; @@ -9,13 +9,13 @@ import CONST from '../../CONST'; import * as FileUtils from '../../libs/fileDownload/FileUtils'; import * as Expensicons from '../Icon/Expensicons'; import launchCamera from './launchCamera'; -import KeyboardShortcut from '../../libs/KeyboardShortcut'; import Popover from '../Popover'; import MenuItem from '../MenuItem'; import styles from '../../styles/styles'; import ArrowKeyFocusManager from '../ArrowKeyFocusManager'; import useLocalize from '../../hooks/useLocalize'; import useWindowDimensions from '../../hooks/useWindowDimensions'; +import useKeyboardShortcut from '../../hooks/useKeyboardShortcut'; const propTypes = { ...basePropTypes, @@ -88,7 +88,6 @@ function AttachmentPicker({type, children}) { const completeAttachmentSelection = useRef(); const onModalHide = useRef(); - const keyboardListener = useRef(); const {translate} = useLocalize(); const {isSmallScreenWidth} = useWindowDimensions(); @@ -127,44 +126,50 @@ function AttachmentPicker({type, children}) { * @param {function} imagePickerFunc - RNImagePicker.launchCamera or RNImagePicker.launchImageLibrary * @returns {Promise} */ - const showImagePicker = useCallback((imagePickerFunc) => - new Promise((resolve, reject) => { - imagePickerFunc(getImagePickerOptions(type), (response) => { - if (response.didCancel) { - // When the user cancelled resolve with no attachment - return resolve(); - } - if (response.errorCode) { - switch (response.errorCode) { - case 'permission': - showPermissionsAlert(); - return resolve(); - default: - showGeneralAlert(); - break; + const showImagePicker = useCallback( + (imagePickerFunc) => + new Promise((resolve, reject) => { + imagePickerFunc(getImagePickerOptions(type), (response) => { + if (response.didCancel) { + // When the user cancelled resolve with no attachment + return resolve(); + } + if (response.errorCode) { + switch (response.errorCode) { + case 'permission': + showPermissionsAlert(); + return resolve(); + default: + showGeneralAlert(); + break; + } + + return reject(new Error(`Error during attachment selection: ${response.errorMessage}`)); } - return reject(new Error(`Error during attachment selection: ${response.errorMessage}`)); - } - - return resolve(response.assets); - }); - }), [showGeneralAlert, showPermissionsAlert, type]); + return resolve(response.assets); + }); + }), + [showGeneralAlert, showPermissionsAlert, type], + ); /** * Launch the DocumentPicker. Results are in the same format as ImagePicker * * @returns {Promise} */ - const showDocumentPicker = useCallback(() => - RNDocumentPicker.pick(documentPickerOptions).catch((error) => { - if (RNDocumentPicker.isCancel(error)) { - return; - } + const showDocumentPicker = useCallback( + () => + RNDocumentPicker.pick(documentPickerOptions).catch((error) => { + if (RNDocumentPicker.isCancel(error)) { + return; + } - showGeneralAlert(error.message); - throw error; - }), [showGeneralAlert]); + showGeneralAlert(error.message); + throw error; + }), + [showGeneralAlert], + ); const menuItemData = useMemo(() => { const data = [ @@ -273,50 +278,19 @@ function AttachmentPicker({type, children}) { [pickAttachment], ); - const attachKeyboardListener = useCallback(() => { - const shortcutConfig = CONST.KEYBOARD_SHORTCUTS.ENTER; - keyboardListener.current = KeyboardShortcut.subscribe( - shortcutConfig.shortcutKey, - () => { - if (focusedIndex === -1) { - return; - } - selectItem(menuItemData[focusedIndex]); - setFocusedIndex(-1); // Reset the focusedIndex on selecting any menu - }, - shortcutConfig.descriptionKey, - shortcutConfig.modifiers, - true, - ); - }, [focusedIndex, menuItemData, selectItem]); - - const removeKeyboardListener = () => { - if (!keyboardListener.current) { - return; - } - keyboardListener.current(); - }; - - useEffect(() => { - // When selecting an image on a native device, it would be redundant to have a second option for choosing a document, - // so it is excluded in this case. - if (type === CONST.ATTACHMENT_PICKER_TYPE.IMAGE) { - return; - } - - return () => { - removeKeyboardListener(); - }; - // eslint-disable-next-line react-hooks/exhaustive-deps -- we only want this effect to run on initial render - }, []); - - useEffect(() => { - if (isVisible) { - attachKeyboardListener(); - } else { - removeKeyboardListener(); - } - }, [isVisible, attachKeyboardListener]); + useKeyboardShortcut( + CONST.KEYBOARD_SHORTCUTS.ENTER, + () => { + if (focusedIndex === -1) { + return; + } + selectItem(menuItemData[focusedIndex]); + setFocusedIndex(-1); // Reset the focusedIndex on selecting any menu + }, + { + isActive: isVisible, + }, + ); /** * Call the `children` renderProp with the interface defined in propTypes From 48922a7535b931a89825916169faa51e0ed774fc Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Fri, 11 Aug 2023 13:44:51 +0200 Subject: [PATCH 19/21] take advantage of useArrowKeyFocusManager hook --- .../AttachmentPicker/index.native.js | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index 11495f348a9c..e07dd23d6f9f 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -12,10 +12,10 @@ import launchCamera from './launchCamera'; import Popover from '../Popover'; import MenuItem from '../MenuItem'; import styles from '../../styles/styles'; -import ArrowKeyFocusManager from '../ArrowKeyFocusManager'; import useLocalize from '../../hooks/useLocalize'; import useWindowDimensions from '../../hooks/useWindowDimensions'; import useKeyboardShortcut from '../../hooks/useKeyboardShortcut'; +import useArrowKeyFocusManager from '../../hooks/useArrowKeyFocusManager'; const propTypes = { ...basePropTypes, @@ -84,7 +84,6 @@ const getDataForUpload = (fileData) => { function AttachmentPicker({type, children}) { const [isVisible, setIsVisible] = useState(false); - const [focusedIndex, setFocusedIndex] = useState(-1); const completeAttachmentSelection = useRef(); const onModalHide = useRef(); @@ -196,6 +195,8 @@ function AttachmentPicker({type, children}) { return data; }, [showDocumentPicker, showImagePicker, type]); + const [focusedIndex, setFocusedIndex] = useArrowKeyFocusManager({initialFocusedIndex: -1, maxIndex: menuItemData.length - 1, isActive: isVisible}); + /** * An attachment error dialog when user selected malformed images */ @@ -311,21 +312,15 @@ function AttachmentPicker({type, children}) { onModalHide={onModalHide.current} > - setFocusedIndex(index)} - > - {_.map(menuItemData, (item, menuIndex) => ( - selectItem(item)} - focused={focusedIndex === menuIndex} - /> - ))} - + {_.map(menuItemData, (item, menuIndex) => ( + selectItem(item)} + focused={focusedIndex === menuIndex} + /> + ))} {renderChildren()} From 33c88fcd143927039fde3be62706e34435c9e6a9 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Fri, 18 Aug 2023 12:22:23 +0200 Subject: [PATCH 20/21] restore general comment --- src/components/AttachmentPicker/index.native.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index e07dd23d6f9f..b2ed5c28edaa 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -82,6 +82,15 @@ const getDataForUpload = (fileData) => { }); }; + +/** + * This component renders a function as a child and + * returns a "show attachment picker" method that takes + * a callback. This is the ios/android implementation + * opening a modal with attachment options + * @param {propTypes} props + * @returns {JSX.Element} + */ function AttachmentPicker({type, children}) { const [isVisible, setIsVisible] = useState(false); From 968434b3167bbbadc2223bc0c215f687884f4183 Mon Sep 17 00:00:00 2001 From: Julian Kobrynski Date: Mon, 21 Aug 2023 08:27:57 +0200 Subject: [PATCH 21/21] run prettier --- src/components/AttachmentPicker/index.native.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/AttachmentPicker/index.native.js b/src/components/AttachmentPicker/index.native.js index b2ed5c28edaa..8ba7ae33606b 100644 --- a/src/components/AttachmentPicker/index.native.js +++ b/src/components/AttachmentPicker/index.native.js @@ -82,7 +82,6 @@ const getDataForUpload = (fileData) => { }); }; - /** * This component renders a function as a child and * returns a "show attachment picker" method that takes