Skip to content

migrate AccountType step to FormProvider #56464

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

Merged
merged 13 commits into from
Mar 20, 2025
Merged

migrate AccountType step to FormProvider #56464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
01c4db1
migrate AccountType step to FormProvider
mkzie2 Feb 6, 2025
8de5987
Merge branch 'main' into mkzie2-issue/55688
mkzie2 Feb 16, 2025
3340c4e
remove the extra step for account type step
mkzie2 Feb 16, 2025
99e2c6d
Merge branch 'main' into mkzie2-issue/55688
mkzie2 Feb 18, 2025
8d64c55
update focused key
mkzie2 Feb 18, 2025
669b4d1
Merge branch 'main' into mkzie2-issue/55688
mkzie2 Feb 20, 2025
3239142
Merge branch 'main' into mkzie2-issue/55688
mkzie2 Feb 21, 2025
59863a2
merge main
mkzie2 Mar 11, 2025
72ab9e2
Merge branch 'main' into mkzie2-issue/55688
mkzie2 Mar 15, 2025
9ca4629
remove confirm button when editing the account type step
mkzie2 Mar 15, 2025
d8a5519
Merge branch 'mkzie2-issue/55688' of https://github.com/mkzie2/App in…
mkzie2 Mar 20, 2025
9c50026
add return type
mkzie2 Mar 20, 2025
dbcfd16
Merge branch 'main' of https://github.com/mkzie2/App into mkzie2-issu…
mkzie2 Mar 20, 2025
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
27 changes: 27 additions & 0 deletions src/components/ValuePicker/ValueSelectionList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import React, {useMemo} from 'react';
import SelectionList from '@components/SelectionList';
import RadioListItem from '@components/SelectionList/RadioListItem';
import type {ValueSelectionListProps} from './types';

function ValueSelectionList({items = [], selectedItem, onItemSelected, shouldShowTooltips = true}: ValueSelectionListProps) {
const sections = useMemo(
() => [{data: items.map((item) => ({value: item.value, alternateText: item.description, text: item.label ?? '', isSelected: item === selectedItem, keyForList: item.value ?? ''}))}],
[items, selectedItem],
);

return (
<SelectionList
Copy link
Contributor Author

@mkzie2 mkzie2 Feb 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@cead22 Thanks for your feedback, let me clarify it again.

There's no other way to do this? We have to ValuePicker as the InputComponent to make this work with FormProvider behave and work like TypeBusiness?

ValuePicker is an existing component; we can reuse this in the AccountType step when migrating to FormProvider.

As opposed to clicking on the menu item, and then having to click a button to confirm, right?
What is "menu item" and what is "modal" here?

Yes, here is the result of my first implementation

Screen.Recording.2025-02-11.at.15.57.27.mov
  • And this is the menu item
Screenshot 2025-02-21 at 14 23 52
  • This is the modal
Screenshot 2025-02-21 at 14 24 39

The main idea of the last implementation is creating a way to only show the selection list without the extra step of opening the modal. Then I implement it in ValuePicker with a new prop so we can use this in the feature if we want. And the result will be like this

Screenshot 2025-02-21 at 14 34 56

Why can't we use SelectionList in ValuePicker?

As the idea above, we only need to reuse the SelectionList part on ValueSelectorModal and render it in ValuePicker if shouldShowModal props is false

You can see here ValueSelectionList used the SelectionList. The purpose of this component is prevent the duplicate code when I add both SelectionList in ValuePicker and ValueSelectorModal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cead22 Any thoughts on my comment above.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the delay, it's been a busy few days.

There's no other way to do this? We have to ValuePicker as the InputComponent to make this work with FormProvider behave and work like TypeBusiness?
ValuePicker is an existing component; we can reuse this in the AccountType step when migrating to FormProvider.

To clarify, are there alternative ways to implement this? If so, can you share why they're not as good as this one? If there aren't, can you explain why?

Sorry if this is all known to others, but I'm not super well versed on all the components that exists and how they work together

The main idea of the last implementation is creating a way to only show the selection list without the extra step of opening the modal.

Just to make sure I understand, the selection list looks very similar to the modal, and the main difference is that the modal has a green button you need to click after making the selection?

Then I implement it in ValuePicker with a new prop so we can use this in the feature if we want. And the result will be like this

Again just so I learn, if we didn't want to do this in anticipation of using it in the future, how would the code be different? Not saying we should change it, I just want to understand exactly what's needed for now, vs what we're doing to set this up to be re-used

Why can't we use SelectionList in ValuePicker?
As the idea above, we only need to reuse the SelectionList part on ValueSelectorModal and render it in ValuePicker if shouldShowModal props is false

I didn't quite follow this. Can we use SelectionList in ValuePicker? Does that question make sense?

The purpose of this component is prevent the duplicate code when I add both SelectionList in ValuePicker and ValueSelectorModal.

👍 thanks, I get this now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cead22

To clarify, are there alternative ways to implement this? If so, can you share why they're not as good as this one? If there aren't, can you explain why?
Sorry if this is all known to others, but I'm not super well versed on all the components that exists and how they work together

We can create a new component and use it as InputWrapper of the FormProvider in the AccountType step. But we're good to use the ValuePicker because we can see the SelectionList inside AccountType (the code on the latest main) is the same with the selection list inside ValueSelectorModal of ValuePicker

App/src/pages/settings/Wallet/InternationalDepositAccount/substeps/AccountType.tsx

Lines 81 to 90 in c23850d

<SelectionList
sections={[{data: options}]}
onSelectRow={onSelectionChange}
ListItem={RadioListItem}
initiallyFocusedOptionKey={currentAccountType}
footerContent={button}
shouldSingleExecuteRowSelect
shouldStopPropagation
shouldUseDynamicMaxToRenderPerBatch
shouldUpdateFocusedIndex

The main idea of the last implementation is creating a way to only show the selection list without the extra step of opening the modal.

Just to make sure I understand, the selection list looks very similar to the modal, and the main difference is that the modal has a green button you need to click after making the selection?

The main idea is to keep the current UI and behavior in the AccountType step, but using FormProvier with ValuePicker instead of the current code here. And then when using the ValuePicker, we need a way only to display the SelectionList, then the behavior is the same as we have on the main, making a selection and clicking on the confirm button to go to the next step.

App/src/pages/settings/Wallet/InternationalDepositAccount/substeps/AccountType.tsx

Lines 81 to 90 in c23850d

<SelectionList
sections={[{data: options}]}
onSelectRow={onSelectionChange}
ListItem={RadioListItem}
initiallyFocusedOptionKey={currentAccountType}
footerContent={button}
shouldSingleExecuteRowSelect
shouldStopPropagation
shouldUseDynamicMaxToRenderPerBatch
shouldUpdateFocusedIndex

Again just so I learn, if we didn't want to do this in anticipation of using it in the future, how would the code be different? Not saying we should change it, I just want to understand exactly what's needed for now, vs what we're doing to set this up to be re-used

In the feature, if we have a substep with a single field and we need to make a selection, we can reuse the ValuePicker with shouldShowModal as false to remove the extra step.

  • what's needed for now: Now we need an InputWrapper component that is only a selection list like the current selection in here. And because ValuePicker already had the same list like this, we can use this component with introducing a new prop to simply return the selection list if this prop is false

App/src/pages/settings/Wallet/InternationalDepositAccount/substeps/AccountType.tsx

Lines 81 to 90 in c23850d

<SelectionList
sections={[{data: options}]}
onSelectRow={onSelectionChange}
ListItem={RadioListItem}
initiallyFocusedOptionKey={currentAccountType}
footerContent={button}
shouldSingleExecuteRowSelect
shouldStopPropagation
shouldUseDynamicMaxToRenderPerBatch
shouldUpdateFocusedIndex

  • what we're doing to set this up to be re-used: I set this up to be re-used by introducing the shouldShowModal. If we have a substep that has the same UI and behavior like this AccountType step, we can re-use like this.

Copy link
Contributor

Choose a reason for hiding this comment

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

To clarify, are there alternative ways to implement this? If so, can you share why they're not as good as this one? If there aren't, can you explain why?
We can create a new component and use it as InputWrapper of the FormProvider in the AccountType step. But we're good to use the ValuePicker because we can see the SelectionList inside AccountType (the code on the latest main) is the same with the selection list inside ValueSelectorModal of ValuePicker

Sorry, I still don't understand this. When I asked about whether there's another way, I meant with existing logic/components. I don't think this addresses why the current solution is better (apologies if it does)

The main idea is to keep the current UI and behavior in the AccountType step, but using FormProvier with ValuePicker instead of the current code here. And then when using the ValuePicker, we need a way only to display the SelectionList, then the behavior is the same as we have on the main, making a selection and clicking on the confirm button to go to the next step.

Sorry about this one too, but I'm not sure if/how this answers my question. What does "the extra step of opening the modal" mean in "The main idea of the last implementation is creating a way to only show the selection list without the extra step of opening the modal."?

  • what we're doing to set this up to be re-used: I set this up to be re-used by introducing the shouldShowModal. If we have a substep that has the same UI and behavior like this AccountType step, we can re-use like this.

Thanks for this explanation. Is this something we anticipate using right away, or should we add it when we need it?

I'm gonna find a better reviewer for this PR. I don't want to keep delaying it, but I also don't want to merge code I don't understand

sections={sections}
onSelectRow={(item) => onItemSelected?.(item)}
initiallyFocusedOptionKey={selectedItem?.value}
shouldStopPropagation
shouldShowTooltips={shouldShowTooltips}
shouldUpdateFocusedIndex
ListItem={RadioListItem}
/>
);
}

ValueSelectionList.displayName = 'ValueSelectionList';

export default ValueSelectionList;
20 changes: 6 additions & 14 deletions src/components/ValuePicker/ValueSelectorModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import React, {useMemo} from 'react';
import React from 'react';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import Modal from '@components/Modal';
import ScreenWrapper from '@components/ScreenWrapper';
import SelectionList from '@components/SelectionList';
import RadioListItem from '@components/SelectionList/RadioListItem';
import useThemeStyles from '@hooks/useThemeStyles';
import CONST from '@src/CONST';
import type {ValueSelectorModalProps} from './types';
import ValueSelectionList from './ValueSelectionList';

function ValueSelectorModal({
items = [],
Expand All @@ -21,11 +20,6 @@ function ValueSelectorModal({
}: ValueSelectorModalProps) {
const styles = useThemeStyles();

const sections = useMemo(
() => [{data: items.map((item) => ({value: item.value, alternateText: item.description, text: item.label ?? '', isSelected: item === selectedItem, keyForList: item.value ?? ''}))}],
[items, selectedItem],
);

return (
<Modal
type={CONST.MODAL.MODAL_TYPE.RIGHT_DOCKED}
Expand All @@ -47,13 +41,11 @@ function ValueSelectorModal({
title={label}
onBackButtonPress={onClose}
/>
<SelectionList
sections={sections}
onSelectRow={(item) => onItemSelected?.(item)}
initiallyFocusedOptionKey={selectedItem?.value}
shouldStopPropagation
<ValueSelectionList
items={items}
selectedItem={selectedItem}
onItemSelected={onItemSelected}
shouldShowTooltips={shouldShowTooltips}
ListItem={RadioListItem}
/>
</ScreenWrapper>
</Modal>
Expand Down
61 changes: 38 additions & 23 deletions src/components/ValuePicker/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@ import MenuItemWithTopDescription from '@components/MenuItemWithTopDescription';
import Navigation from '@libs/Navigation/Navigation';
import CONST from '@src/CONST';
import type {ValuePickerItem, ValuePickerProps} from './types';
import ValueSelectionList from './ValueSelectionList';
import ValueSelectorModal from './ValueSelectorModal';

function ValuePicker({value, label, items, placeholder = '', errorText = '', onInputChange, furtherDetails, shouldShowTooltips = true}: ValuePickerProps, forwardedRef: ForwardedRef<View>) {
function ValuePicker(
{value, label, items, placeholder = '', errorText = '', onInputChange, furtherDetails, shouldShowTooltips = true, shouldShowModal = true}: ValuePickerProps,
forwardedRef: ForwardedRef<View>,
) {
const [isPickerVisible, setIsPickerVisible] = useState(false);

const showPickerModal = () => {
Expand All @@ -29,28 +33,39 @@ function ValuePicker({value, label, items, placeholder = '', errorText = '', onI

return (
<View>
<MenuItemWithTopDescription
ref={forwardedRef}
shouldShowRightIcon
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
title={selectedItem?.label || placeholder || ''}
description={label}
onPress={showPickerModal}
furtherDetails={furtherDetails}
brickRoadIndicator={errorText ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
errorText={errorText}
/>
<ValueSelectorModal
isVisible={isPickerVisible}
label={label}
selectedItem={selectedItem}
items={items}
onClose={hidePickerModal}
onItemSelected={updateInput}
shouldShowTooltips={shouldShowTooltips}
onBackdropPress={Navigation.dismissModal}
shouldEnableKeyboardAvoidingView={false}
/>
{shouldShowModal ? (
<>
<MenuItemWithTopDescription
ref={forwardedRef}
shouldShowRightIcon
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
title={selectedItem?.label || placeholder || ''}
description={label}
onPress={showPickerModal}
furtherDetails={furtherDetails}
brickRoadIndicator={errorText ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined}
errorText={errorText}
/>
<ValueSelectorModal
isVisible={isPickerVisible}
label={label}
selectedItem={selectedItem}
items={items}
onClose={hidePickerModal}
onItemSelected={updateInput}
shouldShowTooltips={shouldShowTooltips}
onBackdropPress={Navigation.dismissModal}
shouldEnableKeyboardAvoidingView={false}
/>
</>
) : (
<ValueSelectionList
items={items}
selectedItem={selectedItem}
onItemSelected={updateInput}
shouldShowTooltips={shouldShowTooltips}
/>
)}
</View>
);
}
Expand Down
7 changes: 6 additions & 1 deletion src/components/ValuePicker/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ type ValueSelectorModalProps = {
shouldEnableKeyboardAvoidingView?: boolean;
};

type ValueSelectionListProps = Pick<ValueSelectorModalProps, 'items' | 'selectedItem' | 'onItemSelected' | 'shouldShowTooltips'>;

type ValuePickerProps = {
/** Item to display */
value?: string;
Expand All @@ -63,6 +65,9 @@ type ValuePickerProps = {

/** Whether to show the tooltip text */
shouldShowTooltips?: boolean;

/** Whether to show the selector modal */
shouldShowModal?: boolean;
};

export type {ValuePickerItem, ValueSelectorModalProps, ValuePickerProps, ValuePickerListItem};
export type {ValuePickerItem, ValueSelectorModalProps, ValuePickerProps, ValuePickerListItem, ValueSelectionListProps};
4 changes: 2 additions & 2 deletions src/libs/actions/FormActions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ function clearErrorFields(formID: OnyxFormKey) {
Onyx.merge(formID, {errorFields: null});
}

function setDraftValues(formID: OnyxFormKey, draftValues: NullishDeep<OnyxValue<OnyxFormDraftKey>>) {
Onyx.merge(`${formID}Draft`, draftValues ?? null);
function setDraftValues(formID: OnyxFormKey, draftValues: NullishDeep<OnyxValue<OnyxFormDraftKey>>): Promise<void> {
return Onyx.merge(`${formID}Draft`, draftValues ?? null);
}

function clearDraftValues(formID: OnyxFormKey) {
Expand Down
109 changes: 49 additions & 60 deletions src/pages/settings/Wallet/InternationalDepositAccount/substeps/AccountType.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,95 +1,84 @@
import React, {useCallback, useMemo, useState} from 'react';
import React, {useCallback, useMemo, useRef} from 'react';
import {View} from 'react-native';
import FormAlertWithSubmitButton from '@components/FormAlertWithSubmitButton';
import SelectionList from '@components/SelectionList';
import RadioListItem from '@components/SelectionList/RadioListItem';
import FormProvider from '@components/Form/FormProvider';
import InputWrapper from '@components/Form/InputWrapper';
import type {FormInputErrors, FormOnyxValues, FormRef} from '@components/Form/types';
import ValuePicker from '@components/ValuePicker';
import useInternationalBankAccountFormSubmit from '@hooks/useInternationalBankAccountFormSubmit';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import type {Option} from '@libs/searchOptions';
import {setDraftValues} from '@libs/actions/FormActions';
import type {CustomSubStepProps} from '@pages/settings/Wallet/InternationalDepositAccount/types';
import {setDraftValues} from '@userActions/FormActions';
import Text from '@src/components/Text';
import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';

function AccountType({isEditing, onNext, formValues, fieldsMap}: CustomSubStepProps) {
function AccountType({isEditing, onNext, fieldsMap}: CustomSubStepProps) {
const {translate} = useLocalize();
const styles = useThemeStyles();
const [currentAccountType, setCurrentAccountType] = useState(formValues[CONST.CORPAY_FIELDS.ACCOUNT_TYPE_KEY]);
const [error, setError] = useState<TranslationPaths | undefined>(undefined);
const formRef = useRef<FormRef>(null);

const fieldData = fieldsMap[CONST.CORPAY_FIELDS.STEPS_NAME.ACCOUNT_TYPE]?.[CONST.CORPAY_FIELDS.ACCOUNT_TYPE_KEY] ?? {};

const onAccountTypeSelected = useCallback(() => {
setError(undefined);
if (isEditing && formValues[CONST.CORPAY_FIELDS.ACCOUNT_TYPE_KEY] === currentAccountType) {
onNext();
return;
}
if (fieldData.isRequired && !currentAccountType) {
setError('common.error.pleaseSelectOne');
return;
}
setDraftValues(ONYXKEYS.FORMS.INTERNATIONAL_BANK_ACCOUNT_FORM, {[CONST.CORPAY_FIELDS.ACCOUNT_TYPE_KEY]: currentAccountType});
onNext();
}, [currentAccountType, fieldData.isRequired, formValues, isEditing, onNext]);
const handleSubmit = useInternationalBankAccountFormSubmit({
fieldIds: Object.keys(fieldsMap[CONST.CORPAY_FIELDS.STEPS_NAME.ACCOUNT_TYPE]),
onNext,
shouldSaveDraft: isEditing,
});

const onSelectionChange = useCallback(
(country: Option) => {
if (!isEditing) {
setDraftValues(ONYXKEYS.FORMS.INTERNATIONAL_BANK_ACCOUNT_FORM, {[CONST.CORPAY_FIELDS.ACCOUNT_TYPE_KEY]: country.value});
const validate = useCallback(
(values: FormOnyxValues<typeof ONYXKEYS.FORMS.INTERNATIONAL_BANK_ACCOUNT_FORM>): FormInputErrors<typeof ONYXKEYS.FORMS.INTERNATIONAL_BANK_ACCOUNT_FORM> => {
if (!fieldData.isRequired || values[CONST.CORPAY_FIELDS.ACCOUNT_TYPE_KEY]) {
return {};
}
setCurrentAccountType(country.value);
return {[CONST.CORPAY_FIELDS.ACCOUNT_TYPE_KEY]: translate('common.error.pleaseSelectOne')};
},
[isEditing],
[fieldData.isRequired, translate],
);

const options = useMemo(
() =>
(fieldData.valueSet ?? []).map((item) => {
return {
value: item.id,
keyForList: item.id,
text: item.text,
isSelected: currentAccountType === item.id,
searchValue: item.text,
label: item.text,
};
}),
[fieldData.valueSet, currentAccountType],
[fieldData.valueSet],
);

const button = useMemo(() => {
const buttonText = isEditing ? translate('common.confirm') : translate('common.next');
return (
<FormAlertWithSubmitButton
message={error ? translate(error) : ''}
isAlertVisible={!!error}
buttonText={buttonText}
onSubmit={onAccountTypeSelected}
containerStyles={[styles.flexReset, styles.flexGrow0, styles.flexShrink0, styles.flexBasisAuto]}
enabledWhenOffline
/>
);
}, [error, isEditing, onAccountTypeSelected, styles.flexBasisAuto, styles.flexGrow0, styles.flexReset, styles.flexShrink0, translate]);

return (
<>
<FormProvider
formID={ONYXKEYS.FORMS.INTERNATIONAL_BANK_ACCOUNT_FORM}
submitButtonText={translate('common.confirm')}
onSubmit={handleSubmit}
validate={validate}
style={[styles.flexGrow1, styles.mt3]}
submitButtonStyles={[styles.ph5, styles.mb0]}
enabledWhenOffline
ref={formRef}
isSubmitButtonVisible={!isEditing}
>
<View style={styles.ph5}>
<Text style={[styles.textHeadlineLineHeightXXL, styles.mb6]}>{translate('addPersonalBankAccount.accountTypeStepHeader')}</Text>
</View>
<SelectionList
sections={[{data: options}]}
onSelectRow={onSelectionChange}
ListItem={RadioListItem}
initiallyFocusedOptionKey={currentAccountType}
footerContent={button}
shouldSingleExecuteRowSelect
shouldStopPropagation
shouldUseDynamicMaxToRenderPerBatch
shouldUpdateFocusedIndex
<InputWrapper
InputComponent={ValuePicker}
inputID={fieldData.id}
label={fieldData.label}
items={options}
shouldSaveDraft={!isEditing}
shouldShowModal={false}
onValueChange={(value) => {
if (!isEditing) {
return;
}
setDraftValues(ONYXKEYS.FORMS.INTERNATIONAL_BANK_ACCOUNT_FORM, {[CONST.CORPAY_FIELDS.ACCOUNT_TYPE_KEY]: value}).then(() => {
onNext();
});
}}
/>
</>
</FormProvider>
);
}

Expand Down