-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Changes from 12 commits
0479a7e
b5a0ebc
3cbd8d2
53286fd
d88cc41
b8cacd7
f4ae5b2
ae6ebcf
9cfcc96
ff87483
f161188
ce5463c
ba351d1
55253e7
4252fba
178d100
3ec7cd8
c7dfa90
fc551b8
137790c
a49e1aa
19a1e7e
2cca5c0
9104539
e81fe77
cdfe3b8
a96a739
136f90c
a017ee9
9d19973
3af7931
945056d
d2331d3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
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, | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Define inputId |
||||||||
}; | ||||||||
|
||||||||
const defaultProps = { | ||||||||
errorText: '', | ||||||||
value: '', | ||||||||
}; | ||||||||
|
||||||||
function CountryPickerMenuItem({errorText, value}, ref) { | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
let's rename it. |
||||||||
const {translate} = useLocalize(); | ||||||||
const getCountryFromCountryCode = (code) => translate('allCountries')[code]; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
|
||||||||
const countryTitleDescStyle = value && value.length === 0 ? styles.textNormal : null; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
|
||||||||
return ( | ||||||||
<View> | ||||||||
<MenuItemWithTopDescription | ||||||||
shouldShowRightIcon | ||||||||
title={getCountryFromCountryCode(value)} | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
ref={ref} | ||||||||
descriptionTextStyle={countryTitleDescStyle} | ||||||||
description={translate('common.country')} | ||||||||
onPress={() => { | ||||||||
const activeRoute = Navigation.getActiveRoute().replace(/\?.*/, ''); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing displayName. |
||||||||
CountryPickerMenuItem.displayName = 'CountryPickerMenuItem'; | ||||||||
|
||||||||
export default React.forwardRef(CountryPickerMenuItem); |
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'; | ||
|
@@ -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 */ | ||
|
@@ -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 = { | ||
|
@@ -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}); | ||
|
@@ -115,7 +124,7 @@ function AddressPage({privatePersonalDetails}) { | |
return errors; | ||
}, []); | ||
|
||
const handleAddressChange = (value, key) => { | ||
const handleAddressChange = useCallback((value, key) => { | ||
if (key !== 'country' && key !== 'state') { | ||
return; | ||
} | ||
|
@@ -125,7 +134,12 @@ function AddressPage({privatePersonalDetails}) { | |
return; | ||
} | ||
setState(value); | ||
}; | ||
}, []); | ||
|
||
useEffect(() => { | ||
if (!countryFromUrl) return; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 />; | ||
|
@@ -175,10 +189,9 @@ function AddressPage({privatePersonalDetails}) { | |
/> | ||
<View style={styles.formSpaceVertical} /> | ||
<View style={styles.mhn5}> | ||
<CountryPicker | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happened to old There was a problem hiding this comment. Choose a reason for hiding this commentThe 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} /> | ||
|
There was a problem hiding this comment.
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