Repository navigation
Use ActivityIndicator inside the Per Diem tab while the selected workspace loads - #103421
Conversation
…space loads Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@MelvinBot add QA steps |
|
Here are the QA steps for this PR. I couldn't update the PR description from this run. Please paste these into the QA Steps section. Prerequisite: an account that is a member of at least two workspaces with **Per diem** enabled (Workspace > More features > Per diem), each with at least one per diem rate.
1. Sign out and sign back in, so the workspaces' per diem rates are not already loaded. On web, you can set DevTools > Network to "Slow 3G" to make the loading state easier to see.
2. Click the FAB > **Create expense** > **Per diem** tab.
3. Select one of the workspaces.
4. While the workspace loads, verify:
- A spinner shows inside the Per diem tab content.
- The "Create expense" header and the tab bar stay visible above the spinner.
- No extra "Go back" button appears under the header.
5. Verify the **Destination** step opens after loading finishes, with no stuck spinner.
6. Repeat steps 1-3, but go offline right before step 3. Verify the "You appear to be offline" view shows inside the tab, and no spinner or crash appears.
7. Verify that no errors appear in the JS console.
- [ ] Verify that no errors appear in the JS consoleWhy these stepsThe loader only renders when you pick a workspace whose per diem rates are not in Onyx yet. While they load, |
|
@MelvinBot add QA steps like below: |
|
I added your QA steps to the QA Steps section of the PR description, word for word. They replace the TODO placeholder. I kept the "Verify that no errors appear in the JS console" checkbox below them, and left the rest of the description unchanged. |
|
@MelvinBot mark this PR ready for review |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
🤖 This PR is now marked ready for review. Heads-up: the |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safaritest.mov |
| return <FullPageOfflineBlockingView>{null}</FullPageOfflineBlockingView>; | ||
| } | ||
| return <FullScreenLoadingIndicator />; | ||
| // This step only renders inside the Per Diem tab, so the header and tab bar stay visible above the loader |
There was a problem hiding this comment.
Comment unecessary I think
Julesssss
left a comment
There was a problem hiding this comment.
Minor change please: https://github.com/Expensify/App/pull/103421/changes#r4221978820
Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
@situchan @Julesssss 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] |
|
✋ 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.5.7-0 🚀
|
|
No help site update needed: this PR only changes which loading spinner appears inside the Per Diem tab while a workspace's per diem data loads, which is loading behavior and doesn't make any help article sentence incorrect. |
|
This PR failing because of the issue #103721 |
I managed to reproduce this in production so not blocker bug.mov |
|
This PR failing because of the issue #103838 |
I reproduced this in production too so not blocker |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.5.7-3 🚀
Bundle Size Analysis (Sentry): |


Explanation of Change
IOURequestStepPerDiemWorkspaceis only rendered inside the Per Diem tab ofIOURequestStartPage(it has no standalone route). While a selected workspace's per diem data loads (pendingPolicyIDis set and the device is online), it returns aFullScreenLoadingIndicator. That loader'sabsoluteFillcovers only the tab content, so the start page'sHeaderWithBackButtonandTabSelectorstay visible above it. Once #96098 flipsFullScreenLoadingIndicator'sshouldUseGoBackButtondefault totrue, the tab would show a second, redundant "Go Back" button under the header.This swaps it for the same
ActivityIndicatorpatternIOURequestStartPagealready uses for its other tab-content loaders (Viewwithstyles.flex1andstyles.fullScreenLoadingwrapping a largeActivityIndicator). The loading condition and the offline branch are unchanged.This is one of the sites found in the re-audit before the flip.
AI tests run locally
npm run lint-changed: passednpm run typecheck: passednpm run react-compiler-compliance-check check <file>: passednpm run spell-changed: passedoxfmt: clean--findRelatedTests: 91/92 suites passed.tests/unit/pages/HomePage.test.tsxfailed only in the parallel run and passes in isolation on both this change andmain, so it's a pre-existing isolation flake.Fixed Issues
$ #96098
PROPOSAL: #96098 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validates your changed worked.
Offline tests
QA Steps
Prerequisite: an account that is a member of at least two workspaces with Per diem enabled (Workspace > More features > Per diem), each with at least one per diem rate.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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