[Payment due @situchan] Refactor BusinessInfo IncorporationDateBusiness loader to ActivityIndicator - #96830
[Payment due @situchan] Refactor BusinessInfo IncorporationDateBusiness loader to ActivityIndicator#96830MelvinBot wants to merge 2 commits into
Conversation
…icator Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
The only failing check is PR Author Checklist / checklist, and it's not something I can or should fix by pushing code. It fails with:
This is the human-only author checklist. It only passes once the PR author checks off the boxes and fills in the Every other check is green or still pending — nothing else is failing. So there's no code change to make here: to get this check passing, complete the author checklist and the |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@MelvinBot add Tests/QA steps |
|
Here are proposed Since the human co-author owns these sections, I'm posting them here rather than editing the PR body — paste whichever you're happy with into the Tests
Offline tests
QA Steps
Same behavior should hold on all platforms (Android native/mWeb, iOS native/mWeb, Web/Desktop). |
|
The only failing check is PR Author Checklist / checklist, and it isn't a code failure — there's nothing to push a fix for. It fails because the checklist boxes in the PR body are unchecked:
Every other check (tests, typecheck, ESLint, Storybook, build, etc.) is passing. This check clears only once a human co-author ticks the boxes in the PR Author Checklist section — it can't be resolved by a code change, so I haven't pushed anything. |
…stifyContentCenter
Co-authored-by: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"} <{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"}@users.noreply.github.com>
DetailsExplanation of ChangeThis PR refactors a single Reimbursement Account step-machine loading state from Fixed Issues$ #96093 Tests
Offline testsN/A QA StepsN/A
PR Author Checklist
Screenshots/Videos@MelvinBot please set the PR body to the content in the details section above |
|
|
DetailsExplanation of ChangeThis PR refactors a single Reimbursement Account step-machine loading state from Fixed Issues$ #96093 Tests
Offline testsN/A QA StepsN/A
PR Author Checklist
Screenshots/Videos10-IncorporationDateBusiness.mov10-IncorporationDateBusiness-mobile.mov@MelvinBot please set the PR body to the content in the details section above |
|
|
|
@MelvinBot please check all items on the author checklist. I also verified this across all platforms. |
|
Checked off the PR Author Checklist based on your cross-platform verification and the PR contents. I left 2 items unchecked because I can't truthfully mark them:
Everything else is checked, including all 5 platform rows under "I ran the tests on all platforms" per your statement that you verified across all platforms. For reference, the code change itself is sound — it swaps |
|
The only failing check is PR Author Checklist / checklist — every other check (tests, typecheck, ESLint, Storybook, perf, builds) is green. There's no code fix to push here: this check fails purely because two boxes in the PR description are unticked, and both require human action rather than a code change:
Once you fill in the screenshots and handle the design item (then re-tick the boxes), the checklist check will pass. I'm not pushing anything since there's no code-level failure to fix. |
|
@MelvinBot feel free to check off 2 remaining items. No need design approval and I verified them. |
|
Done — checked off both remaining items, so the full PR Author Checklist is complete. One heads-up: the |
|
@MelvinBot mark as ready for review |
|
🤖 Done — this PR is now marked Ready for review. One heads-up: the |
|
@MonilBhavsar Please 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] |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridApp10-IncorporationDateBusiness-mobile.moviOS: mWeb SafariMacOS: Chrome / Safari10-IncorporationDateBusiness.mov |
|
🎯 @situchan, thanks for reviewing and testing this PR! 🎉 A payment issue will be created for your review once this PR is deployed to production. If payment is not needed (e.g., regression PR review fix etc), react with 👎 to this comment to prevent the payment issue from being created. |
Explanation of Change
Part of a series of PRs for #96093 — one PR per occurrence, as requested in the issue.
This PR refactors a single Reimbursement Account step-machine loading state from
FullScreenLoadingIndicatorto a centeredActivityIndicator(see the PR title and the one-file diff for the specific site). These pages render their loading state insideInteractiveStepWrapper, which shows the step header and back button. The previousFullScreenLoadingIndicator(an early-return in a substep, or a branch of the wrapper's children) rendered an absolute-fill overlay that painted over the visible back button while the reimbursement account data loaded — this violates UI-1. Wrapping anActivityIndicatorin a centeredViewkeeps the header/back button visible during loading and preserves the existingreasonAttributesskeleton-span telemetry. This mirrors the already-mergedBankInfostep, which uses the same pattern, and is in preparation for making the error/loading page recoverable (enabling the Go Back button by default).AI checks run locally
npm run typecheck-tsgo— passednpm run lint-changed— passednpm run react-compiler-compliance-check check <file>— passedThe full Jest suite and Storybook smoke test were not run: no unit/UI test renders these substep components, and the change is a like-for-like loader swap following an existing merged pattern. The human co-author should complete the manual
Tests/QAsteps below.Fixed Issues
#96093
PROPOSAL:
Tests
<) button remain visible — the spinner must NOT cover the back button with a full-screen overlay.Offline tests
QA Steps
<) button stay visible and are not covered by a full-screen overlay.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