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

Fix/address page state navigation 2 #36770

Merged
merged 75 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from 73 commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
cd8ca07
Convert StatePicker modal to StateSelection page
ygshbht Oct 7, 2023
1284e59
Incorporation state resets bugfix
ygshbht Oct 7, 2023
963cf67
Modify pages to accomodate for StateSelection
ygshbht Oct 7, 2023
1305561
Merge https://github.com/Expensify/App into fix/address-page-state-na…
ygshbht Oct 7, 2023
16ba084
Refactor to use useGeographicalStateFromRoute hook
ygshbht Oct 8, 2023
0f83cff
move getStateFromRoute to useGeographicalStateFromRoute
ygshbht Oct 27, 2023
0951d5d
Add displayname to StateSelectorWithRef
ygshbht Oct 27, 2023
3b88d46
Merge https://github.com/Expensify/App into fix/address-page-state-na…
ygshbht Oct 27, 2023
598c7c6
Fix AddressPage: State not changing on autocomplete
ygshbht Oct 27, 2023
4a21636
Refactor StateSelector logic
ygshbht Oct 27, 2023
312d0c6
Refactor AddressPage
ygshbht Oct 27, 2023
8a293b5
linting fixes StateSelector
ygshbht Oct 28, 2023
c946cbb
linting
ygshbht Oct 28, 2023
144eea9
StateSelector and related files refactoring
ygshbht Oct 29, 2023
d2848d3
Merge https://github.com/Expensify/App into fix/address-page-state-na…
ygshbht Oct 29, 2023
17f2aab
Fix incorrect merge
ygshbht Oct 29, 2023
9fb4f72
Reverted scripts/shellCheck.sh to state at 33f15e8d85b3366b1a09bca914…
ygshbht Oct 29, 2023
d6a643c
Linting
ygshbht Oct 29, 2023
080285a
Merge https://github.com/Expensify/App into fix/address-page-state-na…
ygshbht Oct 31, 2023
9bb88ee
Comment formatting src/hooks/useGeographicalStateFromRoute.ts
ygshbht Oct 31, 2023
5e38447
Rename useStateFomrUrl to shouldUseStateFromUrl
ygshbht Oct 31, 2023
470ec03
forwardedRef type correction StateSelector
ygshbht Oct 31, 2023
8f28580
StateSelector: rename paramName to queryParam
ygshbht Oct 31, 2023
7be20b3
Remove lodashGet from useGeographicalStateFromRoute
ygshbht Nov 1, 2023
428e48c
StateSelectionPage appendParam restructuring
ygshbht Nov 1, 2023
7cd8b6f
Prettier formatting
ygshbht Nov 1, 2023
5a02b71
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Feb 18, 2024
ec0eb6f
Comment typo fix src/pages/settings/Profile/PersonalDetails/StateSele…
ygshbht Feb 19, 2024
07f6f21
Comment formatting src/pages/settings/Profile/PersonalDetails/Address…
ygshbht Feb 19, 2024
77f236c
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Feb 19, 2024
877740b
Fix TS errors
ygshbht Feb 20, 2024
6731502
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Feb 26, 2024
a6faef4
Refactor StateSelectionPage with more explanatory comments
ygshbht Feb 26, 2024
7db144b
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Feb 28, 2024
c0c1c6b
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Feb 29, 2024
7b15308
Incorrect header space fix: src/pages/settings/Profile/PersonalDetail…
ygshbht Mar 1, 2024
21b215a
Remove unused code StateSelectionPage.tsx
ygshbht Mar 1, 2024
0fd833f
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Mar 1, 2024
8c59969
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Mar 6, 2024
397278d
Prettier formatting
ygshbht Mar 7, 2024
4a8d6d6
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Mar 8, 2024
5d223d6
fix: handle potential 'undefined' object issue
ygshbht Mar 8, 2024
3a3c767
Typo fix src/components/StateSelector.tsx
ygshbht Mar 8, 2024
c56f8d5
Format src/libs/Url.ts
ygshbht Mar 8, 2024
470be54
Format src/libs/Url.ts
ygshbht Mar 8, 2024
1670b66
Format src/pages/settings/Profile/PersonalDetails/StateSelectionPage.tsx
ygshbht Mar 8, 2024
5d2ff1e
Format src/pages/settings/Profile/PersonalDetails/AddressPage.tsx
ygshbht Mar 8, 2024
18717ea
Comment for function useGeographicalStateFromRoute
ygshbht Mar 8, 2024
3ba97a5
hash append for StateSelector edge cases
ygshbht Mar 8, 2024
0dc7f49
Comment update useGeographicalStateFromRoute
ygshbht Mar 8, 2024
57bcee2
Add StateSelector to MoneyRequest Navigation stack to work for /send/…
ygshbht Mar 9, 2024
9ffea05
Formatting src/pages/settings/Profile/PersonalDetails/AddressPage.tsx
ygshbht Mar 9, 2024
d25904c
Commnet udpate src/ROUTES.ts
ygshbht Mar 9, 2024
81f452c
Comment update ROUTES.ts
ygshbht Mar 9, 2024
dc35055
Example update src/hooks/useGeographicalStateFromRoute.ts
ygshbht Mar 11, 2024
a764d21
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Mar 11, 2024
78afcd5
Refactor linking config StateSelector
ygshbht Mar 11, 2024
ecc3953
Revert "hash append for StateSelector edge cases"
ygshbht Mar 12, 2024
713a91c
Sync URL 'state' param with StateSelector value
ygshbht Mar 12, 2024
9a79421
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Mar 12, 2024
2a76bed
change moneyrequest/state url to request/state
ygshbht Mar 13, 2024
49f2e46
change moneyrequest/state url to request/state
ygshbht Mar 13, 2024
15c54b2
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Mar 20, 2024
58d92f1
StateSelector bugfix - for issue causing url to not update in browser…
ygshbht Mar 20, 2024
af6c3c5
useGeographicalStateFromRoute - check for state before accessing stat…
ygshbht Mar 20, 2024
24cbec3
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Mar 23, 2024
2619a29
Fix: Inconsistent validation error for required field in home address
ygshbht Mar 23, 2024
f48a7fc
Remove unused code: StateSelector
ygshbht Mar 23, 2024
4efccc0
StateSelectionPage: Fix issue of page crash when country is changed m…
ygshbht Mar 25, 2024
b9fe092
es lint comment formatting
ygshbht Mar 25, 2024
5f0cc47
eslint Formatting fix
ygshbht Mar 25, 2024
3585296
prettier
ygshbht Mar 25, 2024
639a646
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Mar 28, 2024
7e81480
Merge branch 'main' into fix/address-page-state-navigation-2
ygshbht Mar 29, 2024
3f87bf2
Add explanatory comments to StateSelector
ygshbht Mar 29, 2024
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
20 changes: 20 additions & 0 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,16 @@ const ROUTES = {
route: 'settings/profile/address/country',
getRoute: (country: string, backTo?: string) => getUrlWithBackToParam(`settings/profile/address/country?country=${country}`, backTo),
},
SETTINGS_ADDRESS_STATE: {
route: 'settings/profile/address/state',
mountiny marked this conversation as resolved.
Show resolved Hide resolved

getRoute: (state?: string, backTo?: string, label?: string) =>
`${getUrlWithBackToParam(`settings/profile/address/state${state ? `?state=${encodeURIComponent(state)}` : ''}`, backTo)}${
// the label param can be an empty string so we cannot use a nullish ?? operator
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
label ? `${backTo || state ? '&' : '?'}label=${encodeURIComponent(label)}` : ''
}` as const,
},
SETTINGS_CONTACT_METHODS: {
route: 'settings/profile/contact-methods',
getRoute: (backTo?: string) => getUrlWithBackToParam('settings/profile/contact-methods', backTo),
Expand Down Expand Up @@ -407,6 +417,16 @@ const ROUTES = {
`create/${iouType}/start/${transactionID}/${reportID}/scan` as const,
},

MONEY_REQUEST_STATE_SELECTOR: {
route: 'request/state',

getRoute: (state?: string, backTo?: string, label?: string) =>
`${getUrlWithBackToParam(`request/state${state ? `?state=${encodeURIComponent(state)}` : ''}`, backTo)}${
// the label param can be an empty string so we cannot use a nullish ?? operator
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
label ? `${backTo || state ? '&' : '?'}label=${encodeURIComponent(label)}` : ''
}` as const,
},
IOU_REQUEST: 'request/new',
IOU_SEND: 'send/new',
IOU_SEND_ADD_BANK_ACCOUNT: 'send/new/add-bank-account',
Expand Down
2 changes: 2 additions & 0 deletions src/SCREENS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const SCREENS = {
DATE_OF_BIRTH: 'Settings_DateOfBirth',
ADDRESS: 'Settings_Address',
ADDRESS_COUNTRY: 'Settings_Address_Country',
ADDRESS_STATE: 'Settings_Address_State',
},

PREFERENCES: {
Expand Down Expand Up @@ -156,6 +157,7 @@ const SCREENS = {
WAYPOINT: 'Money_Request_Waypoint',
EDIT_WAYPOINT: 'Money_Request_Edit_Waypoint',
RECEIPT: 'Money_Request_Receipt',
STATE_SELECTOR: 'Money_Request_State_Selector',
},

IOU_SEND: {
Expand Down
4 changes: 2 additions & 2 deletions src/components/AddressForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import CountrySelector from './CountrySelector';
import FormProvider from './Form/FormProvider';
import InputWrapper from './Form/InputWrapper';
import type {FormOnyxValues} from './Form/types';
import StatePicker from './StatePicker';
import StateSelector from './StateSelector';
import TextInput from './TextInput';

type CountryZipRegex = {
Expand Down Expand Up @@ -190,7 +190,7 @@ function AddressForm({
{isUSAForm ? (
<View style={styles.mhn5}>
<InputWrapper
InputComponent={StatePicker}
InputComponent={StateSelector}
inputID={INPUT_IDS.STATE}
defaultValue={state}
onValueChange={onAddressChanged}
Expand Down
4 changes: 2 additions & 2 deletions src/components/Form/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type Picker from '@components/Picker';
import type RadioButtons from '@components/RadioButtons';
import type RoomNameInput from '@components/RoomNameInput';
import type SingleChoiceQuestion from '@components/SingleChoiceQuestion';
import type StatePicker from '@components/StatePicker';
import type StateSelector from '@components/StateSelector';
import type TextInput from '@components/TextInput';
import type TextPicker from '@components/TextPicker';
import type ValuePicker from '@components/ValuePicker';
Expand All @@ -40,7 +40,7 @@ type ValidInputs =
| typeof CountrySelector
| typeof AmountForm
| typeof BusinessTypePicker
| typeof StatePicker
| typeof StateSelector
| typeof RoomNameInput
| typeof ValuePicker
| typeof DatePicker
Expand Down
114 changes: 0 additions & 114 deletions src/components/StatePicker/StateSelectorModal.tsx

This file was deleted.

93 changes: 0 additions & 93 deletions src/components/StatePicker/index.tsx

This file was deleted.

103 changes: 103 additions & 0 deletions src/components/StateSelector.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import {useIsFocused} from '@react-navigation/native';
import {CONST as COMMON_CONST} from 'expensify-common/lib/CONST';
import React, {useEffect, useRef} from 'react';
import type {ForwardedRef} from 'react';
import {View} from 'react-native';
import useGeographicalStateFromRoute from '@hooks/useGeographicalStateFromRoute';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import type {MaybePhraseKey} from '@libs/Localize';
import Navigation from '@libs/Navigation/Navigation';
import ROUTES from '@src/ROUTES';
import FormHelpMessage from './FormHelpMessage';
import type {MenuItemProps} from './MenuItem';
import MenuItemWithTopDescription from './MenuItemWithTopDescription';

type State = keyof typeof COMMON_CONST.STATES;

type StateSelectorProps = {
/** Form error text. e.g when no state is selected */
errorText?: MaybePhraseKey;

/** Current selected state */
value?: State | '';

/** Callback to call when the input changes */
onInputChange?: (value: string) => void;

/** Label to display on field */
label?: string;

/** Any additional styles to apply */
wrapperStyle?: MenuItemProps['wrapperStyle'];

/** Callback to call when the picker modal is dismissed */
onBlur?: () => void;

/** object to get route details from */
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB

Suggested change
/** object to get route details from */
/** Object to get route details from */

stateSelectorRoute?: typeof ROUTES.SETTINGS_ADDRESS_STATE | typeof ROUTES.MONEY_REQUEST_STATE_SELECTOR;
};

function StateSelector(
{errorText, onBlur, value: stateCode, label, onInputChange, wrapperStyle, stateSelectorRoute = ROUTES.SETTINGS_ADDRESS_STATE}: StateSelectorProps,
ref: ForwardedRef<View>,
) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const stateFromUrl = useGeographicalStateFromRoute();

const didOpenStateSelector = useRef(false);
const isFocused = useIsFocused();

useEffect(() => {
if (isFocused && didOpenStateSelector.current) {
didOpenStateSelector.current = false;
if (!stateFromUrl) {
onBlur?.();
}
}

if (!stateFromUrl) {
return;
}

// This will cause the form to revalidate and remove any error related to country name
if (onInputChange) {
onInputChange(stateFromUrl);
}

Navigation.setParams({state: undefined});

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [stateFromUrl, onBlur, isFocused]);

const title = stateCode && Object.keys(COMMON_CONST.STATES).includes(stateCode) ? translate(`allStates.${stateCode}.stateName`) : '';
Copy link
Member

@parasharrajat parasharrajat Mar 29, 2024

Choose a reason for hiding this comment

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

I am trying to understand the usages of state from URL stateFromUrl and state as a direct prop stateCode.

Here the title is calculated form the stateCode from prop value. What about the value that is coming from URL? We are dependent on onInputChange to pass back the URL value as prop. What if this component is not used in the Form and we don't have onInputChange handler which updates the prop value. Will this component work?

If this is the case, we are creating an incomplete error-prone component.

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, it won't work if not used within form. But I suppose since it in an input component, it will always be used inside a form. Do you want it to be more independent, generic, futureproof, and have its own state?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently it is not used without a Form and If it is used as a standalone component, it can always be wrapped around a Form like IncorporationStateBusiness. Or the devs can modify the code as per their needs. For current uses and its intended purpose, do you think it still needs a local state?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Current StatePicker too doesnt have local state. Since we're just mimicking its functionality, do you still think we need to worry about local state?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I am fine with its current state and it can be modified later if needed. But let's add explanatory comments to the logic above to explain what's going on in the effect and how this component is working.

Copy link
Contributor Author

@ygshbht ygshbht Mar 29, 2024

Choose a reason for hiding this comment

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

Sure, done.

const descStyle = title.length === 0 ? styles.textNormal : null;

return (
<View>
<MenuItemWithTopDescription
descriptionTextStyle={descStyle}
ref={ref}
shouldShowRightIcon
title={title}
// Label can be an empty string
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
description={label || translate('common.state')}
onPress={() => {
const activeRoute = Navigation.getActiveRoute();
didOpenStateSelector.current = true;
Navigation.navigate(stateSelectorRoute.getRoute(stateCode, activeRoute, label));
}}
wrapperStyle={wrapperStyle}
/>
<View style={styles.ml5}>
<FormHelpMessage message={errorText} />
</View>
</View>
);
}

StateSelector.displayName = 'StateSelector';

export default React.forwardRef(StateSelector);
Loading
Loading