Onboarding fixes #40688

rezkiy37 · 2024-04-22T12:13:00Z

Details

The PR fixes and improves the onboarding flow. The main changes:

Fixed modal background of the pages. It is translucent for web and desktop right now.
Fixed replies count.
Use bold text following the design.
Integrated failureData for the CompleteGuidedSetup command.
Updated a translation.

Fixed Issues

$ #38771
$ #40725
PROPOSAL: N/A

Tests

Note: To test with an existing user, you can initiate the flow from Settings > Troubleshoot > Onboarding Flow.

With the creation of a workspace (Track business expenses..., Manage my team’s...)

Open the app with an entire new user.
Verify that the "What do you want to do today?" modal appears.
Select any tutorial.
Click on "Continue".
Verify that the "What’s your name?" modal appears.
Enter any names.
Click on "Continue".
Verify that the "Where do you work?" modal appears.
Enter any name.
Click on "Continue".
Verify that the app redirects to the home page.
Open the Concierge chat.
Verify that the app has generated the proper messages.
Verify that the app has created a workspace with the previously entered name.
Verify that attachment messages are replaced with videos, once the backend sends an update.

Without the creation of a workspace (others)

Open the app with an entire new user.
Verify that the "What do you want to do today?" modal appears.
Select any tutorial.
Click on "Continue".
Verify that the "What’s your name?" modal appears.
Enter any names.
Click on "Continue".
Verify that the app redirects to the home page.
Open the Concierge chat.
Verify that the app has generated the proper messages.
Verify that attachment messages are replaced with videos, once the backend sends an update.

Fixes:

Fixed modal background of the pages. It is translucent for web and desktop right now.
a. Verify that the background behind modal screens is translucent.
Fixed replies count.
a. Verify that the app optimistically shows 1 replay count for the "Meet you specialist" task of the "Manage my team's..." purpose.
Use bold text following the design.
a. Verify that the app optimistically generates bold texts as required: [Onboarding - Stage 2] Onboarding Flow - enabled #39687 (comment), [Onboarding - Stage 2] Onboarding Flow - enabled #39687 (comment).
Integrated failureData for the CompleteGuidedSetup command.
a. Enable the "Simulate failing network requests" setting.
b. Verify that the app sets errors for messages in the Concierge tasks. Also, the app deletes task reports and resets the onboarding choice.
Updated a translation.
a. Verify that content is translated properly on the onboarding pages.

Verify that no errors appear in the JS console

Offline tests

Same as "Tests".

QA Steps

Same as "Tests".

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

Android: mWeb Chrome

Android.Chrome.mp4

iOS: Native

IOS.mp4

iOS: mWeb Safari

IOS.Safari.mp4

MacOS: Chrome / Safari

Translations

Integrated `failureData` for the CompleteGuidedSetup command.

Bold texts

MacOS: Desktop

Desktop.mp4

…/onboarding-fixes

melvin-bot · 2024-04-23T14:44:02Z

@rayane-djouah @mountiny One of you needs to 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]

rushatgabhane · 2024-04-23T19:29:29Z

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-04-24.at.16.37.31.mov

Android: mWeb Chrome

WhatsApp.Video.2024-04-24.at.01.09.01.mp4

iOS: Native

Screen.Recording.2024-04-24.at.16.36.24.mov

iOS: mWeb Safari

MacOS: Chrome / Safari

Screen.Recording.2024-04-24.at.00.58.00.mov

Screen.Recording.2024-04-24.at.00.56.21.mov

MacOS: Desktop

rushatgabhane

Bug: App crash when proceeding with "Something else" in onboarding flow

Start onboarding flow from troubleshoot menu
Select something else
Add name, go next
App crash

Screen.Recording.2024-04-24.at.01.16.03.mov

@rezkiy37 are you able to reproduce this?

rushatgabhane · 2024-04-23T19:52:10Z

it's crashing for all flows actually.

Screen.Recording.2024-04-24.at.01.20.47.mov

trjExpensify · 2024-04-23T20:56:01Z

Is it just on iOS native, @rushatgabhane?

mountiny · 2024-04-23T22:53:23Z

@rushatgabhane do you have any errors in console in the mweb flow?

trjExpensify · 2024-04-24T01:02:02Z

@rezkiy37 is this PR supposed to include the fix for the error when completing a task from the DM chat before clicking into the taskReport? CC: @thienlnam

thienlnam · 2024-04-24T01:07:11Z

I forgot where we landed on that - but yes let me know if there's an additional change that is needed to be made on my end

trjExpensify · 2024-04-24T01:14:10Z

Pretty sure this is where we landed, to share the task with the participant of the Concierge/SystemDM.

It's just a little unclear if Michael is actioning on that here (for me at least!).

trjExpensify · 2024-04-24T01:33:24Z

👋 @rezkiy37 can you make sure to update this as well, to the suggested copy by @dubielzyk-expensify here. Thanks!

rushatgabhane · 2024-04-24T05:21:06Z

yes this is iOS native, and i don't see any errors

…/onboarding-fixes

github-actions · 2024-04-24T09:21:53Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.amazonaws.com/android/40688/index.html	https://ad-hoc-expensify-cash.s3.amazonaws.com/ios/40688/index.html

Desktop 💻	Web 🕸️
https://ad-hoc-expensify-cash.s3.amazonaws.com/desktop/40688/NewExpensify.dmg	https://40688.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

src/languages/es.ts

src/languages/en.ts

mountiny

other than the translations, changes look good to me

src/styles/variables.ts

rushatgabhane

Should the popover be over a blank screen? It should be over the home screen, right?

Screen.Recording.2024-04-24.at.16.38.57.mov

trjExpensify · 2024-04-24T11:18:29Z

I tested this case. The backend does not accept completing by clicking on task reports. The app passed all IDs correctly. I can complete tasks, once I open task reports. I am not sure that it is a frontend bug. The backend successfully opens task reports, but not complete. Is it possible that the backend does not see task reports until I open it?

On this, @thienlnam's explanation in thread is that until you open a task, you aren't granted ‘read, write, share’ permissions. So if you try to complete the task from the "preview" in the parent chat before you've opened the taskReport up, it fails due to a lack of permissions.

It sounds like we need to fix this on the backend then. It's pretty strange to show a preview and allow an action to be taken within said preview that will always result in an error. You can take actions to like [Approve]/[Pay] an expenseReport from it's preview component in the parent chat without having to open the expenseReport first, so I think we should bring taskReports in-line to allow the same. @thienlnam will you progress this?

rushatgabhane · 2024-04-24T11:22:49Z

Yes, it says you don't have permission but you can dismiss the error and the task will be marked as complete.

rushatgabhane

Other than #40688 (review), PR LGTM! 🎉

trjExpensify · 2024-04-24T11:31:16Z

I tested this case. The backend does not accept completing by clicking on task reports. The app passed all IDs correctly. I can complete tasks, once I open task reports. I am not sure that it is a frontend bug. The backend successfully opens task reports, but not complete. Is it possible that the backend does not see task reports until I open it?

On this, @thienlnam's explanation in thread is that until you open a task, you aren't granted ‘read, write, share’ permissions. So if you try to complete the task from the "preview" in the parent chat before you've opened the taskReport up, it fails due to a lack of permissions.

It sounds like we need to fix this on the backend then. It's pretty strange to show a preview and allow an action to be taken within said preview that will always result in an error. You can take actions to like [Approve]/[Pay] an expenseReport from it's preview component in the parent chat without having to open the expenseReport first, so I think we should bring taskReports in-line to allow the same. @thienlnam will you progress this?

Follow-up on this. It looks like for a task in a "normal" DM, we don't let the user get that far until they've clicked into the taskReport first. We disable the action from being taken until then:

So another option as an interim step might be to do the same in the Concierge DM until we figure out a broader plan.

shawnborton · 2024-04-24T11:57:32Z

Also cc @Expensify/design @dubielzyk-expensify - is this the correct padding around the "Welcome" message below the video?

rezkiy37 · 2024-04-24T12:24:26Z

I tested this case. The backend does not accept completing by clicking on task reports. The app passed all IDs correctly. I can complete tasks, once I open task reports. I am not sure that it is a frontend bug. The backend successfully opens task reports, but not complete. Is it possible that the backend does not see task reports until I open it?

On this, @thienlnam's explanation in thread is that until you open a task, you aren't granted ‘read, write, share’ permissions. So if you try to complete the task from the "preview" in the parent chat before you've opened the taskReport up, it fails due to a lack of permissions.
It sounds like we need to fix this on the backend then. It's pretty strange to show a preview and allow an action to be taken within said preview that will always result in an error. You can take actions to like [Approve]/[Pay] an expenseReport from it's preview component in the parent chat without having to open the expenseReport first, so I think we should bring taskReports in-line to allow the same. @thienlnam will you progress this?

Follow-up on this. It looks like for a task in a "normal" DM, we don't let the user get that far until they've clicked into the taskReport first. We disable the action from being taken until then:
So another option as an interim step might be to do the same in the Concierge DM until we figure out a broader plan.

This behavior happens because the receiver does not have a task report object. There is a check for this. So it is impossible for the onboarding tasks because the current user generates them on their own side.
cc @trjExpensify

rezkiy37 · 2024-04-24T12:33:22Z

Should the popover be over a blank screen? It should be over the home screen, right?

Screen.Recording.2024-04-24.at.16.38.57.mov

Fixed - 2353824.

Details

thienlnam · 2024-04-24T16:35:53Z

I can spin up a PR to have it shared within DM reports. Created an issue to track it here https://github.com/Expensify/Expensify/issues/390483

mountiny

Looks good, I would make sure to merge this before we deploy again

mountiny · 2024-04-24T19:06:20Z

So I am going to merge this one now. Francois has a fix for the thumbnails not working as expected here.

This PR is adding the correct thumbnails #40933

And finally @rezkiy37 got a PR for the videos

OSBotify · 2024-04-24T19:13:43Z

✋ 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 · 2024-04-25T14:08:32Z

🚀 Deployed to staging by https://github.com/mountiny in version: 1.4.66-0 🚀

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

OSBotify · 2024-04-25T14:26:11Z

🚀 Deployed to staging by https://github.com/mountiny in version: 1.4.66-0 🚀

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

OSBotify · 2024-04-26T22:57:13Z

🚀 Deployed to production by https://github.com/mountiny in version: 1.4.66-5 🚀

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

parasharrajat · 2024-05-17T20:07:46Z

src/libs/Navigation/getOnboardingModalScreenOptions/index.native.ts

+import type {StyleUtilsType} from '@styles/utils';
+
+function getOnboardingModalScreenOptions(isSmallScreenWidth: boolean, styles: ThemeStyles, StyleUtils: StyleUtilsType) {
+    return getRootNavigatorScreenOptions(isSmallScreenWidth, styles, StyleUtils).fullScreen;


We failed to handle gestures. #40555

@parasharrajat sorry, I didn't get what you meant. What is the problem with have with the gestures?

Technically, this modal should not close even from gestures but we didn't block user gestures which can be used to close this Modal. You fixed it here #41765 for the #40555

rezkiy37 added 4 commits April 22, 2024 13:21

fix nav options

2e68cfb

fix reply counter

f06b9d9

fix subtitle bold texts

0ef0b24

implement failure data

7ee2d7a

rezkiy37 mentioned this pull request Apr 23, 2024

[HOLD for payment 2024-05-03] Onboarding - Message received in Concierge DM does not include correct and bold text #40725

Closed

6 tasks

update translation

b129552

rezkiy37 mentioned this pull request Apr 23, 2024

[Guided Setup Stage 2] Keyboard not opened when navigating to enter name screen #40750

Closed

6 tasks

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fix…

51eebcb

…/onboarding-fixes

mountiny assigned rezkiy37 Apr 23, 2024

mountiny requested a review from rushatgabhane April 23, 2024 13:04

rezkiy37 marked this pull request as ready for review April 23, 2024 14:43

rezkiy37 requested a review from a team as a code owner April 23, 2024 14:43

melvin-bot bot requested review from mountiny and rayane-djouah and removed request for a team April 23, 2024 14:44

trjExpensify added the Ready To Build label Apr 23, 2024

This comment has been minimized.

Sign in to view

rushatgabhane suggested changes Apr 23, 2024

View reviewed changes

Merge branch 'main' of https://github.com/rezkiy37/Expensify into fix…

fbd5bb5

…/onboarding-fixes

pecanoro requested changes Apr 24, 2024

View reviewed changes

src/languages/es.ts Outdated Show resolved Hide resolved

mountiny reviewed Apr 24, 2024

View reviewed changes

src/languages/en.ts Show resolved Hide resolved

mountiny previously approved these changes Apr 24, 2024

View reviewed changes

src/styles/variables.ts Show resolved Hide resolved

fix translation

9968865

rezkiy37 dismissed mountiny’s stale review via 9968865 April 24, 2024 10:50

rezkiy37 requested review from pecanoro and mountiny April 24, 2024 10:50

rushatgabhane reviewed Apr 24, 2024

View reviewed changes

rushatgabhane approved these changes Apr 24, 2024

View reviewed changes

fix welcome video modal options

2353824

pecanoro approved these changes Apr 24, 2024

View reviewed changes

mountiny approved these changes Apr 24, 2024

View reviewed changes

mountiny merged commit 619ddaa into Expensify:main Apr 24, 2024
16 checks passed

github-actions bot mentioned this pull request Apr 25, 2024

Deploy Checklist: New Expensify 2024-04-25 #40981

Closed

53 tasks

lanitochka17 mentioned this pull request Apr 25, 2024

Onboarding - Onboarding video does not have an audio #41000

Closed

6 tasks

melvin-bot bot mentioned this pull request Apr 26, 2024

[HOLD for payment 2024-05-03] [Guided Setup Stage 2] Use new CompleteGuidedSetup API to create tasks and messages (Phase 1) #38771

Closed

rezkiy37 mentioned this pull request May 7, 2024

[HOLD for payment 2024-05-22] [$250] Onboarding- By pulling vertically, user is able to dismiss iOS Onboarding #40555

Closed

6 tasks

parasharrajat reviewed May 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Onboarding fixes #40688

Onboarding fixes #40688

rezkiy37 commented Apr 22, 2024 •

edited

Loading

melvin-bot bot commented Apr 23, 2024

This comment has been minimized.

rushatgabhane commented Apr 23, 2024 •

edited

Loading

rushatgabhane left a comment •

edited

Loading

rushatgabhane commented Apr 23, 2024

trjExpensify commented Apr 23, 2024

mountiny commented Apr 23, 2024

trjExpensify commented Apr 24, 2024

thienlnam commented Apr 24, 2024

trjExpensify commented Apr 24, 2024

trjExpensify commented Apr 24, 2024

rushatgabhane commented Apr 24, 2024 •

edited

Loading

github-actions bot commented Apr 24, 2024

mountiny left a comment

rushatgabhane left a comment

trjExpensify commented Apr 24, 2024

rushatgabhane commented Apr 24, 2024 •

edited

Loading

rushatgabhane left a comment

trjExpensify commented Apr 24, 2024

shawnborton commented Apr 24, 2024

rezkiy37 commented Apr 24, 2024 •

edited

Loading

rezkiy37 commented Apr 24, 2024

thienlnam commented Apr 24, 2024

mountiny left a comment

mountiny commented Apr 24, 2024

OSBotify commented Apr 24, 2024

OSBotify commented Apr 25, 2024

OSBotify commented Apr 25, 2024

OSBotify commented Apr 26, 2024

parasharrajat May 17, 2024

rezkiy37 May 20, 2024

parasharrajat May 20, 2024

Onboarding fixes #40688

Onboarding fixes #40688

Conversation

rezkiy37 commented Apr 22, 2024 • edited Loading

Details

Fixed Issues

Tests

With the creation of a workspace (Track business expenses..., Manage my team’s...)

Without the creation of a workspace (others)

Fixes:

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Translations

Integrated failureData for the CompleteGuidedSetup command.

Bold texts

melvin-bot bot commented Apr 23, 2024

This comment has been minimized.

rushatgabhane commented Apr 23, 2024 • edited Loading

Reviewer Checklist

Screenshots/Videos

rushatgabhane left a comment • edited Loading

Choose a reason for hiding this comment

rushatgabhane commented Apr 23, 2024

trjExpensify commented Apr 23, 2024

mountiny commented Apr 23, 2024

trjExpensify commented Apr 24, 2024

thienlnam commented Apr 24, 2024

trjExpensify commented Apr 24, 2024

trjExpensify commented Apr 24, 2024

rushatgabhane commented Apr 24, 2024 • edited Loading

github-actions bot commented Apr 24, 2024

mountiny left a comment

Choose a reason for hiding this comment

rushatgabhane left a comment

Choose a reason for hiding this comment

trjExpensify commented Apr 24, 2024

rushatgabhane commented Apr 24, 2024 • edited Loading

rushatgabhane left a comment

Choose a reason for hiding this comment

trjExpensify commented Apr 24, 2024

shawnborton commented Apr 24, 2024

rezkiy37 commented Apr 24, 2024 • edited Loading

rezkiy37 commented Apr 24, 2024

thienlnam commented Apr 24, 2024

mountiny left a comment

Choose a reason for hiding this comment

mountiny commented Apr 24, 2024

OSBotify commented Apr 24, 2024

OSBotify commented Apr 25, 2024

OSBotify commented Apr 25, 2024

OSBotify commented Apr 26, 2024

parasharrajat May 17, 2024

Choose a reason for hiding this comment

rezkiy37 May 20, 2024

Choose a reason for hiding this comment

parasharrajat May 20, 2024

Choose a reason for hiding this comment

rezkiy37 commented Apr 22, 2024 •

edited

Loading

Integrated `failureData` for the CompleteGuidedSetup command.

rushatgabhane commented Apr 23, 2024 •

edited

Loading

rushatgabhane left a comment •

edited

Loading

rushatgabhane commented Apr 24, 2024 •

edited

Loading

rushatgabhane commented Apr 24, 2024 •

edited

Loading

rezkiy37 commented Apr 24, 2024 •

edited

Loading