Conversation
…tructure for onboarding
…m/barttom/App-expensify into feature/38771-tasks-for-guided-setup
…tructure for onboarding
…m/barttom/App-expensify into feature/38771-tasks-for-guided-setup
…ture/38771-tasks-for-guided-setup
…ture/38771-tasks-for-guided-setup
…ture/38771-tasks-for-guided-setup
Yep noting these are known We are moving to using a new NVP: I think that the changes here are minor and we could merge this first and then retest the enablement of the flow. @rayane-djouah could you complete the checklist? |
mountiny
left a comment
There was a problem hiding this comment.
Non blocking comments, I would love to get this merged to main before we enable the onboarding flow
| <RootStack.Screen | ||
| name={NAVIGATORS.ONBOARDING_MODAL_NAVIGATOR} | ||
| options={onboardingScreenOptions} | ||
| options={screenOptions.fullScreen} |
There was a problem hiding this comment.
This is I assume only for testing now, right?
There was a problem hiding this comment.
I'm not sure about this one, onboardingScreenOptions come from here
const onboardingScreenOptions = useMemo(() => screenOptions.onboardingModalNavigator(shouldUseNarrowLayout), [screenOptions, shouldUseNarrowLayout]);There was a problem hiding this comment.
We can handle that in the follow up PR. thanks
src/pages/OnboardingPersonalDetails/BaseOnboardingPersonalDetails.tsx
Outdated
Show resolved
Hide resolved
src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx
Outdated
Show resolved
Hide resolved
src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx
Outdated
Show resolved
Hide resolved
|
Thank you very much! I have tested this out and we are going to move the finishing touches to the Enablement PR. I think we need to remove the videos for phase 1 Screen.Recording.2024-04-21.at.22.24.12.mp4 |
mountiny
left a comment
There was a problem hiding this comment.
Thanks @fabioh8010 for help ❤️
this PR should not influence the user experience on main. The flow is going to be enabled in this PR #39687 and we are going to work on Monday to clean everything up
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
@rezkiy37 Hello |
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.4.64-6 🚀
|
| // Only navigate to concierge chat when central pane is visible | ||
| // Otherwise stay on the chats screen. | ||
| if (isSmallScreenWidth) { | ||
| Navigation.navigate(ROUTES.HOME); |
There was a problem hiding this comment.
@rezkiy37 , why did we navigate to HOME instead of Concierge on small devices ? We have a GH issue here which has expected result that we should always navigate to concierge on all devices, was this intentional ? c.c. @mountiny and @rayane-djouah as they seem to have reviewed the PR
There was a problem hiding this comment.
Yes, it was intentional. The doc says:
In the component OnboardingPurpose we can find a method that sends a message as the Concierge persona. After this action, the user should be able to see the video modal and be navigated to the root screen.
| function getDBTimeWithSkew(timestamp: string | number = ''): string { | ||
| if (networkTimeSkew > 0) { | ||
| return getDBTime(new Date().valueOf() + networkTimeSkew); | ||
| return getDBTime(new Date(timestamp).valueOf() + networkTimeSkew); |
There was a problem hiding this comment.
Looks like this caused an issue #40724 preventing chat to scroll on new messages. When timestamp is not passed new Date('').valueOf() becomes NaN.
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT}${targetChatReportID}`, | ||
| value: { | ||
| lastMentionedTime: DateUtils.getDBTime(), |
There was a problem hiding this comment.
We haven't provided lastVisibleActionCreated optimistically for the target chat Report here, so the Concierge chat can auto scroll to the bottom after onboarding process. #52025
| tasks: [ | ||
| { |
There was a problem hiding this comment.
We did not include selfGuidedTourTask in the tasks if the tour purpose is combinedTrackSubmitOnboardingChoices.PERSONAL_SPEND leading to this issue.
There was a problem hiding this comment.
Sorry for that and thanks a lot for tracking. I will post i there
| onyxMethod: Onyx.METHOD.MERGE, | ||
| key: `${ONYXKEYS.COLLECTION.REPORT}${targetChatReportID}`, | ||
| value: { | ||
| lastMentionedTime: DateUtils.getDBTime(), |
There was a problem hiding this comment.
Came from this issue
We have redundant lastMentionedTime what causes unnecessary GRB for Concierge chat in LHN after onboarding when there is no onboarding task
More information here

Details
The PR introduces new onboarding flow. The app uses the new API command CompleteGuidedSetup and pass new tasks and message's optimistic data, on behalf of Concierge. After, the app redirects user to root screen.
Fixed Issues
$ #38771
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...)
Without the creation of a workspace (others)
Offline tests
Same as Tests.
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android.mp4
Android: mWeb Chrome
Android.Chrome.mp4
iOS: Native
iOS: mWeb Safari
IOS.Safari.mp4
MacOS: Chrome / Safari
Chrome.Track.mp4
Chrome.Chat.mp4
MacOS: Desktop
Desktop.mp4