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

Refactor: reuse ValidateCodeActionModal #49445

Merged
merged 33 commits into from
Oct 21, 2024
Merged

Refactor: reuse ValidateCodeActionModal #49445

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
14ffa19
refactor: reuse ValidateCodeActionModal
getusha Sep 19, 2024
8b8046e
Merge remote-tracking branch 'exfy/main' into reuse-validate-code-act…
getusha Sep 24, 2024
c1bac17
fix: match phone number to BE format
getusha Sep 24, 2024
39f7823
fix: navigate only after validation
getusha Sep 24, 2024
b2aab12
fix: several improvements
getusha Sep 24, 2024
af7f72c
fix: lint
getusha Sep 24, 2024
201f533
Merge remote-tracking branch 'exfy/main' into reuse-validate-code-act…
getusha Sep 24, 2024
0ea996a
Merge remote-tracking branch 'exfy/main' into reuse-validate-code-act…
getusha Sep 24, 2024
92a43b5
refactor: create a reusable shared modal for delegate validation
getusha Sep 29, 2024
0c52912
fix: modal closes automatically
getusha Oct 3, 2024
6349f44
fix: run prettier
getusha Oct 3, 2024
592343f
fix: lint
getusha Oct 3, 2024
49b9234
Merge remote-tracking branch 'exfy/main' into reuse-validate-code-act…
getusha Oct 3, 2024
89970ef
fix: lint
getusha Oct 3, 2024
7ee9659
fix: lint
getusha Oct 3, 2024
b5695c4
Merge remote-tracking branch 'exfy/main' into reuse-validate-code-act…
getusha Oct 7, 2024
f52d5a8
fix: run prettier
getusha Oct 7, 2024
6bdd1bd
fix: tscheck
getusha Oct 7, 2024
4d86852
fix: prevent sending a magic code on every rerender
getusha Oct 10, 2024
5301e27
fix: lint
getusha Oct 10, 2024
6ebcad9
Merge remote-tracking branch 'exfy/main' into reuse-validate-code-act…
getusha Oct 14, 2024
1ec0b40
fix: several undetected conflicts
getusha Oct 14, 2024
07ed2f2
fix: remove unneccessary magic code modal
getusha Oct 14, 2024
ac5e8eb
fix: lint
getusha Oct 14, 2024
74cc224
fix: navigation flicker on verified contact
getusha Oct 14, 2024
07b6501
fix: lint
getusha Oct 14, 2024
c8db647
fix: lint
getusha Oct 14, 2024
11741ab
fix: validation code not being sent
getusha Oct 15, 2024
35606ef
fix: navigation issues
getusha Oct 17, 2024
72a8705
fix: lint
getusha Oct 17, 2024
063ff58
fix: navigation issues
getusha Oct 17, 2024
83bce36
Merge remote-tracking branch 'exfy/main' into reuse-validate-code-act…
getusha Oct 17, 2024
30df3bd
Merge remote-tracking branch 'exfy/main' into reuse-validate-code-act…
getusha Oct 18, 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
1 change: 0 additions & 1 deletion src/ROUTES.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ const ROUTES = {
route: 'settings/profile/contact-methods/:contactMethod/details',
getRoute: (contactMethod: string, backTo?: string) => getUrlWithBackToParam(`settings/profile/contact-methods/${encodeURIComponent(contactMethod)}/details`, backTo),
},
SETINGS_CONTACT_METHOD_VALIDATE_ACTION: 'settings/profile/contact-methods/validate-action',
SETTINGS_NEW_CONTACT_METHOD: {
route: 'settings/profile/contact-methods/new',
getRoute: (backTo?: string) => getUrlWithBackToParam('settings/profile/contact-methods/new', backTo),
Expand Down
1 change: 0 additions & 1 deletion src/SCREENS.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ const SCREENS = {
DISPLAY_NAME: 'Settings_Display_Name',
CONTACT_METHODS: 'Settings_ContactMethods',
CONTACT_METHOD_DETAILS: 'Settings_ContactMethodDetails',
CONTACT_METHOD_VALIDATE_ACTION: 'Settings_ValidateContactMethodAction',
NEW_CONTACT_METHOD: 'Settings_NewContactMethod',
STATUS_CLEAR_AFTER: 'Settings_Status_Clear_After',
STATUS_CLEAR_AFTER_DATE: 'Settings_Status_Clear_After_Date',
Expand Down
15 changes: 5 additions & 10 deletions src/components/ValidateCodeActionModal/ValidateCodeForm/BaseValidateCodeForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ type ValidateCodeFormProps = {

/** Function to clear error of the form */
clearError: () => void;

sendValidateCode: () => void;
Copy link
Contributor

Choose a reason for hiding this comment

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

Docs

};

type BaseValidateCodeFormProps = BaseValidateCodeFormOnyxProps & ValidateCodeFormProps;
Expand All @@ -78,6 +80,7 @@ function BaseValidateCodeForm({
validateError,
handleSubmitForm,
clearError,
sendValidateCode,
}: BaseValidateCodeFormProps) {
const {translate} = useLocalize();
const {isOffline} = useNetwork();
Expand Down Expand Up @@ -128,14 +131,6 @@ function BaseValidateCodeForm({
}, []),
);

useEffect(() => {
if (!validateError) {
return;
}
clearError();
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, [clearError, validateError]);

useEffect(() => {
if (!hasMagicCodeBeenSent) {
return;
Expand All @@ -147,7 +142,7 @@ function BaseValidateCodeForm({
* Request a validate code / magic code be sent to verify this contact method
*/
const resendValidateCode = () => {
User.requestValidateCodeAction();
sendValidateCode();
inputValidateCodeRef.current?.clear();
};

Expand Down Expand Up @@ -196,7 +191,7 @@ function BaseValidateCodeForm({
errorText={formError?.validateCode ? translate(formError?.validateCode) : ErrorUtils.getLatestErrorMessage(account ?? {})}
hasError={!isEmptyObject(validateError)}
onFulfill={validateAndSubmitForm}
autoFocus={false}
autoFocus
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding autofocus here caused the keyboard not to open on android (#51279), we fixed this in #51453.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you. We already discussed it during PR phase, but then forgot to update it here.

/>
<OfflineWithFeedback
pendingAction={validateCodeAction?.pendingFields?.validateCodeSent}
Expand Down
23 changes: 19 additions & 4 deletions src/components/ValidateCodeActionModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,25 @@ import Modal from '@components/Modal';
import ScreenWrapper from '@components/ScreenWrapper';
import Text from '@components/Text';
import useThemeStyles from '@hooks/useThemeStyles';
import * as User from '@libs/actions/User';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {ValidateCodeActionModalProps} from './type';
import ValidateCodeForm from './ValidateCodeForm';
import type {ValidateCodeFormHandle} from './ValidateCodeForm/BaseValidateCodeForm';

function ValidateCodeActionModal({isVisible, title, description, onClose, validatePendingAction, validateError, handleSubmitForm, clearError}: ValidateCodeActionModalProps) {
function ValidateCodeActionModal({
isVisible,
title,
description,
onClose,
validatePendingAction,
validateError,
handleSubmitForm,
clearError,
footer,
sendValidateCode,
hasMagicCodeBeenSent,
}: ValidateCodeActionModalProps) {
const themeStyles = useThemeStyles();
const firstRenderRef = useRef(true);
const validateCodeFormRef = useRef<ValidateCodeFormHandle>(null);
Expand All @@ -30,8 +41,9 @@ function ValidateCodeActionModal({isVisible, title, description, onClose, valida
return;
}
firstRenderRef.current = false;
User.requestValidateCodeAction();
}, [isVisible]);

sendValidateCode();
}, [isVisible, sendValidateCode]);

return (
<Modal
Expand Down Expand Up @@ -61,10 +73,13 @@ function ValidateCodeActionModal({isVisible, title, description, onClose, valida
validatePendingAction={validatePendingAction}
validateError={validateError}
handleSubmitForm={handleSubmitForm}
sendValidateCode={sendValidateCode}
Copy link
Contributor

@youssef-lr youssef-lr Oct 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is there any reason why we made this a prop instead of implementing it in this 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.

sendValidateCode is passed to be used only for requesting a new magic code in BaseValidateCodeForm

App/src/components/ValidateCodeActionModal/ValidateCodeForm/BaseValidateCodeForm.tsx

Lines 141 to 143 in 44c9260

const resendValidateCode = () => {
sendValidateCode();
inputValidateCodeRef.current?.clear();

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, so why isn't the implementation of sendValidateCode is not in ValidateCodeActionModal here

App/src/components/ValidateCodeActionModal/index.tsx

Line 48 in d774a7f

sendValidateCode();
, why do we need to supply it from parent components given that it's always going to be the same?

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically this code is repeated in every component that uses this modal

    const sendValidateCode = () => {
        if (loginData?.validateCodeSent) {
            return;
        }

        User.requestValidateCodeAction();
    };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two functions to request a magic code requestContactMethodValidateCode & requestValidateCodeAction

And there may be conditions before calling the function, which means if we want to implement sendValidateCode in the modal we'll still need to supply loginData and contact method with a boolean prop for which function to use.

App/src/pages/settings/Wallet/ExpensifyCardPage.tsx

Lines 150 to 151 in 9508b17

const primaryLogin = account?.primaryLogin ?? '';
const loginData = loginList?.[primaryLogin];

I thought this would be the most convenient way to make the component flexible.

clearError={clearError}
ref={validateCodeFormRef}
hasMagicCodeBeenSent={hasMagicCodeBeenSent}
/>
</View>
{footer?.()}
</ScreenWrapper>
</Modal>
);
Expand Down
10 changes: 10 additions & 0 deletions src/components/ValidateCodeActionModal/type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type React from 'react';
import type {Errors, PendingAction} from '@src/types/onyx/OnyxCommon';

type ValidateCodeActionModalProps = {
Expand All @@ -24,6 +25,15 @@ type ValidateCodeActionModalProps = {

/** Function to clear error of the form */
clearError: () => void;

/** A component to be rendered inside the modal */
footer?: () => React.JSX.Element;

/** Function is called when validate code modal is mounted and on magic code resend */
sendValidateCode: () => void;

/** If the magic code has been resent previously */
hasMagicCodeBeenSent?: boolean;
};

// eslint-disable-next-line import/prefer-default-export
Expand Down
1 change: 0 additions & 1 deletion src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ const SettingsModalStackNavigator = createModalStackNavigator<SettingsNavigatorP
[SCREENS.SETTINGS.PROFILE.ADDRESS_STATE]: () => require<ReactComponentModule>('../../../../pages/settings/Profile/PersonalDetails/StateSelectionPage').default,
[SCREENS.SETTINGS.PROFILE.CONTACT_METHODS]: () => require<ReactComponentModule>('../../../../pages/settings/Profile/Contacts/ContactMethodsPage').default,
[SCREENS.SETTINGS.PROFILE.CONTACT_METHOD_DETAILS]: () => require<ReactComponentModule>('../../../../pages/settings/Profile/Contacts/ContactMethodDetailsPage').default,
[SCREENS.SETTINGS.PROFILE.CONTACT_METHOD_VALIDATE_ACTION]: () => require<ReactComponentModule>('../../../../pages/settings/Profile/Contacts/ValidateContactActionPage').default,
[SCREENS.SETTINGS.PROFILE.NEW_CONTACT_METHOD]: () => require<ReactComponentModule>('../../../../pages/settings/Profile/Contacts/NewContactMethodPage').default,
[SCREENS.SETTINGS.PREFERENCES.PRIORITY_MODE]: () => require<ReactComponentModule>('../../../../pages/settings/Preferences/PriorityModePage').default,
[SCREENS.WORKSPACE.ACCOUNTING.ROOT]: () => require<ReactComponentModule>('../../../../pages/workspace/accounting/PolicyAccountingPage').default,
Expand Down
1 change: 0 additions & 1 deletion src/libs/Navigation/linkingConfig/CENTRAL_PANE_TO_RHP_MAPPING.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const CENTRAL_PANE_TO_RHP_MAPPING: Partial<Record<CentralPaneName, string[]>> =
SCREENS.SETTINGS.PROFILE.DISPLAY_NAME,
SCREENS.SETTINGS.PROFILE.CONTACT_METHODS,
SCREENS.SETTINGS.PROFILE.CONTACT_METHOD_DETAILS,
SCREENS.SETTINGS.PROFILE.CONTACT_METHOD_VALIDATE_ACTION,
SCREENS.SETTINGS.PROFILE.NEW_CONTACT_METHOD,
SCREENS.SETTINGS.PROFILE.STATUS_CLEAR_AFTER,
SCREENS.SETTINGS.PROFILE.STATUS_CLEAR_AFTER_DATE,
Expand Down
3 changes: 0 additions & 3 deletions src/libs/Navigation/linkingConfig/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,6 @@ const config: LinkingOptions<RootStackParamList>['config'] = {
[SCREENS.SETTINGS.PROFILE.CONTACT_METHOD_DETAILS]: {
path: ROUTES.SETTINGS_CONTACT_METHOD_DETAILS.route,
},
[SCREENS.SETTINGS.PROFILE.CONTACT_METHOD_VALIDATE_ACTION]: {
path: ROUTES.SETINGS_CONTACT_METHOD_VALIDATE_ACTION,
},
[SCREENS.SETTINGS.PROFILE.NEW_CONTACT_METHOD]: {
path: ROUTES.SETTINGS_NEW_CONTACT_METHOD.route,
exact: true,
Expand Down
9 changes: 4 additions & 5 deletions src/libs/actions/Delegate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,6 @@ function clearDelegatorErrors() {
Onyx.merge(ONYXKEYS.ACCOUNT, {delegatedAccess: {delegators: delegatedAccess.delegators.map((delegator) => ({...delegator, errorFields: undefined}))}});
}

function requestValidationCode() {
API.write(WRITE_COMMANDS.RESEND_VALIDATE_CODE, null);
}

function addDelegate(email: string, role: DelegateRole, validateCode: string) {
const existingDelegate = delegatedAccess?.delegates?.find((delegate) => delegate.email === email);

Expand Down Expand Up @@ -206,6 +202,7 @@ function addDelegate(email: string, role: DelegateRole, validateCode: string) {
delegatedAccess: {
delegates: optimisticDelegateData(),
},
isLoading: true,
},
},
];
Expand Down Expand Up @@ -250,6 +247,7 @@ function addDelegate(email: string, role: DelegateRole, validateCode: string) {
delegatedAccess: {
delegates: successDelegateData(),
},
isLoading: false,
},
},
];
Expand Down Expand Up @@ -292,6 +290,7 @@ function addDelegate(email: string, role: DelegateRole, validateCode: string) {
delegatedAccess: {
delegates: failureDelegateData(),
},
isLoading: false,
},
},
];
Expand Down Expand Up @@ -325,4 +324,4 @@ function removePendingDelegate(email: string) {
});
}

export {connect, disconnect, clearDelegatorErrors, addDelegate, requestValidationCode, clearAddDelegateErrors, removePendingDelegate};
export {connect, disconnect, clearDelegatorErrors, addDelegate, clearAddDelegateErrors, removePendingDelegate};
7 changes: 2 additions & 5 deletions src/libs/actions/User.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,7 @@ function resetContactMethodValidateCodeSentState(contactMethod: string) {
* Clears unvalidated new contact method action
*/
function clearUnvalidatedNewContactMethodAction() {
Onyx.merge(ONYXKEYS.PENDING_CONTACT_ACTION, {
validateCodeSent: null,
pendingFields: null,
errorFields: null,
});
Onyx.merge(ONYXKEYS.PENDING_CONTACT_ACTION, null);
}

/**
Expand Down Expand Up @@ -447,6 +443,7 @@ function addNewContactMethod(contactMethod: string, validateCode = '') {
key: ONYXKEYS.PENDING_CONTACT_ACTION,
value: {
validateCodeSent: null,
actionVerified: true,
errorFields: {
actionVerified: null,
},
Expand Down
Loading
Loading