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.

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

Implement dedicated route for CountryPicker in AddressPage #26742

Merged
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
0479a7e
Implement dedicated route for CountryPicker in AddressPage
ygshbht Sep 5, 2023
b5a0ebc
Comment correction
ygshbht Sep 5, 2023
3cbd8d2
Remove unused dependencies in CountrySelection
ygshbht Sep 6, 2023
53286fd
CountrySelection refactoring
ygshbht Sep 6, 2023
d88cc41
CountrySelection remove defaultProps
ygshbht Sep 6, 2023
b8cacd7
Make CountryPickerMenuItem a new component on its own page
ygshbht Sep 6, 2023
f4ae5b2
CountrySelection Navigation bugfix after selecting country
ygshbht Sep 6, 2023
ae6ebcf
Delete Old CountryPicker Modal
ygshbht Sep 6, 2023
9cfcc96
Add display name to CountryPickerMenuitem
ygshbht Sep 6, 2023
ff87483
Remove memoing CountryPickerMenuItem
ygshbht Sep 6, 2023
f161188
Comment modifications to CountrySelection and related files
ygshbht Sep 6, 2023
ce5463c
Merge branch 'fix/address-page-country-picker-navigation' of https://…
ygshbht Sep 6, 2023
ba351d1
CountySelectionPage & related files refactoring and comments
ygshbht Sep 7, 2023
55253e7
Rename CountrySelection to CountrySelectionPage
ygshbht Sep 7, 2023
4252fba
Rename CountrySelection to CountrySelectionPage
ygshbht Sep 7, 2023
178d100
AddressPage: revalidate input forms when country name changes
ygshbht Sep 7, 2023
3ec7cd8
Merge https://github.com/Expensify/App into fix/address-page-country-…
ygshbht Sep 13, 2023
c7dfa90
CountyPicker refactoring
ygshbht Sep 19, 2023
fc551b8
Rename CountryPickerMenuItem to CountrySelector
ygshbht Sep 19, 2023
137790c
Refactoring src/pages/settings/Profile/PersonalDetails/CountrySelecti…
ygshbht Sep 19, 2023
a49e1aa
Refactor src/pages/settings/Profile/PersonalDetails/AddressPage.js
ygshbht Sep 19, 2023
19a1e7e
Refactor src/components/CountrySelector.js
ygshbht Sep 19, 2023
2cca5c0
Pull from remote
ygshbht Sep 19, 2023
9104539
Disable eslint warning CountrySelector
ygshbht Sep 19, 2023
e81fe77
CountrySelector refactoring
ygshbht Sep 20, 2023
cdfe3b8
Merge branch 'main' into fix/address-page-country-picker-navigation
ygshbht Sep 22, 2023
a96a739
Format code
ygshbht Sep 23, 2023
136f90c
Merge https://github.com/Expensify/App into fix/address-page-country-…
ygshbht Sep 25, 2023
a017ee9
Merge branch 'fix/address-page-country-picker-navigation' of https://…
ygshbht Sep 25, 2023
9d19973
Formatting
ygshbht Sep 25, 2023
3af7931
Formatting
ygshbht Sep 25, 2023
945056d
set default prop value of 'value' of CountrySelector to undefined
ygshbht Sep 26, 2023
d2331d3
Merge https://github.com/Expensify/App into fix/address-page-country-…
ygshbht Sep 26, 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
9 changes: 9 additions & 0 deletions src/ROUTES.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ export default {
SETTINGS_PERSONAL_DETAILS_LEGAL_NAME: `${SETTINGS_PERSONAL_DETAILS}/legal-name`,
SETTINGS_PERSONAL_DETAILS_DATE_OF_BIRTH: `${SETTINGS_PERSONAL_DETAILS}/date-of-birth`,
SETTINGS_PERSONAL_DETAILS_ADDRESS: `${SETTINGS_PERSONAL_DETAILS}/address`,
getPersonalDetailsAddressRoute: (country) => `${SETTINGS_PERSONAL_DETAILS}/address?country=${country}`,
SETTINGS_PERSONAL_DETAILS_ADDRESS_COUNTRY: `${SETTINGS_PERSONAL_DETAILS}/address/country`,
getCountryRoute: (country, backTo) => {
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need two functions? when getCountryRoute can be used for both. Also, I see that getPersonalDetailsAddressRoute is not used anywhere

let route = `${SETTINGS_PERSONAL_DETAILS}/address/country?country=${country}`;
if (backTo) {
route += `&backTo=${backTo}`;
}
return route;
},
SETTINGS_CONTACT_METHODS,
SETTINGS_CONTACT_METHOD_DETAILS: `${SETTINGS_CONTACT_METHODS}/:contactMethod/details`,
getEditContactMethodRoute: (contactMethod) => `${SETTINGS_CONTACT_METHODS}/${encodeURIComponent(contactMethod)}/details`,
Expand Down
102 changes: 0 additions & 102 deletions src/components/CountryPicker/CountrySelectorModal.js

This file was deleted.

89 changes: 0 additions & 89 deletions src/components/CountryPicker/index.js

This file was deleted.

53 changes: 53 additions & 0 deletions src/components/CountryPickerMenuItem.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import React from 'react';
import PropTypes from 'prop-types';
import {View} from 'react-native';
import styles from '../styles/styles';
import Navigation from '../libs/Navigation/Navigation';
import ROUTES from '../ROUTES';
import useLocalize from '../hooks/useLocalize';
import MenuItemWithTopDescription from './MenuItemWithTopDescription';
import FormHelpMessage from './FormHelpMessage';

const propTypes = {
/** Error text from form, e.g when no country is selected */
errorText: PropTypes.string,
/** Current selected country */
value: PropTypes.string,
Copy link
Member

Choose a reason for hiding this comment

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

Define inputId prop.

};

const defaultProps = {
errorText: '',
value: '',
};

function CountryPickerMenuItem({errorText, value}, ref) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
function CountryPickerMenuItem({errorText, value}, ref) {
function CountryPickerMenuItem({errorText, countryCode}, ref) {

let's rename it.

const {translate} = useLocalize();
const getCountryFromCountryCode = (code) => translate('allCountries')[code];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const getCountryFromCountryCode = (code) => translate('allCountries')[code];
const countries = translate('allCountries');


const countryTitleDescStyle = value && value.length === 0 ? styles.textNormal : null;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const countryTitleDescStyle = value && value.length === 0 ? styles.textNormal : null;
const country = countries[countryCode] || '';
const countryTitleDescStyle = country.length === 0 ? styles.textNormal : null;


return (
<View>
<MenuItemWithTopDescription
shouldShowRightIcon
title={getCountryFromCountryCode(value)}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
title={getCountryFromCountryCode(value)}
title={country}

ref={ref}
descriptionTextStyle={countryTitleDescStyle}
description={translate('common.country')}
onPress={() => {
const activeRoute = Navigation.getActiveRoute().replace(/\?.*/, '');
Copy link
Member

Choose a reason for hiding this comment

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

we should encode this querystring.

Navigation.navigate(ROUTES.getCountryRoute(value, activeRoute));
}}
/>
<View style={styles.ml5}>
<FormHelpMessage message={errorText} />
</View>
</View>
);
}

CountryPickerMenuItem.propTypes = propTypes;
CountryPickerMenuItem.defaultProps = defaultProps;
Copy link
Member

Choose a reason for hiding this comment

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

Missing displayName.

CountryPickerMenuItem.displayName = 'CountryPickerMenuItem';

export default React.forwardRef(CountryPickerMenuItem);
7 changes: 7 additions & 0 deletions src/libs/Navigation/AppNavigator/ModalStackNavigators.js
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,13 @@ const SettingsModalStackNavigator = createModalStackNavigator([
},
name: 'Settings_PersonalDetails_Address',
},
{
getComponent: () => {
const CountrySelectionPage = require('../../../pages/settings/Profile/PersonalDetails/CountrySelection').default;
return CountrySelectionPage;
},
name: 'Settings_PersonalDetails_Address_Country',
},
{
getComponent: () => {
const SettingsContactMethodsPage = require('../../../pages/settings/Profile/Contacts/ContactMethodsPage').default;
Expand Down
4 changes: 4 additions & 0 deletions src/libs/Navigation/linkingConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,10 @@ export default {
path: ROUTES.SETTINGS_PERSONAL_DETAILS_ADDRESS,
exact: true,
},
Settings_PersonalDetails_Address_Country: {
path: ROUTES.SETTINGS_PERSONAL_DETAILS_ADDRESS_COUNTRY,
exact: true,
},
Settings_TwoFactorAuth: {
path: ROUTES.SETTINGS_2FA,
exact: true,
Expand Down
31 changes: 22 additions & 9 deletions src/pages/settings/Profile/PersonalDetails/AddressPage.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import lodashGet from 'lodash/get';
import _ from 'underscore';
import React, {useState, useCallback} from 'react';
import React, {useState, useCallback, useEffect} from 'react';
import PropTypes from 'prop-types';
import {View} from 'react-native';
import {CONST as COMMON_CONST} from 'expensify-common/lib/CONST';
Expand All @@ -15,13 +15,13 @@ import styles from '../../../../styles/styles';
import * as PersonalDetails from '../../../../libs/actions/PersonalDetails';
import * as ValidationUtils from '../../../../libs/ValidationUtils';
import AddressSearch from '../../../../components/AddressSearch';
import CountryPicker from '../../../../components/CountryPicker';
import StatePicker from '../../../../components/StatePicker';
import Navigation from '../../../../libs/Navigation/Navigation';
import ROUTES from '../../../../ROUTES';
import useLocalize from '../../../../hooks/useLocalize';
import usePrivatePersonalDetails from '../../../../hooks/usePrivatePersonalDetails';
import FullscreenLoadingIndicator from '../../../../components/FullscreenLoadingIndicator';
import CountryPickerMenuItem from '../../../../components/CountryPickerMenuItem';

const propTypes = {
/* Onyx Props */
Expand All @@ -37,6 +37,14 @@ const propTypes = {
country: PropTypes.string,
}),
}),
/** Route from navigation */
ygshbht marked this conversation as resolved.
Show resolved Hide resolved
route: PropTypes.shape({
/** Params from the route */
params: PropTypes.shape({
/** Currently selected country */
country: PropTypes.string,
}),
}).isRequired,
};

const defaultProps = {
Expand All @@ -59,10 +67,11 @@ function updateAddress(values) {
PersonalDetails.updateAddress(values.addressLine1.trim(), values.addressLine2.trim(), values.city.trim(), values.state.trim(), values.zipPostCode.trim().toUpperCase(), values.country);
}

function AddressPage({privatePersonalDetails}) {
function AddressPage({privatePersonalDetails, route}) {
usePrivatePersonalDetails();
const {translate} = useLocalize();
const [currentCountry, setCurrentCountry] = useState(PersonalDetails.getCountryISO(lodashGet(privatePersonalDetails, 'address.country')));
const countryFromUrl = lodashGet(route, 'params.country');
const [currentCountry, setCurrentCountry] = useState(countryFromUrl || PersonalDetails.getCountryISO(lodashGet(privatePersonalDetails, 'address.country')));
const isUSAForm = currentCountry === CONST.COUNTRY.US;
const zipSampleFormat = lodashGet(CONST.COUNTRY_ZIP_REGEX_DATA, [currentCountry, 'samples'], '');
const zipFormat = translate('common.zipCodeExampleFormat', {zipSampleFormat});
Expand Down Expand Up @@ -115,7 +124,7 @@ function AddressPage({privatePersonalDetails}) {
return errors;
}, []);

const handleAddressChange = (value, key) => {
const handleAddressChange = useCallback((value, key) => {
if (key !== 'country' && key !== 'state') {
return;
}
Expand All @@ -125,7 +134,12 @@ function AddressPage({privatePersonalDetails}) {
return;
}
setState(value);
};
}, []);

useEffect(() => {
if (!countryFromUrl) return;
Copy link
Member

Choose a reason for hiding this comment

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

Let's fix this.

handleAddressChange(countryFromUrl, 'country');
}, [countryFromUrl, handleAddressChange]);

if (lodashGet(privatePersonalDetails, 'isLoading', true)) {
return <FullscreenLoadingIndicator />;
Expand Down Expand Up @@ -175,10 +189,9 @@ function AddressPage({privatePersonalDetails}) {
/>
<View style={styles.formSpaceVertical} />
<View style={styles.mhn5}>
<CountryPicker
Copy link
Member

Choose a reason for hiding this comment

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

What happened to old CountryPicker component? Why are we keeping that if not used anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that can be deleted

<CountryPickerMenuItem
inputID="country"
defaultValue={currentCountry}
onValueChange={handleAddressChange}
value={currentCountry}
/>
</View>
<View style={styles.formSpaceVertical} />
Expand Down
Loading