fix: focus modal is not displayed when account reaches 30 reports - #73988
Conversation
|
LGTM |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| } | ||
| const isNarrowLayout = getIsNarrowLayout(); | ||
| const shouldHideModalOnNavigation = (isNarrowLayout && currentRouteName !== SCREENS.HOME) || (!isNarrowLayout && currentRouteName !== SCREENS.REPORT); | ||
| const isNavigatingToPriorityModePage = currentRouteName === SCREENS.SETTINGS.PREFERENCES.PRIORITY_MODE; |
There was a problem hiding this comment.
Sorry @samranahm, but could you help me understand the proposed changes? I can't reproduce the issue on the web (Chrome), the modal is displayed normally:

Why does this only happen on Android web? Thank you.
There was a problem hiding this comment.
@brunovjk Sorry for the delay, let me explain.
We recently added the useEffect to close the focus modal when user press settings and they navigate to another page (on mobile fromHOME screen and on large width devices fromREPORT screen)
Now when we create 30th report from global create button we switch to #focus mode, and shouldShowModal become true and because on mobile we navigate from HOME to REPORT screen when we create a report through global create button the navigation happen and focus modal was closing instantly, because shouldHideModalOnNavigation was true for narrow layout.
There was a problem hiding this comment.
Thank you for clarifying, I will continue the review now.
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp73988_android_native.movAndroid: mWeb Chrome73988_android_web.moviOS: HybridApp73988_ios_native.moviOS: mWeb Safari73988_ios_web.movMacOS: Chrome / Safari73988_web_chrome_small.mov73988_web_chrome_large.movMacOS: Desktop73988_web_desktop.mov |
|
@samranahm as soon as I open the app, for the first time after build, on the web I get a warning: Screen.Recording.2025-11-03.at.12.02.34.movNot reproducible in main, did you also have this error? Did you test it on all platforms? Thank you. |
|
But the regression is no longer reproducible, I'll continue testing on the other platforms after lunch: Screen.Recording.2025-11-03.at.12.08.44.mov |
|
Looking into it |
I just merge main and it did not appear for me, please check again. |
|
@brunovjk I can actually still reproduce this after some retakes on android native. I don't think this is regression since it's only about how we navigate to report page after creating the report through global_create. But we should indeed fix it. focus.mode.notification.mov |
|
Sorry, I got a little confused @samranahm, are you working on a solution? Thank you. |
|
Ahh, sorry if anything make you confused, I actually replied to this comment
that I am still able to reproduce this on main (not in this current branch) so we should fix it. Let's escalate this PR since the fix is ready. |
|
@samranahm, on most platforms work fine, Screenshots. However, on native iOS the app freezes when the modal should appear, It's not very clear in the video, but I'm trying to press any button in the app, and nothing happens: 73988_ios_native_bug.movCan you reproduce it? Thank you. |
|
@brunovjk This issue coming from main because it persist even if we revert our PR. Let me fix this too. |
|
Testing solution on all platforms. |
|
@brunovjk Please take a look. |
|
Sure |
|
The same bug is occurring on iOS @samranahm: Screen.Recording.2025-11-04.at.11.57.11.mov |
|
@brunovjk It's working absolutely fine for me. IOS.native.movCan you please confirm if you include the latest commit, and restarted the metro bundler. |
|
I did, but I will try it again, thank you. |
brunovjk
left a comment
There was a problem hiding this comment.
LGTM :D Thank you @samranahm
|
@jasperhuangg Friendly bump. |
|
@jasperhuangg Friendly bump ^ |
|
✋ 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/jasperhuangg in version: 9.2.58-0 🚀
|
|
Hello @samranahm, the Priority Mode changed to #focus mode after creating 20 reports, not 30 reports as stated in step 2. Is this expected? or consider an issue ? |
|
@mitarachim can you share how you create 30 reports, please note that after completing the onboarding process if you choose manage team ~10 reports automatically created as task reports, admin room, and expense chat. |
|
@samranahm sorry i forgot to attach video in previous question. QA team try to create 20 empty report in workspace chat . bandicam.2025-11-13.20-59-46-020.mp4 |
|
@mitarachim Looking at the display name in attachment tester more likely select manage team that create 10 reports, and then they created 20 more reports that change Priority mode. This is Expected behaviour not a bug. |
|
@samranahm I just want to confirm: since I selected Screen.Recording.2025-11-14.at.4.53.32.PM.mov |
|
@mitarachim Here's the breakdown of After sign-up
Create workspace
Grand total = 12 without creating a single report manually, so Priority Mode should trigger after creating 18 reports manually. Expected behaviour. |
|
Thank you so much for the explanation @samranahm |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.2.58-3 🚀
|
Explanation of Change
Fixed Issues
$ #72052
$ #73588
PROPOSAL:
Tests
#focusOffline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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
Android.native.mp4
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop