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: Room - Sign in RHP disappears after clicking on the '<' button. #46760

Merged
merged 10 commits into from
Aug 16, 2024

Conversation

Krishna2323
Copy link
Contributor

@Krishna2323 Krishna2323 commented Aug 3, 2024

Details

Fixed Issues

$ #46106
PROPOSAL: #46106 (comment)

Tests

  1. Open Chrome in incognito
  2. Navigate to https://staging.new.expensify.com/r/2091104345528462
  3. Click on the "Sign in" button at the bottom right corner
  4. Enter a existing email in Expensify and click on the "Continue" button
  5. Click on the "<" button
  6. Verify you are taken back to first step
  7. Input a new email and click on the "Continue" button
  8. Click on the "<" button
  9. Verify you are taken back to first step
  10. Enter 123@gmail.com and continue
  11. Click on the "<" button
  12. Verify you are taken back to first step
  13. Click on the "<" button
  14. Verify sign-in modal closes
  • Verify that no errors appear in the JS console

Offline tests

  1. Open Chrome in incognito
  2. Navigate to https://staging.new.expensify.com/r/2091104345528462
  3. Click on the "Sign in" button at the bottom right corner
  4. Enter a existing email in Expensify and click on the "Continue" button
  5. Click on the "<" button
  6. Verify you are taken back to first step
  7. Input a new email and click on the "Continue" button
  8. Click on the "<" button
  9. Verify you are taken back to first step
  10. Enter 123@gmail.com and continue
  11. Click on the "<" button
  12. Verify you are taken back to first step
  13. Click on the "<" button
  14. Verify sign-in modal closes

QA Steps

  1. Open Chrome in incognito
  2. Navigate to https://staging.new.expensify.com/r/2091104345528462
  3. Click on the "Sign in" button at the bottom right corner
  4. Enter a existing email in Expensify and click on the "Continue" button
  5. Click on the "<" button
  6. Verify you are taken back to first step
  7. Input a new email and click on the "Continue" button
  8. Click on the "<" button
  9. Verify you are taken back to first step
  10. Enter 123@gmail.com and continue
  11. Click on the "<" button
  12. Verify you are taken back to first step
  13. Click on the "<" button
  14. Verify sign-in modal closes
  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
    • MacOS: Desktop
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick)
    • I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
      • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
  • I verified that if a function's arguments changed that all usages have also been updated correctly
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
android_native.mp4
Android: mWeb Chrome
android_chrome.mp4
iOS: Native
ios_native.mp4
iOS: mWeb Safari
ios_safari.mp4
MacOS: Chrome / Safari
web_chrome.mp4
MacOS: Desktop

Signed-off-by: krishna2323 <belivethatkg@gmail.com>
@Krishna2323 Krishna2323 requested a review from a team as a code owner August 3, 2024 11:17
@melvin-bot melvin-bot bot removed the request for review from a team August 3, 2024 11:17
Copy link

melvin-bot bot commented Aug 3, 2024

@ishpaul777 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot melvin-bot bot requested a review from ishpaul777 August 3, 2024 11:17
@Krishna2323
Copy link
Contributor Author

@ishpaul777, do you know how we can open the public report in the native and desktop apps?

@ishpaul777
Copy link
Contributor

ishpaul777 commented Aug 3, 2024

For native:
https://expensify.enterprise.slack.com/archives/C01GTK53T8Q/p1698849973538749?thread_ts=1698848483.535709&cid=C01GTK53T8Q

for desktop, make sure desktop app is running and then on browser visit new-expensify://{route you want to visiit}

@Krishna2323
Copy link
Contributor Author

@ishpaul777, can you please share the steps from slack, I'm not in the channel.

@ishpaul777
Copy link
Contributor

You can use npx uri-scheme open , e.g.

npx uri-scheme open new-expensify://r/123456789 --android

npx uri-scheme open new-expensify://r/123456789 --ios

@luacmartins luacmartins self-requested a review August 5, 2024 07:45
@ishpaul777
Copy link
Contributor

ishpaul777 commented Aug 5, 2024

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified tests pass on all platforms & I tested again on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
    • MacOS: Desktop
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
    • I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
    • I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
    • I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
    • I verified the JSDocs style guidelines (in STYLE.md) were followed
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
  • I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: Native
Screen.Recording.2024-08-15.at.12.41.51.AM.mov
Android: mWeb Chrome
Screen.Recording.2024-08-15.at.12.29.27.AM.mov
iOS: Native
Screen.Recording.2024-08-13.at.2.03.08.AM.mov
iOS: mWeb Safari
Screen.Recording.2024-08-15.at.12.19.44.AM.mov
MacOS: Chrome / Safari
Screen.Recording.2024-08-13.at.12.27.46.AM.mov
MacOS: Desktop

Comment on lines 187 to 189
useEffect(() => {
setClearSignInData(clearSignInData);
}, [clearSignInData, setClearSignInData]);
Copy link
Contributor

Choose a reason for hiding this comment

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

@Krishna2323
Copy link
Contributor Author

I will update today, I have to resolve one more issue, the back button has extra top margin on desktop app because we are using ScreenWrapper inside ScreenWrapper for sign in modal.

@Krishna2323
Copy link
Contributor Author

@ishpaul777, what do you think about remove the ScreenWrapper from SignInModal and accept new prop in SignInPage for overwriting props of ScreenWrapper?.

@ishpaul777
Copy link
Contributor

ishpaul777 commented Aug 7, 2024

how about keeping the HeaderWithBackButton in SigninModal as it is now and using a navigateBack function as ref in SigninModal using useImperativeHandle ?

@Krishna2323
Copy link
Contributor Author

Sounds good to me. Will update accordingly.

@ishpaul777
Copy link
Contributor

gentle bump @Krishna2323

@Krishna2323
Copy link
Contributor Author

@ishpaul777, I was trying to update the test the code but for some reason I can't login on desktop app. I will try to fix the issue and push the code changes.

Monosnap.screencast.2024-08-09.16-05-33.mp4

@ishpaul777
Copy link
Contributor

Can you try clearing onyx data and then try again

@Krishna2323
Copy link
Contributor Author

@ishpaul777, thanks, it worked. I'm struggling a bit to implement useImperativeHandle in SignInPage component. I have not use useImperativeHandle yet and in SignInPage component we need to pass the ref to SignInPageThemeWrapper and then to SignInPage. I will update as soon as I get it working. Here is the code I implemented if you want to take a look.

Code
import {Str} from 'expensify-common';
import React, {forwardRef, useEffect, useImperativeHandle, useRef, useState} from 'react';
import {ForwardedRef} from 'react';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import ColorSchemeWrapper from '@components/ColorSchemeWrapper';
import CustomStatusBarAndBackground from '@components/CustomStatusBarAndBackground';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import ScreenWrapper from '@components/ScreenWrapper';
import ThemeProvider from '@components/ThemeProvider';
import ThemeStylesProvider from '@components/ThemeStylesProvider';
import useLocalize from '@hooks/useLocalize';
import useResponsiveLayout from '@hooks/useResponsiveLayout';
import useSafeAreaInsets from '@hooks/useSafeAreaInsets';
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import * as ActiveClientManager from '@libs/ActiveClientManager';
import * as Localize from '@libs/Localize';
import Log from '@libs/Log';
import Navigation from '@libs/Navigation/Navigation';
import Performance from '@libs/Performance';
import Visibility from '@libs/Visibility';
import * as App from '@userActions/App';
import * as Session from '@userActions/Session';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Account, Credentials, Locale} from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import ChooseSSOOrMagicCode from './ChooseSSOOrMagicCode';
import EmailDeliveryFailurePage from './EmailDeliveryFailurePage';
import LoginForm from './LoginForm';
import type {InputHandle} from './LoginForm/types';
import SignInPageLayout from './SignInPageLayout';
import type {SignInPageLayoutRef} from './SignInPageLayout/types';
import SignUpWelcomeForm from './SignUpWelcomeForm';
import UnlinkLoginForm from './UnlinkLoginForm';
import ValidateCodeForm from './ValidateCodeForm';

type SignInPageInnerOnyxProps = {
    /** The details about the account that the user is signing in with */
    account: OnyxEntry<Account>;

    /** The credentials of the person signing in */
    credentials: OnyxEntry<Credentials>;

    /** Active Clients connected to ONYX Database */
    activeClients: OnyxEntry<string[]>;

    /** The user's preferred locale */
    preferredLocale: OnyxEntry<Locale>;
};

type SignInPageInnerProps = SignInPageInnerOnyxProps & {
    shouldEnableMaxHeight?: boolean;
    shouldShowBackButton?: boolean;
};

type SignInPageRef = {
    navigateBack: () => void;
};

type RenderOption = {
    shouldShowLoginForm: boolean;
    shouldShowEmailDeliveryFailurePage: boolean;
    shouldShowUnlinkLoginForm: boolean;
    shouldShowValidateCodeForm: boolean;
    shouldShowChooseSSOOrMagicCode: boolean;
    shouldInitiateSAMLLogin: boolean;
    shouldShowWelcomeHeader: boolean;
    shouldShowWelcomeText: boolean;
    shouldShouldSignUpWelcomeForm: boolean;
};

type GetRenderOptionsParams = {
    hasLogin: boolean;
    hasValidateCode: boolean;
    account: OnyxEntry<Account>;
    isPrimaryLogin: boolean;
    isUsingMagicCode: boolean;
    hasInitiatedSAMLLogin: boolean;
    shouldShowAnotherLoginPageOpenedMessage: boolean;
    credentials: OnyxEntry<Credentials>;
};

/**
 * @param hasLogin
 * @param hasValidateCode
 * @param account
 * @param isPrimaryLogin
 * @param isUsingMagicCode
 * @param hasInitiatedSAMLLogin
 * @param hasEmailDeliveryFailure
 */
function getRenderOptions({
    hasLogin,
    hasValidateCode,
    account,
    isPrimaryLogin,
    isUsingMagicCode,
    hasInitiatedSAMLLogin,
    shouldShowAnotherLoginPageOpenedMessage,
    credentials,
}: GetRenderOptionsParams): RenderOption {
    const hasAccount = !isEmptyObject(account);
    const isSAMLEnabled = !!account?.isSAMLEnabled;
    const isSAMLRequired = !!account?.isSAMLRequired;
    const hasEmailDeliveryFailure = !!account?.hasEmailDeliveryFailure;

    // True, if the user has SAML required, and we haven't yet initiated SAML for their account
    const shouldInitiateSAMLLogin = hasAccount && hasLogin && isSAMLRequired && !hasInitiatedSAMLLogin && !!account.isLoading;
    const shouldShowChooseSSOOrMagicCode = hasAccount && hasLogin && isSAMLEnabled && !isSAMLRequired && !isUsingMagicCode;

    // SAML required users may reload the login page after having already entered their login details, in which
    // case we want to clear their sign in data so they don't end up in an infinite loop redirecting back to their
    // SSO provider's login page
    if (hasLogin && isSAMLRequired && !shouldInitiateSAMLLogin && !hasInitiatedSAMLLogin && !account.isLoading) {
        Session.clearSignInData();
    }

    // Show the Welcome form if a user is signing up for a new account in a domain that is not controlled
    const shouldShouldSignUpWelcomeForm = !!credentials?.login && !account?.validated && !account?.accountExists && !account?.domainControlled;
    const shouldShowLoginForm = !shouldShowAnotherLoginPageOpenedMessage && !hasLogin && !hasValidateCode;
    const shouldShowEmailDeliveryFailurePage = hasLogin && hasEmailDeliveryFailure && !shouldShowChooseSSOOrMagicCode && !shouldInitiateSAMLLogin;
    const isUnvalidatedSecondaryLogin = hasLogin && !isPrimaryLogin && !account?.validated && !hasEmailDeliveryFailure;
    const shouldShowValidateCodeForm =
        !shouldShouldSignUpWelcomeForm &&
        hasAccount &&
        (hasLogin || hasValidateCode) &&
        !isUnvalidatedSecondaryLogin &&
        !hasEmailDeliveryFailure &&
        !shouldShowChooseSSOOrMagicCode &&
        !isSAMLRequired;
    const shouldShowWelcomeHeader = shouldShowLoginForm || shouldShowValidateCodeForm || shouldShowChooseSSOOrMagicCode || isUnvalidatedSecondaryLogin || shouldShouldSignUpWelcomeForm;
    const shouldShowWelcomeText =
        shouldShowLoginForm || shouldShowValidateCodeForm || shouldShowChooseSSOOrMagicCode || shouldShowAnotherLoginPageOpenedMessage || shouldShouldSignUpWelcomeForm;

    return {
        shouldShowLoginForm,
        shouldShowEmailDeliveryFailurePage,
        shouldShowUnlinkLoginForm: !shouldShouldSignUpWelcomeForm && isUnvalidatedSecondaryLogin,
        shouldShowValidateCodeForm,
        shouldShowChooseSSOOrMagicCode,
        shouldInitiateSAMLLogin,
        shouldShowWelcomeHeader,
        shouldShowWelcomeText,
        shouldShouldSignUpWelcomeForm,
    };
}

const SignInPage = forwardRef(function SignInPage(
    {credentials, account, activeClients = [], preferredLocale, shouldEnableMaxHeight = true, shouldShowBackButton}: SignInPageInnerProps,
    ref: ForwardedRef<SignInPageRef>,
) {
    const styles = useThemeStyles();
    const StyleUtils = useStyleUtils();
    const {translate, formatPhoneNumber} = useLocalize();
    const {shouldUseNarrowLayout, isInNarrowPaneModal} = useResponsiveLayout();
    const safeAreaInsets = useSafeAreaInsets();
    const signInPageLayoutRef = useRef<SignInPageLayoutRef>(null);
    const loginFormRef = useRef<InputHandle>(null);
    const clearValidateCodeFormDataRef = useRef<() => void>(null);
    /** This state is needed to keep track of if user is using recovery code instead of 2fa code,
     * and we need it here since welcome text(`welcomeText`) also depends on it */
    const [isUsingRecoveryCode, setIsUsingRecoveryCode] = useState(false);

    /** This state is needed to keep track of whether the user has opted to use magic codes
     * instead of signing in via SAML when SAML is enabled and not required */
    const [isUsingMagicCode, setIsUsingMagicCode] = useState(false);

    /** This state is needed to keep track of whether the user has been directed to their SSO provider's login page and
     *  if we need to clear their sign in details so they can enter a login */
    const [hasInitiatedSAMLLogin, setHasInitiatedSAMLLogin] = useState(false);

    const [login, setLogin] = useState(() => Str.removeSMSDomain(credentials?.login ?? ''));

    const isClientTheLeader = !!activeClients && ActiveClientManager.isClientTheLeader();
    // We need to show "Another login page is opened" message if the page isn't active and visible
    // eslint-disable-next-line rulesdir/no-negated-variables
    const shouldShowAnotherLoginPageOpenedMessage = Visibility.isVisible() && !isClientTheLeader;

    useEffect(() => Performance.measureTTI(), []);
    useEffect(() => {
        if (preferredLocale) {
            return;
        }
        App.setLocale(Localize.getDevicePreferredLocale());
    }, [preferredLocale]);
    useEffect(() => {
        if (credentials?.login) {
            return;
        }

        // If we don't have a login set, reset the user's SAML login preferences
        if (isUsingMagicCode) {
            setIsUsingMagicCode(false);
        }
        if (hasInitiatedSAMLLogin) {
            setHasInitiatedSAMLLogin(false);
        }
    }, [credentials?.login, isUsingMagicCode, setIsUsingMagicCode, hasInitiatedSAMLLogin, setHasInitiatedSAMLLogin]);

    const {
        shouldShowLoginForm,
        shouldShowEmailDeliveryFailurePage,
        shouldShowUnlinkLoginForm,
        shouldShowValidateCodeForm,
        shouldShowChooseSSOOrMagicCode,
        shouldInitiateSAMLLogin,
        shouldShowWelcomeHeader,
        shouldShowWelcomeText,
        shouldShouldSignUpWelcomeForm,
    } = getRenderOptions({
        hasLogin: !!credentials?.login,
        hasValidateCode: !!credentials?.validateCode,
        account,
        isPrimaryLogin: !account?.primaryLogin || account.primaryLogin === credentials?.login,
        isUsingMagicCode,
        hasInitiatedSAMLLogin,
        shouldShowAnotherLoginPageOpenedMessage,
        credentials,
    });

    if (shouldInitiateSAMLLogin) {
        setHasInitiatedSAMLLogin(true);
        Navigation.isNavigationReady().then(() => Navigation.navigate(ROUTES.SAML_SIGN_IN));
    }

    let welcomeHeader = '';
    let welcomeText = '';
    const headerText = translate('login.hero.header');

    const userLogin = Str.removeSMSDomain(credentials?.login ?? '');

    // replacing spaces with "hard spaces" to prevent breaking the number
    const userLoginToDisplay = Str.isSMSLogin(userLogin) ? formatPhoneNumber(userLogin) : userLogin;

    if (shouldShowAnotherLoginPageOpenedMessage) {
        welcomeHeader = translate('welcomeText.anotherLoginPageIsOpen');
        welcomeText = translate('welcomeText.anotherLoginPageIsOpenExplanation');
    } else if (shouldShowLoginForm) {
        welcomeHeader = shouldUseNarrowLayout ? headerText : translate('welcomeText.getStarted');
        welcomeText = shouldUseNarrowLayout ? translate('welcomeText.getStarted') : '';
    } else if (shouldShowValidateCodeForm) {
        if (account?.requiresTwoFactorAuth) {
            // We will only know this after a user signs in successfully, without their 2FA code
            welcomeHeader = shouldUseNarrowLayout ? '' : translate('welcomeText.welcome');
            welcomeText = isUsingRecoveryCode ? translate('validateCodeForm.enterRecoveryCode') : translate('validateCodeForm.enterAuthenticatorCode');
        } else {
            welcomeHeader = shouldUseNarrowLayout ? '' : translate('welcomeText.welcome');
            welcomeText = shouldUseNarrowLayout
                ? `${translate('welcomeText.welcome')} ${translate('welcomeText.welcomeEnterMagicCode', {login: userLoginToDisplay})}`
                : translate('welcomeText.welcomeEnterMagicCode', {login: userLoginToDisplay});
        }
    } else if (shouldShowUnlinkLoginForm || shouldShowEmailDeliveryFailurePage || shouldShowChooseSSOOrMagicCode) {
        welcomeHeader = shouldUseNarrowLayout ? headerText : translate('welcomeText.welcome');

        // Don't show any welcome text if we're showing the user the email delivery failed view
        if (shouldShowEmailDeliveryFailurePage || shouldShowChooseSSOOrMagicCode) {
            welcomeText = '';
        }
    } else if (shouldShouldSignUpWelcomeForm) {
        welcomeHeader = shouldUseNarrowLayout ? headerText : translate('welcomeText.welcome');
        welcomeText = shouldUseNarrowLayout
            ? `${translate('welcomeText.welcomeWithoutExclamation')} ${translate('welcomeText.welcomeNewFace', {login: userLoginToDisplay})}`
            : translate('welcomeText.welcomeNewFace', {login: userLoginToDisplay});
    } else if (!shouldInitiateSAMLLogin && !hasInitiatedSAMLLogin) {
        Log.warn('SignInPage in unexpected state!');
    }

    const navigateFocus = () => {
        signInPageLayoutRef.current?.scrollPageToTop();
        loginFormRef.current?.clearDataAndFocus();
    };

    const navigateBack = () => {
        if (
            shouldShouldSignUpWelcomeForm ||
            (!shouldShowAnotherLoginPageOpenedMessage && (shouldShowEmailDeliveryFailurePage || shouldShowUnlinkLoginForm || shouldShowChooseSSOOrMagicCode))
        ) {
            Session.clearSignInData();
            return;
        }

        if (shouldShowValidateCodeForm) {
            clearValidateCodeFormDataRef?.current?.();
            return;
        }

        Navigation.goBack();
    };
    useImperativeHandle(ref, () => ({
        navigateBack,
    }));
    return (
        // Bottom SafeAreaView is removed so that login screen svg displays correctly on mobile.
        // The SVG should flow under the Home Indicator on iOS.
        <ScreenWrapper
            shouldShowOfflineIndicator={false}
            shouldEnableMaxHeight={shouldEnableMaxHeight}
            shouldUseCachedViewportHeight
            style={[styles.signInPage, StyleUtils.getSafeAreaPadding({...safeAreaInsets, bottom: 0, top: isInNarrowPaneModal ? 0 : safeAreaInsets.top}, 1)]}
            testID={SignInPageThemeWrapper.displayName}
        >
            {shouldShowBackButton && <HeaderWithBackButton onBackButtonPress={navigateBack} />}
            <SignInPageLayout
                welcomeHeader={welcomeHeader}
                welcomeText={welcomeText}
                shouldShowWelcomeHeader={shouldShowWelcomeHeader || !shouldUseNarrowLayout}
                shouldShowWelcomeText={shouldShowWelcomeText}
                ref={signInPageLayoutRef}
                navigateFocus={navigateFocus}
            >
                {/* LoginForm must use the isVisible prop. This keeps it mounted, but visually hidden
             so that password managers can access the values. Conditionally rendering this component will break this feature. */}
                <LoginForm
                    ref={loginFormRef}
                    isVisible={shouldShowLoginForm}
                    login={login}
                    onLoginChanged={setLogin}
                    blurOnSubmit={account?.validated === false}
                    scrollPageToTop={signInPageLayoutRef.current?.scrollPageToTop}
                />
                {shouldShouldSignUpWelcomeForm && <SignUpWelcomeForm />}
                {shouldShowValidateCodeForm && (
                    <ValidateCodeForm
                        isVisible={!shouldShowAnotherLoginPageOpenedMessage}
                        isUsingRecoveryCode={isUsingRecoveryCode}
                        setIsUsingRecoveryCode={setIsUsingRecoveryCode}
                        setClearSignInData={(clearSignInData: () => void) => {
                            (clearValidateCodeFormDataRef.current as (() => void) | null) = clearSignInData;
                        }}
                    />
                )}
                {!shouldShowAnotherLoginPageOpenedMessage && (
                    <>
                        {shouldShowUnlinkLoginForm && <UnlinkLoginForm />}
                        {shouldShowChooseSSOOrMagicCode && <ChooseSSOOrMagicCode setIsUsingMagicCode={setIsUsingMagicCode} />}
                        {shouldShowEmailDeliveryFailurePage && <EmailDeliveryFailurePage />}
                    </>
                )}
            </SignInPageLayout>
        </ScreenWrapper>
    );
});

type SignInPageProps = SignInPageInnerProps;
type SignInPageOnyxProps = SignInPageInnerOnyxProps;

function SignInPageThemeWrapper(props: SignInPageProps, ref: ForwardedRef<SignInPageRef>) {
    return (
        <ThemeProvider theme={CONST.THEME.DARK}>
            <ThemeStylesProvider>
                <ColorSchemeWrapper>
                    <CustomStatusBarAndBackground isNested />
                    <SignInPage
                        ref={ref}
                        // eslint-disable-next-line react/jsx-props-no-spreading
                        {...props}
                    />
                </ColorSchemeWrapper>
            </ThemeStylesProvider>
        </ThemeProvider>
    );
}

SignInPageThemeWrapper.displayName = 'SignInPage';

export default withOnyx<SignInPageProps, SignInPageOnyxProps>({
    account: {key: ONYXKEYS.ACCOUNT},
    credentials: {key: ONYXKEYS.CREDENTIALS},
    /**
      This variable is only added to make sure the component is re-rendered
      whenever the activeClients change, so that we call the
      ActiveClientManager.isClientTheLeader function
      everytime the leader client changes.
      We use that function to prevent repeating code that checks which client is the leader.
    */
    activeClients: {key: ONYXKEYS.ACTIVE_CLIENTS},
    preferredLocale: {
        key: ONYXKEYS.NVP_PREFERRED_LOCALE,
    },
})(forwardRef(SignInPageThemeWrapper));

@Krishna2323 Krishna2323 force-pushed the krishna2323/issue/46106 branch from e6fb313 to febd1ef Compare August 11, 2024 12:33
Signed-off-by: krishna2323 <belivethatkg@gmail.com>
@Krishna2323
Copy link
Contributor Author

@ishpaul777, I have implemented useImperativeHandle in both components, and it works well on all platforms, likely including the desktop. The header button spacing should also be fixed because I added the HeaderWithBackbutton in SignInModal. All recordings have been added except for the Desktop App. I can't open the room on the desktop app now. Please let me know if you have any ideas about that. I've tried after clearing all the data. Code changes are ready for review.

Monosnap.screencast.2024-08-11.19-02-20.mp4

Comment on lines 80 to 86
onPress={() => {
if (Navigation.isActiveRoute(ROUTES.SIGN_IN_MODAL)) {
Session.clearSignInData();
return;
}
redirectToSignIn();
}}
Copy link
Contributor

Choose a reason for hiding this comment

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

we can just call Session.clearSignInData() like we do at other place, i dont think we ever need redirectToSignIn and clear all onyx data 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

redirectToSignIn() is being used till now so I only added the change for the modal, can you pls look at #46760 (comment)? after that I will check this out and will remove redirectToSignIn() if needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

gentle bump here @Krishna2323

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will finish this today. Thanks for the ping

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ishpaul777, code updated.

@ishpaul777
Copy link
Contributor

i am also not able to sign in as anonymous user. There is no support for that on desktop App.

App/src/libs/actions/App.ts

Lines 451 to 455 in 6839fcb

function beginDeepLinkRedirect(shouldAuthenticateWithCurrentAccount = true) {
// There's no support for anonymous users on desktop
if (Session.isAnonymousUser()) {
return;
}

@@ -272,6 +302,7 @@ function SignInPage({credentials, account, activeClients = [], preferredLocale,
style={[styles.signInPage, StyleUtils.getSafeAreaPadding({...safeAreaInsets, bottom: 0, top: isInNarrowPaneModal ? 0 : safeAreaInsets.top}, 1)]}
testID={SignInPageThemeWrapper.displayName}
>
{/* {shouldShowBackButton && <HeaderWithBackButton onBackButtonPress={navigateBack} />} */}
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry for that. Removed.

Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Copy link
Contributor

@ishpaul777 ishpaul777 left a comment

Choose a reason for hiding this comment

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

Only a NAB comment Rest LGTM!

@@ -141,14 +147,19 @@ function getRenderOptions({
};
}

function SignInPage({credentials, account, activeClients = [], preferredLocale, shouldEnableMaxHeight = true}: SignInPageInnerProps) {
const SignInPage = forwardRef(function SignInPage(
Copy link
Contributor

Choose a reason for hiding this comment

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

NAB but it would be nice if we can create new variable SignInPageWithRef= forwardRef(SignInPage) to clean this up

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do that cc @Krishna2323

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will be on it in few moments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Signed-off-by: krishna2323 <belivethatkg@gmail.com>
Copy link
Contributor

@luacmartins luacmartins left a comment

Choose a reason for hiding this comment

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

LGTM

@luacmartins luacmartins merged commit 279f561 into Expensify:main Aug 16, 2024
17 checks passed
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by https://github.com/luacmartins in version: 9.0.22-0 🚀

platform result
🤖 android 🤖 failure ❌
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by https://github.com/luacmartins in version: 9.0.22-1 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/chiragsalian in version: 9.0.22-9 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants