From d7e146183ed1ab652bce366985ccdc5c24636c7e Mon Sep 17 00:00:00 2001 From: Alex Beaman Date: Wed, 1 Nov 2023 17:51:01 +0200 Subject: [PATCH 1/2] Revert "[Form Provider Refactor] AddDebitCardPage" --- src/components/AddressSearch/index.js | 148 ++++++++---------- src/components/CheckboxWithLabel.js | 3 +- src/components/Form/FormProvider.js | 21 +-- src/components/Form/InputWrapper.js | 3 +- src/pages/settings/Wallet/AddDebitCardPage.js | 36 ++--- 5 files changed, 80 insertions(+), 131 deletions(-) diff --git a/src/components/AddressSearch/index.js b/src/components/AddressSearch/index.js index f8219c028853..e1077a0199cd 100644 --- a/src/components/AddressSearch/index.js +++ b/src/components/AddressSearch/index.js @@ -1,6 +1,6 @@ import lodashGet from 'lodash/get'; import PropTypes from 'prop-types'; -import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react'; +import React, {useEffect, useMemo, useRef, useState} from 'react'; import {ActivityIndicator, Keyboard, LogBox, ScrollView, Text, View} from 'react-native'; import {GooglePlacesAutocomplete} from 'react-native-google-places-autocomplete'; import _ from 'underscore'; @@ -140,46 +140,27 @@ const defaultProps = { resultTypes: 'address', }; -function AddressSearch({ - canUseCurrentLocation, - containerStyles, - defaultValue, - errorText, - hint, - innerRef, - inputID, - isLimitedToUSA, - label, - maxInputLength, - network, - onBlur, - onInputChange, - onPress, - predefinedPlaces, - preferredLocale, - renamedInputKeys, - resultTypes, - shouldSaveDraft, - translate, - value, -}) { +// Do not convert to class component! It's been tried before and presents more challenges than it's worth. +// Relevant thread: https://expensify.slack.com/archives/C03TQ48KC/p1634088400387400 +// Reference: https://github.com/FaridSafi/react-native-google-places-autocomplete/issues/609#issuecomment-886133839 +function AddressSearch(props) { const [displayListViewBorder, setDisplayListViewBorder] = useState(false); const [isTyping, setIsTyping] = useState(false); const [isFocused, setIsFocused] = useState(false); - const [searchValue, setSearchValue] = useState(value || defaultValue || ''); + const [searchValue, setSearchValue] = useState(props.value || props.defaultValue || ''); const [locationErrorCode, setLocationErrorCode] = useState(null); const [isFetchingCurrentLocation, setIsFetchingCurrentLocation] = useState(false); const shouldTriggerGeolocationCallbacks = useRef(true); const containerRef = useRef(); const query = useMemo( () => ({ - language: preferredLocale, - types: resultTypes, - components: isLimitedToUSA ? 'country:us' : undefined, + language: props.preferredLocale, + types: props.resultTypes, + components: props.isLimitedToUSA ? 'country:us' : undefined, }), - [preferredLocale, resultTypes, isLimitedToUSA], + [props.preferredLocale, props.resultTypes, props.isLimitedToUSA], ); - const shouldShowCurrentLocationButton = canUseCurrentLocation && searchValue.trim().length === 0 && isFocused; + const shouldShowCurrentLocationButton = props.canUseCurrentLocation && searchValue.trim().length === 0 && isFocused; const saveLocationDetails = (autocompleteData, details) => { const addressComponents = details.address_components; @@ -188,7 +169,7 @@ function AddressSearch({ // to this component which don't match the usual properties coming from auto-complete. In that case, only a limited // amount of data massaging needs to happen for what the parent expects to get from this function. if (_.size(details)) { - onPress({ + props.onPress({ address: lodashGet(details, 'description'), lat: lodashGet(details, 'geometry.location.lat', 0), lng: lodashGet(details, 'geometry.location.lng', 0), @@ -275,7 +256,7 @@ function AddressSearch({ // Not all pages define the Address Line 2 field, so in that case we append any additional address details // (e.g. Apt #) to Address Line 1 - if (subpremise && typeof renamedInputKeys.street2 === 'undefined') { + if (subpremise && typeof props.renamedInputKeys.street2 === 'undefined') { values.street += `, ${subpremise}`; } @@ -284,19 +265,19 @@ function AddressSearch({ values.country = country; } - if (inputID) { - _.each(values, (inputValue, key) => { - const inputKey = lodashGet(renamedInputKeys, key, key); + if (props.inputID) { + _.each(values, (value, key) => { + const inputKey = lodashGet(props.renamedInputKeys, key, key); if (!inputKey) { return; } - onInputChange(inputValue, inputKey); + props.onInputChange(value, inputKey); }); } else { - onInputChange(values); + props.onInputChange(values); } - onPress(values); + props.onPress(values); }; /** Gets the user's current location and registers success/error callbacks */ @@ -344,16 +325,16 @@ function AddressSearch({ }; const renderHeaderComponent = () => - predefinedPlaces.length > 0 && ( + props.predefinedPlaces.length > 0 && ( <> {/* This will show current location button in list if there are some recent destinations */} {shouldShowCurrentLocationButton && ( )} - {!value && {translate('common.recentDestinations')}} + {!props.value && {props.translate('common.recentDestinations')}} ); @@ -365,26 +346,6 @@ function AddressSearch({ }; }, []); - const listEmptyComponent = useCallback( - () => - network.isOffline || !isTyping ? null : ( - {translate('common.noResultsFound')} - ), - [isTyping, translate, network.isOffline], - ); - - const listLoader = useCallback( - () => ( - - - - ), - [], - ); - return ( /* * The GooglePlacesAutocomplete component uses a VirtualizedList internally, @@ -411,10 +372,20 @@ function AddressSearch({ fetchDetails suppressDefaultStyles enablePoweredByContainer={false} - predefinedPlaces={predefinedPlaces} - listEmptyComponent={listEmptyComponent} - listLoaderComponent={listLoader} - renderHeaderComponent={renderHeaderComponent} + predefinedPlaces={props.predefinedPlaces} + listEmptyComponent={ + props.network.isOffline || !isTyping ? null : ( + {props.translate('common.noResultsFound')} + ) + } + listLoaderComponent={ + + + + } renderRow={(data) => { const title = data.isPredefinedPlace ? data.name : data.structured_formatting.main_text; const subtitle = data.isPredefinedPlace ? data.description : data.structured_formatting.secondary_text; @@ -425,6 +396,7 @@ function AddressSearch({ ); }} + renderHeaderComponent={renderHeaderComponent} onPress={(data, details) => { saveLocationDetails(data, details); setIsTyping(false); @@ -439,31 +411,34 @@ function AddressSearch({ query={query} requestUrl={{ useOnPlatform: 'all', - url: network.isOffline ? null : ApiUtils.getCommandURL({command: 'Proxy_GooglePlaces&proxyUrl='}), + url: props.network.isOffline ? null : ApiUtils.getCommandURL({command: 'Proxy_GooglePlaces&proxyUrl='}), }} textInputProps={{ InputComp: TextInput, ref: (node) => { - if (!innerRef) { + if (!props.innerRef) { return; } - if (_.isFunction(innerRef)) { - innerRef(node); + if (_.isFunction(props.innerRef)) { + props.innerRef(node); return; } // eslint-disable-next-line no-param-reassign - innerRef.current = node; + props.innerRef.current = node; }, - label, - containerStyles, - errorText, - hint: displayListViewBorder || (predefinedPlaces.length === 0 && shouldShowCurrentLocationButton) || (canUseCurrentLocation && isTyping) ? undefined : hint, - value, - defaultValue, - inputID, - shouldSaveDraft, + label: props.label, + containerStyles: props.containerStyles, + errorText: props.errorText, + hint: + displayListViewBorder || (props.predefinedPlaces.length === 0 && shouldShowCurrentLocationButton) || (props.canUseCurrentLocation && isTyping) + ? undefined + : props.hint, + value: props.value, + defaultValue: props.defaultValue, + inputID: props.inputID, + shouldSaveDraft: props.shouldSaveDraft, onFocus: () => { setIsFocused(true); }, @@ -473,24 +448,24 @@ function AddressSearch({ setIsFocused(false); setIsTyping(false); } - onBlur(); + props.onBlur(); }, autoComplete: 'off', onInputChange: (text) => { setSearchValue(text); setIsTyping(true); - if (inputID) { - onInputChange(text); + if (props.inputID) { + props.onInputChange(text); } else { - onInputChange({street: text}); + props.onInputChange({street: text}); } // If the text is empty and we have no predefined places, we set displayListViewBorder to false to prevent UI flickering - if (_.isEmpty(text) && _.isEmpty(predefinedPlaces)) { + if (_.isEmpty(text) && _.isEmpty(props.predefinedPlaces)) { setDisplayListViewBorder(false); } }, - maxLength: maxInputLength, + maxLength: props.maxInputLength, spellCheck: false, }} styles={{ @@ -511,18 +486,17 @@ function AddressSearch({ }} inbetweenCompo={ // We want to show the current location button even if there are no recent destinations - predefinedPlaces.length === 0 && shouldShowCurrentLocationButton ? ( + props.predefinedPlaces.length === 0 && shouldShowCurrentLocationButton ? ( ) : ( <> ) } - placeholder="" /> setLocationErrorCode(null)} diff --git a/src/components/CheckboxWithLabel.js b/src/components/CheckboxWithLabel.js index f18ec346dfa2..4bffadecb733 100644 --- a/src/components/CheckboxWithLabel.js +++ b/src/components/CheckboxWithLabel.js @@ -7,7 +7,6 @@ import variables from '@styles/variables'; import Checkbox from './Checkbox'; import FormHelpMessage from './FormHelpMessage'; import PressableWithFeedback from './Pressable/PressableWithFeedback'; -import refPropTypes from './refPropTypes'; import Text from './Text'; /** @@ -55,7 +54,7 @@ const propTypes = { defaultValue: PropTypes.bool, /** React ref being forwarded to the Checkbox input */ - forwardedRef: refPropTypes, + forwardedRef: PropTypes.func, /** The ID used to uniquely identify the input in a Form */ /* eslint-disable-next-line react/no-unused-prop-types */ diff --git a/src/components/Form/FormProvider.js b/src/components/Form/FormProvider.js index 85408323c9f2..92baa9727832 100644 --- a/src/components/Form/FormProvider.js +++ b/src/components/Form/FormProvider.js @@ -71,8 +71,6 @@ const propTypes = { shouldValidateOnChange: PropTypes.bool, }; -const VALIDATE_DELAY = 200; - const defaultProps = { isSubmitButtonVisible: true, formState: { @@ -248,28 +246,19 @@ function FormProvider({validate, formID, shouldValidateOnBlur, shouldValidateOnC // as this is already happening by the value prop. defaultValue: undefined, onTouched: (event) => { - setTimeout(() => { - setTouchedInput(inputID); - }, VALIDATE_DELAY); + setTouchedInput(inputID); if (_.isFunction(propsToParse.onTouched)) { propsToParse.onTouched(event); } }, onPress: (event) => { - setTimeout(() => { - setTouchedInput(inputID); - }, VALIDATE_DELAY); + setTouchedInput(inputID); if (_.isFunction(propsToParse.onPress)) { propsToParse.onPress(event); } }, - onPressOut: (event) => { - // To prevent validating just pressed inputs, we need to set the touched input right after - // onValidate and to do so, we need to delays setTouchedInput of the same amount of time - // as the onValidate is delayed - setTimeout(() => { - setTouchedInput(inputID); - }, VALIDATE_DELAY); + onPressIn: (event) => { + setTouchedInput(inputID); if (_.isFunction(propsToParse.onPressIn)) { propsToParse.onPressIn(event); } @@ -285,7 +274,7 @@ function FormProvider({validate, formID, shouldValidateOnBlur, shouldValidateOnC if (shouldValidateOnBlur) { onValidate(inputValues, !hasServerError); } - }, VALIDATE_DELAY); + }, 200); } if (_.isFunction(propsToParse.onBlur)) { diff --git a/src/components/Form/InputWrapper.js b/src/components/Form/InputWrapper.js index b2e6f4477e89..99237fd8db43 100644 --- a/src/components/Form/InputWrapper.js +++ b/src/components/Form/InputWrapper.js @@ -1,13 +1,12 @@ import PropTypes from 'prop-types'; import React, {forwardRef, useContext} from 'react'; -import refPropTypes from '@components/refPropTypes'; import FormContext from './FormContext'; const propTypes = { InputComponent: PropTypes.oneOfType([PropTypes.func, PropTypes.elementType]).isRequired, inputID: PropTypes.string.isRequired, valueType: PropTypes.string, - forwardedRef: refPropTypes, + forwardedRef: PropTypes.oneOfType([PropTypes.func, PropTypes.shape({current: PropTypes.instanceOf(React.Component)})]), }; const defaultProps = { diff --git a/src/pages/settings/Wallet/AddDebitCardPage.js b/src/pages/settings/Wallet/AddDebitCardPage.js index 5b99bd49812c..ab948dcdc589 100644 --- a/src/pages/settings/Wallet/AddDebitCardPage.js +++ b/src/pages/settings/Wallet/AddDebitCardPage.js @@ -4,8 +4,7 @@ import {View} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import AddressSearch from '@components/AddressSearch'; import CheckboxWithLabel from '@components/CheckboxWithLabel'; -import FormProvider from '@components/Form/FormProvider'; -import InputWrapper from '@components/Form/InputWrapper'; +import Form from '@components/Form'; import HeaderWithBackButton from '@components/HeaderWithBackButton'; import ScreenWrapper from '@components/ScreenWrapper'; import StatePicker from '@components/StatePicker'; @@ -118,7 +117,7 @@ function DebitCardPage(props) { title={translate('addDebitCardPage.addADebitCard')} onBackButtonPress={() => Navigation.goBack(ROUTES.SETTINGS_WALLET)} /> - - (nameOnCardRef.current = ref)} spellCheck={false} /> - - - - - - + - ( {`${translate('common.iAcceptThe')}`} @@ -210,7 +198,7 @@ function DebitCardPage(props) { )} style={[styles.mt4]} /> - + ); } From 3488601c8482cf18450388813303bb1da823c413 Mon Sep 17 00:00:00 2001 From: Alex Beaman Date: Wed, 1 Nov 2023 17:54:07 +0200 Subject: [PATCH 2/2] Oops missed a revert --- src/components/AddressSearch/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/AddressSearch/index.js b/src/components/AddressSearch/index.js index e1077a0199cd..3e122e029969 100644 --- a/src/components/AddressSearch/index.js +++ b/src/components/AddressSearch/index.js @@ -307,7 +307,7 @@ function AddressSearch(props) { lng: successData.coords.longitude, address: CONST.YOUR_LOCATION_TEXT, }; - onPress(location); + props.onPress(location); }, (errorData) => { if (!shouldTriggerGeolocationCallbacks.current) {