[Payment due @situchan] Refactor BusinessInfo PhoneNumberBusiness loader to ActivityIndicator - #96832
[Payment due @situchan] Refactor BusinessInfo PhoneNumberBusiness loader to ActivityIndicator#96832MelvinBot wants to merge 2 commits into
Conversation
Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
The only failing check is It fails because the PR Author Checklist is empty: the run reports Every automated code check is green: Bun tests, ESLint, Oxfmt, typecheck, React Compiler Compliance, Check for unused styles, knip, spellcheck, Storybook tests, and both builds all passed. (The two perf-test jobs were still running when I checked; they're benchmarks and don't gate on code correctness for a like-for-like loader swap.) So: no action needed from me on the code. Complete the author checklist to clear the failing check. 👍 |
|
@MelvinBot add Tests/QA steps |
|
Here are suggested
Tests
Offline tests
QA Steps
|
|
The only failing check is PR Author Checklist / checklist — there's no code fix to push. Every real CI check is green (tests, To clear it: complete the PR Author Checklist and fill in the human-only |
…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/Videos7-PhoneNumberBusiness.mov7-PhoneNumberBusiness-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. |
|
Done — I ticked all 38 items in the PR Author Checklist (0 left unchecked), based on your confirmation that you verified this across all platforms. The I kept the rest of the body intact — including the One thing to double-check: the Screenshots/Videos section is still empty (the platform |
|
@MelvinBot mark as ready for review |
|
🤖 Marked this PR as ready for review,
|
|
@carlosmiceli 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: HybridApp7-PhoneNumberBusiness-mobile.moviOS: mWeb SafariMacOS: Chrome / Safari7-PhoneNumberBusiness.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