Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Form Provider Refactor] AddDebitCardPage #29983

Merged
merged 27 commits into from
Oct 30, 2023
Merged

[Form Provider Refactor] AddDebitCardPage #29983

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
b4d942f
WIP
kowczarz Oct 19, 2023
a348d1b
Fix checkbox proptypes
kowczarz Oct 19, 2023
38d9184
Fix address search prop type issue
kowczarz Oct 19, 2023
00e12fc
Add checkbox default value
kowczarz Oct 19, 2023
b05c8ba
Prettier
kowczarz Oct 19, 2023
c290dbb
Spread props
kowczarz Oct 19, 2023
64dbcd4
Lint fix
kowczarz Oct 19, 2023
196689d
Fix propType
kowczarz Oct 20, 2023
b418a4c
Code review changes
kowczarz Oct 23, 2023
280e8f6
Fix not displaying all errors
kowczarz Oct 24, 2023
677c2a5
Fix prettier
kowczarz Oct 24, 2023
377cdb9
Merge remote-tracking branch 'expensify/main' into form-migration/add…
kowczarz Oct 24, 2023
c206be1
Merge remote-tracking branch 'expensify/main' into form-migration/add…
kowczarz Oct 24, 2023
c2f54d3
Remove unused ref
kowczarz Oct 24, 2023
06a6f27
Fix ref issue
kowczarz Oct 25, 2023
d93b649
Fix listLoader issue
kowczarz Oct 25, 2023
1142573
Merge remote-tracking branch 'expensify/main' into form-migration/add…
kowczarz Oct 25, 2023
de75ffa
Merge remote-tracking branch 'expensify/main' into form-migration/add…
kowczarz Oct 26, 2023
8dfc203
Merge fixes
kowczarz Oct 26, 2023
211934d
Fix too early error message
kowczarz Oct 26, 2023
674a0dc
Fix prettier
kowczarz Oct 26, 2023
89b00f6
Merge remote-tracking branch 'expensify/main' into form-migration/add…
kowczarz Oct 26, 2023
58f09ab
merge main into form-migration/add-debit-card-page
cdOut Oct 30, 2023
6a4f127
fix lint errors and include new import method
cdOut Oct 30, 2023
0f6688b
Update src/components/CheckboxWithLabel.js
cdOut Oct 30, 2023
b35ebf5
revert import to fix prettier and lint
cdOut Oct 30, 2023
890dd54
remove unused code
cdOut Oct 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 88 additions & 59 deletions src/components/AddressSearch/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import _ from 'underscore';
import React, {useEffect, useMemo, useRef, useState} from 'react';
import React, {useCallback, useEffect, useMemo, useRef, useState} from 'react';
import PropTypes from 'prop-types';
import {Keyboard, LogBox, ScrollView, View, Text, ActivityIndicator} from 'react-native';
import {ActivityIndicator, Keyboard, LogBox, ScrollView, Text, View} from 'react-native';
import {GooglePlacesAutocomplete} from 'react-native-google-places-autocomplete';
import lodashGet from 'lodash/get';
import compose from '../../libs/compose';
Expand Down Expand Up @@ -140,24 +140,46 @@ const defaultProps = {
// 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
kowczarz marked this conversation as resolved.
Show resolved Hide resolved
function AddressSearch(props) {
function AddressSearch({
canUseCurrentLocation,
containerStyles,
defaultValue,
errorText,
hint,
innerRef,
inputID,
isLimitedToUSA,
label,
maxInputLength,
network,
onBlur,
onInputChange,
onPress,
predefinedPlaces,
preferredLocale,
renamedInputKeys,
resultTypes,
shouldSaveDraft,
translate,
value,
}) {
Comment on lines +143 to +165
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to spread props to avoid disabling eslint exhaustive-deps rule in line 372.

const [displayListViewBorder, setDisplayListViewBorder] = useState(false);
const [isTyping, setIsTyping] = useState(false);
const [isFocused, setIsFocused] = useState(false);
const [searchValue, setSearchValue] = useState(props.value || props.defaultValue || '');
const [searchValue, setSearchValue] = useState(value || defaultValue || '');
const [locationErrorCode, setLocationErrorCode] = useState(null);
const [isFetchingCurrentLocation, setIsFetchingCurrentLocation] = useState(false);
const shouldTriggerGeolocationCallbacks = useRef(true);
const containerRef = useRef();
const query = useMemo(
() => ({
language: props.preferredLocale,
types: props.resultTypes,
components: props.isLimitedToUSA ? 'country:us' : undefined,
language: preferredLocale,
types: resultTypes,
components: isLimitedToUSA ? 'country:us' : undefined,
}),
[props.preferredLocale, props.resultTypes, props.isLimitedToUSA],
[preferredLocale, resultTypes, isLimitedToUSA],
);
const shouldShowCurrentLocationButton = props.canUseCurrentLocation && searchValue.trim().length === 0 && isFocused;
const shouldShowCurrentLocationButton = canUseCurrentLocation && searchValue.trim().length === 0 && isFocused;

const saveLocationDetails = (autocompleteData, details) => {
const addressComponents = details.address_components;
Expand All @@ -166,7 +188,7 @@ function AddressSearch(props) {
// 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)) {
props.onPress({
onPress({
address: lodashGet(details, 'description', ''),
lat: lodashGet(details, 'geometry.location.lat', 0),
lng: lodashGet(details, 'geometry.location.lng', 0),
Expand Down Expand Up @@ -252,7 +274,7 @@ function AddressSearch(props) {

// 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 props.renamedInputKeys.street2 === 'undefined') {
if (subpremise && typeof renamedInputKeys.street2 === 'undefined') {
values.street += `, ${subpremise}`;
}

Expand All @@ -261,19 +283,19 @@ function AddressSearch(props) {
values.country = country;
}

if (props.inputID) {
_.each(values, (value, key) => {
const inputKey = lodashGet(props.renamedInputKeys, key, key);
if (inputID) {
_.each(values, (inputValue, key) => {
const inputKey = lodashGet(renamedInputKeys, key, key);
if (!inputKey) {
return;
}
props.onInputChange(value, inputKey);
onInputChange(inputValue, inputKey);
});
} else {
props.onInputChange(values);
onInputChange(values);
}

props.onPress(values);
onPress(values);
};

/** Gets the user's current location and registers success/error callbacks */
Expand Down Expand Up @@ -303,7 +325,7 @@ function AddressSearch(props) {
lng: successData.coords.longitude,
address: CONST.YOUR_LOCATION_TEXT,
};
props.onPress(location);
onPress(location);
},
(errorData) => {
if (!shouldTriggerGeolocationCallbacks.current) {
Expand All @@ -321,16 +343,16 @@ function AddressSearch(props) {
};

const renderHeaderComponent = () =>
props.predefinedPlaces.length > 0 && (
predefinedPlaces.length > 0 && (
<>
{/* This will show current location button in list if there are some recent destinations */}
{shouldShowCurrentLocationButton && (
<CurrentLocationButton
onPress={getCurrentLocation}
isDisabled={props.network.isOffline}
isDisabled={network.isOffline}
/>
)}
{!props.value && <Text style={[styles.textLabel, styles.colorMuted, styles.pv2, styles.ph3, styles.overflowAuto]}>{props.translate('common.recentDestinations')}</Text>}
{!value && <Text style={[styles.textLabel, styles.colorMuted, styles.pv2, styles.ph3, styles.overflowAuto]}>{translate('common.recentDestinations')}</Text>}
</>
);

Expand All @@ -342,6 +364,26 @@ function AddressSearch(props) {
};
}, []);

const listEmptyComponent = useCallback(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can use useMemo here since we just want to store the value

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the function is best suited for a child to be used as a functional component. Otherwise, the react lifecycle will not work properly and the prop used inside network.isOffline will not work as expected.

Copy link
Contributor Author

@kowczarz kowczarz Oct 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luacmartins I think I think it's safer to use callback and the value of the prop needs to be a function.

() =>
network.isOffline || !isTyping ? null : (
<Text style={[styles.textLabel, styles.colorMuted, styles.pv4, styles.ph3, styles.overflowAuto]}>{translate('common.noResultsFound')}</Text>
),
[isTyping, translate, network.isOffline],
);

const listLoader = useCallback(
kowczarz marked this conversation as resolved.
Show resolved Hide resolved
() => (
<View style={[styles.pv4]}>
<ActivityIndicator
color={themeColors.spinner}
size="small"
/>
</View>
),
[],
);

return (
/*
* The GooglePlacesAutocomplete component uses a VirtualizedList internally,
Expand All @@ -368,20 +410,9 @@ function AddressSearch(props) {
fetchDetails
suppressDefaultStyles
enablePoweredByContainer={false}
predefinedPlaces={props.predefinedPlaces}
listEmptyComponent={
props.network.isOffline || !isTyping ? null : (
<Text style={[styles.textLabel, styles.colorMuted, styles.pv4, styles.ph3, styles.overflowAuto]}>{props.translate('common.noResultsFound')}</Text>
)
}
listLoaderComponent={
<View style={[styles.pv4]}>
<ActivityIndicator
color={themeColors.spinner}
size="small"
/>
</View>
}
predefinedPlaces={predefinedPlaces}
listEmptyComponent={listEmptyComponent}
listLoaderComponent={listLoader}
renderHeaderComponent={renderHeaderComponent}
onPress={(data, details) => {
saveLocationDetails(data, details);
Expand All @@ -397,34 +428,31 @@ function AddressSearch(props) {
query={query}
requestUrl={{
useOnPlatform: 'all',
url: props.network.isOffline ? null : ApiUtils.getCommandURL({command: 'Proxy_GooglePlaces&proxyUrl='}),
url: network.isOffline ? null : ApiUtils.getCommandURL({command: 'Proxy_GooglePlaces&proxyUrl='}),
}}
textInputProps={{
InputComp: TextInput,
ref: (node) => {
if (!props.innerRef) {
if (!innerRef) {
return;
}

if (_.isFunction(props.innerRef)) {
props.innerRef(node);
if (_.isFunction(innerRef)) {
innerRef(node);
return;
}

// eslint-disable-next-line no-param-reassign
props.innerRef.current = node;
innerRef.current = node;
},
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,
label,
containerStyles,
errorText,
hint: displayListViewBorder || (predefinedPlaces.length === 0 && shouldShowCurrentLocationButton) || (canUseCurrentLocation && isTyping) ? undefined : hint,
value,
defaultValue,
inputID,
shouldSaveDraft,
onFocus: () => {
setIsFocused(true);
},
Expand All @@ -434,24 +462,24 @@ function AddressSearch(props) {
setIsFocused(false);
setIsTyping(false);
}
props.onBlur();
onBlur();
},
autoComplete: 'off',
onInputChange: (text) => {
setSearchValue(text);
setIsTyping(true);
if (props.inputID) {
props.onInputChange(text);
if (inputID) {
onInputChange(text);
} else {
props.onInputChange({street: text});
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(props.predefinedPlaces)) {
if (_.isEmpty(text) && _.isEmpty(predefinedPlaces)) {
setDisplayListViewBorder(false);
}
},
maxLength: props.maxInputLength,
maxLength: maxInputLength,
spellCheck: false,
}}
styles={{
Expand All @@ -472,17 +500,18 @@ function AddressSearch(props) {
}}
inbetweenCompo={
// We want to show the current location button even if there are no recent destinations
props.predefinedPlaces.length === 0 && shouldShowCurrentLocationButton ? (
predefinedPlaces.length === 0 && shouldShowCurrentLocationButton ? (
<View style={[StyleUtils.getGoogleListViewStyle(true), styles.overflowAuto, styles.borderLeft, styles.borderRight]}>
<CurrentLocationButton
onPress={getCurrentLocation}
isDisabled={props.network.isOffline}
isDisabled={network.isOffline}
/>
</View>
) : (
<></>
)
}
placeholder=""
kowczarz marked this conversation as resolved.
Show resolved Hide resolved
/>
<LocationErrorMessage
onClose={() => setLocationErrorCode(null)}
Expand Down
2 changes: 1 addition & 1 deletion src/components/CheckboxWithLabel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const propTypes = {
defaultValue: PropTypes.bool,

/** React ref being forwarded to the Checkbox input */
forwardedRef: PropTypes.func,
forwardedRef: PropTypes.oneOfType([PropTypes.func, PropTypes.shape({current: PropTypes.instanceOf(React.Component)})]),
kowczarz marked this conversation as resolved.
Show resolved Hide resolved

/** The ID used to uniquely identify the input in a Form */
/* eslint-disable-next-line react/no-unused-prop-types */
Expand Down
Loading
Loading