Enable Go Back button on FullscreenLoadingIndicator in withReportOrNotFound - #88745
Conversation
…rtOrNotFound FullscreenLoadingIndicator includes a Go Back button that would be redundant alongside the existing navigation controls (LHN, RHP, stack navigator) visible during the loading state. Replace it with an ActivityIndicator wrapped in a View that reproduces the same layout and background styling (absoluteFillObject, fullScreenLoading, w100). Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
….absoluteFill Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
Fixed the failing ESLint check: replaced deprecated |
|
@MelvinBot add Tests/QA Steps and check off all items in author checklist as verified by me. And pull main |
|
Updated the PR description:
|
|
@MelvinBot pull main |
…icatorInWithReportOrNotFound Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
Merged |
| return ( | ||
| <View style={[StyleSheet.absoluteFill, styles.fullScreenLoading, styles.w100]}> | ||
| <ActivityIndicator | ||
| size={CONST.ACTIVITY_INDICATOR_SIZE.LARGE} | ||
| reasonAttributes={reasonAttributes} | ||
| /> | ||
| </View> | ||
| ); |
There was a problem hiding this comment.
@MelvinBot which pattern should we follow based on https://github.com/Expensify/App/blob/main/.claude/skills/coding-standards/rules/ui-1-correct-loading-indicator.md?
|
Per UI-1,
|
@MelvinBot but I don't see back navigation because of early return - never reaches wrapped component render App/src/pages/inbox/report/withReportOrNotFound.tsx Lines 117 to 130 in da0f92e |
|
You're right — I was wrong in my previous comment. On narrow/mobile screens, there is no visible navigation during the loading state:
The user sees only a bare spinner with no way to navigate back (except iOS swipe gesture or Android hardware back — but nothing on mobile web).
|
@MelvinBot lets do former approach with |
Keep FullscreenLoadingIndicator instead of raw ActivityIndicator, and enable shouldUseGoBackButton so users get a "Go Back" option after a timeout. Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
@MelvinBot update Explanation of Change, Tests / Offline / QA Steps based on latest update |
|
Reverted to |
|
Updated the PR title to match the current implementation. The Explanation of Change, Tests, Offline tests, and QA Steps in the body already accurately reflect the latest change (enabling |
|
@MelvinBot mark as ready for review |
|
I wasn't able to mark this PR as ready for review due to a permissions limitation. |
|
@MelvinBot mark as ready for review |
|
Marked this PR as ready for review per |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppios.moviOS: mWeb Safarimsafari.movMacOS: Chrome / Safariweb.mov |
There was a problem hiding this comment.
This PR is adding shouldUseGoBackButton to the existing FullscreenLoadingIndicator as no back button exists.
All other PRs in this batch are replacing FullscreenLoadingIndicator with ActivityIndicator as per UI-1 rule.
cc: @Julesssss
|
No product review needed |
Julesssss
left a comment
There was a problem hiding this comment.
Agree with your point.
Thanks @ShridharGoel, could you please just update the linked issue description to avoid confusion. |
@MelvinBot do this please |
|
Updated the issue description to clarify that this PR enables |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 @Julesssss has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/Julesssss in version: 9.3.75-0 🚀
Bundle Size Analysis (Sentry): |
|
No help site changes are required for this PR. This PR adds |
|
🚀 Deployed to production by https://github.com/roryabraham in version: 9.3.75-14 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Enable
shouldUseGoBackButtonon the existingFullscreenLoadingIndicatorinwithReportOrNotFound.tsx.Previously the loading state used
FullscreenLoadingIndicatorwithout the "Go Back" button. This change enablesshouldUseGoBackButtonso that after a timeout, users see a "Go Back" button if the report takes too long to load — providing a way to navigate away from a stuck loading state.Fixed Issues
$ #85390
Tests
withReportOrNotFound(e.g. open a deep link to a report that needs to be fetched)Offline tests
QA Steps
withReportOrNotFound(e.g. open a deep link to a report)PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari