Conversation
| InteractionManager.runAfterInteractions(() => { | ||
| completeTestDriveTask(viewTourTaskReport, viewTourTaskParentReport, isViewTourTaskParentReportArchived, currentUserPersonalDetails.accountID); | ||
| }); | ||
| }, [viewTourTaskReport, viewTourTaskParentReport, isViewTourTaskParentReportArchived, currentUserPersonalDetails.accountID, completeTestDriveTask]); |
There was a problem hiding this comment.
❌ PERF-6 (docs)
The dependency array includes the entire viewTourTaskReport object, but the hook only uses viewTourTaskReport.stateNum in the conditional check. Passing entire objects as dependencies causes the effect to re-execute whenever any property changes, even unrelated ones.
Suggested fix:
}, [viewTourTaskReport?.stateNum, viewTourTaskParentReport, isViewTourTaskParentReportArchived, currentUserPersonalDetails.accountID]);Additionally, completeTestDriveTask is a stable function imported from an action module and should not be included in the dependency array as it never changes.
Codecov Report❌ Patch coverage is
... and 18 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| InteractionManager.runAfterInteractions(() => { | ||
| completeTestDriveTask(viewTourTaskReport, viewTourTaskParentReport, isViewTourTaskParentReportArchived, currentUserPersonalDetails.accountID); | ||
| }); | ||
| }, [viewTourTaskReport, viewTourTaskParentReport, isViewTourTaskParentReportArchived, currentUserPersonalDetails.accountID, completeTestDriveTask]); |
There was a problem hiding this comment.
| }, [viewTourTaskReport, viewTourTaskParentReport, isViewTourTaskParentReportArchived, currentUserPersonalDetails.accountID, completeTestDriveTask]); | |
| }, [viewTourTaskReport, viewTourTaskParentReport, isViewTourTaskParentReportArchived, currentUserPersonalDetails.accountID]); |
|
Updated |
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-deprecated | ||
| InteractionManager.runAfterInteractions(() => { | ||
| completeTestDriveTask(viewTourTaskReport, viewTourTaskParentReport, isViewTourTaskParentReportArchived, currentUserPersonalDetails.accountID); |
There was a problem hiding this comment.
This call is now duplicated. Please check #73749 and refactor code accordingly.
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Change looks good to me from a product perspective 👍
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppandroid.movAndroid: mWeb Chromemchrome.moviOS: HybridAppios.moviOS: mWeb Safarimsafari.movMacOS: Chrome / Safariweb.movMacOS: Desktop |
|
@ShridharGoel please update test steps.
This doesn't make sense. You missed important step:
|
|
@situchan But the test drive shows even if we select some other option |
But it doesn't lead to completing task |
|
But I've updated the steps now |
Oh ok |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/flodnv in version: 9.2.45-0 🚀
|
|
🚀 Deployed to production by https://github.com/luacmartins in version: 9.2.45-6 🚀
|
Explanation of Change
After finishing the test drive flow and pressing Get Started, the onboarding checklist still shows “Take a test drive” as incomplete. This change fixes the issue.
Fixed Issues
$ #71688
PROPOSAL: #71688 (comment)
Tests
Offline tests
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)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
TestDrive.mov
Android: mWeb Chrome
Screen.Recording.2025-11-03.at.11.15.34.PM.mov
iOS: Native
Simulator.Screen.Recording.-.iPhone.16.Plus.-.2025-11-03.at.22.59.46.mov
iOS: mWeb Safari
Simulator.Screen.Recording.-.iPhone.16.Plus.-.2025-11-03.at.23.01.54.mp4
MacOS: Chrome / Safari
Screen.Recording.2025-11-03.at.10.41.00.PM.mov
MacOS: Desktop
Screen.Recording.2025-11-03.at.11.27.25.PM.mov