Follow-up PR: Private Domain Onboarding Check

[allgandalf]

Explanation of Change

Fixed Issues

$ #54075
PROPOSAL: N/A

Tests

Same as QA

• Verify that no errors appear in the JS console

Offline tests

QA Steps

Test 1:

1. Signup with a new gmail account

Verify that the text [receipts@expensify.com](mailto:receipts@expensify.com) is shown in bold

Test 2:

1. Signup with a new work email which has not been logged in before.
2. Complete onboarding and logout without validating that account.
3. Go to signup screen again, enter a new public email (@gmail.com) and signup
4. On the onboarding work email screen enter the work email you entered before.

Verify that we see a error above the skip button.

• 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 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 used JaimeGPT to get English > Spanish translation. I then posted it 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.ts 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.
• I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
• 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: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

MacOS: Desktop

[melvin-bot]

@ikevin127 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]

[allgandalf]

@ikevin127 this is a follow up PR for the following issues discussed on slack:

When I try to add a work email for an existing account that has 2FA the AddWorkEmail command returns an error but the error isn't displayed in the modal. Link

Looking great! The link should be bold too no? And the top green progress stepper should be on 0%? Link

[allgandalf]

Since the changes don't affect native files, here's the video for desktop, lemme know if you would require videos on all platforms:

Screen.Recording.2025-04-22.at.4.12.32.PM.mov

[ikevin127]

Will start review shortly.

@blimpich Will this follow-up be paid separately, or as part of the main PR which means $250 total for all the work including this follow-up ?

Just wondering since it's been quite some work, which makes a total of $250 seem low for the amount of work.

[ikevin127]

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 that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
• 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 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.ts 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.
• For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
• 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

android.mp4

Android: mWeb Chrome

android-mweb.mp4

iOS: Native

ios.mp4

iOS: mWeb Safari

ios-mweb.mp4

MacOS: Chrome / Safari

web.mov

MacOS: Desktop

desktop.mov

[ikevin127]

🟢 LGTM tests wise, everything works as expected in both tests. The ony thing would be a UI concern regarding the error from test 2 being dismissable (Close):

@Expensify/design I don't remember seeing a lot of dismissable errors in form-setting before, which made this one jump out to me so I just wanted to make sure it LGTY, before moving forward with merge 👍

[dubielzyk-expensify]

We have it in certain areas, but I don't think this error should be dismissible 👍

Also, why is there a giant gap at the top here. I think it should have the stepper bar or if not, then we'd just reduce that gap:

[ikevin127]

Good catch, I think because the PR set progressBarPercentage={0} which is being evaluated here and the if block is skipped because:

In JavaScript, the conditional if (progressBarPercentage) checks whether the value is truthy.

Here’s what happens when you pass:
• progressBarPercentage = 1 (or any non-zero number):
• It is truthy, so the condition evaluates to true, and the code inside the if block will execute.
• progressBarPercentage = 0:
• It is falsy, so the condition evaluates to false, and the code inside the if block will not execute.

cc @allgandalf

It's weird that the prop is typed as number:

    /** 0 - 100 number indicating current progress of the progress bar */
    progressBarPercentage?: number;

Maybe we should change to string and convert it back to number (or not) when we're pasaing the style in HeaderWithBackButton 🤔

@blimpich Please hold on merging until the issues are addressed and I tag you again after retest.

[blimpich]

Sounds good. I've re-requested your review @ikevin127. I'll look again once you've re-approved it.

[allgandalf]

Crap, i missed the ping from kevin, will fix this tomorrow , @dubielzyk-expensify what should the progress bar look like, can you share a mock please

[allgandalf]

bump @dubielzyk-expensify

[dannymcclain]

#not-jon but I think it should look something like this (from Figma):

This is the same progress stepper bar we use in all the onboarding flows AFAIK.

[allgandalf]

No i meant to ask what should be the progress there 😅,the green bar how much should that be filled

[dannymcclain]

No i meant to ask what should be the progress there 😅,the green bar how much should that be filled

🤣 Oh my bad! I'm not totally sure, so I will just see myself out... 😂

[ikevin127]

@allgandalf It was mentioned here that it should be 0%, the only issue was the logic mentioned here wasn't rendering the bar at all.

Given that the Private Domain Onboarding can have a maximum 2 steps (if not skipped), would show 0% on step 1 (email input), and 1 (100%) on the magic code email verification step.

But if I really think about it, keeping the progress bar on the Private Domain Onboarding seems redundant since it can have a maximum of 2 steps (if not skipped) - in contrast to the Onboarding Purpose flow which can have more.

☝️For this reason, I'd vote to simply remove the progress bar from the Private Domain Onboarding flow. Thoughts ?

[blimpich]

Given that the Private Domain Onboarding can have a maximum 2 steps (if not skipped), would show 0% on step 1 (email input), and 1 (100%) on the magic code email verification step.

Isn't the progress bar meant to indicate progress through the entire onboarding modal? Not just the private domain check portion of it?

[allgandalf]

yeah agree with @blimpich , waiting on @dubielzyk-expensify for the mock

[dubielzyk-expensify]

Isn't the progress bar meant to indicate progress through the entire onboarding modal? Not just the private domain check portion of it?

Correct. If there are 5 steps, the first step should be 1/5 filled. If there's 6 steps, step 3 should be 50% filled etc etc.

So just I guess percentageFilled = (current step / totalSteps) * 100 or whatever the math is 😅 #barelyPassedMathClass

[blimpich]

@allgandalf are we making progress here?

[allgandalf]

Correct. If there are 5 steps, the first step should be 1/5 filled. If there's 6 steps, step 3 should be 50% filled etc etc.

The problem is, we don't know the number of steps.

Suppose user signups using public domain, then they are shown work email screen, then there would be in total 5 steps ideally, but if the user enters a work email and it needs verification then there would be 6 steps (+ 1 for the email validation), so how should we proceed here? @blimpich @dubielzyk-expensify ?

[blimpich]

I'd say we just take a guess and assume the maximum number of steps. That way it might not jump evenly in the progress bar but it'll always increase. I.e. if we assume max steps and there are actually fewer steps the progress bar will still fill up and maybe make a big jump, but it we assume the smallest number of steps then we risk having the progress bar stagnate or even decrease even though the user is moving forward in the modal. Does that make sense?

[dubielzyk-expensify]

Yeah I think we should use what information we currently have. That's why we designed the progress indicator as a bar instead of a individual segmented stepper.

Basically if you're on step 3 and it's default is to be a total of 5 steps, then let's do 3/5ths. If that increases to 7 after step 3 is taken, then lets do 4/7ths.

It doesn't have to be incredibly accurate to future steps, mostly just a rough indication of how far you are in the process

[allgandalf]

Checking this one now

[allgandalf]

@ikevin127 can you test again

[ikevin127]

@allgandalf I confirm that the progress bar is visible now, but the 1/6 (0.1666666...) value seems low to me given how the UI looks, here's a comparison between signing up with private domain vs public domain (which prompts private domain onboarding):

Private Domain Email (✅)	Public Domain Email (⚠️)
private-signup.mov	public-signup.mp4

Looking at existing progressBarPercentage throughout the code (see screenshot below) we can notice that the range goes from 20 (min) to 90 (max), so for our case I think the best option would be to set 10 or 15 given that the Private Domain Onboarding is the first in the flow (when its showing up).

⚠️ With the current progressBarPercentage values, for example BaseOnboardingWorkEmailValidation.tsx having 40, after magic code is input, there will be a weird jump in progress bar from 40 to 20 (Onboarding Purpose What do you want to do today? page) which looks weird from a UI perspective since at Private Domain Onboarding magic code validation page you're already 40% ahead then you jump back to 20% (see Public Domain Email (⚠️) video above), even though you're supposed to progress linearly.

🔄 My proposal would be to set progressBarPercentage to 10 for BaseOnboardingWorkEmail.tsx and change from 40 to 15 for BaseOnboardingWorkEmailValidation.tsx for the best results, regardless of whether the Private Domain Onboarding is skipped, here's a look at the UI with this proposal:

final.mov

cc @Expensify/design for validation

[shawnborton]

I haven't been following along closely, but is there no way to know the full number of steps for the onboarding modal before we kick it off?

[dubielzyk-expensify]

@allgandalf I confirm that the progress bar is visible now, but the 1/6 (0.1666666...) value seems low to me given how the UI looks, here's a comparison between signing up with private domain vs public domain (which prompts private domain onboarding):

Agree. At 16% that should be bigger than what it is to my eyes.

[ikevin127]

@dubielzyk-expensify 1/6 is not translated to 16% but 0.16% in this case. That's why I suggested natural number values in #60632 (comment) for best UI.

[dubielzyk-expensify]

Sure. I don't mind what the technical solution is as long as the right progress value is represented 👍

[allgandalf]

@ikevin127 Updated following your suggestions !

[ikevin127]

@ikevin127 Updated following your suggestions !

LGTM, PR tests well and progress bar is now linear regardless of onboarding flow (doesn't jump back) ✅

[blimpich]

Double checking that we tested hybrid app here before merging

[OSBotify]

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

[ikevin127]

@blimpich Are there any issues with this PR which prevents it being deployed or we just didn't deploy in a while ?

[github-actions]

🚀 Deployed to staging by https://github.com/blimpich in version: 9.1.40-0 🚀

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

[blimpich]

@blimpich Are there any issues with this PR which prevents it being deployed or we just didn't deploy in a while ?

Just hadn't deployed in awhile 🙂

[blimpich]

QA-ed this internally, looks good

[github-actions]

🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.1.40-7 🚀

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