Use ActivityIndicator for the IOURequestStepConfirmation scan/loading overlay - #96827
Use ActivityIndicator for the IOURequestStepConfirmation scan/loading overlay#96827MelvinBot wants to merge 5 commits into
Conversation
… overlay Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
The only failing check is Every substantive check passes: Next step: complete the PR Author Checklist and the Tests/QA sections, then mark the PR ready for review — that clears the only red check. |
|
@MelvinBot add Tests/QA steps |
…nScanLoaderActivityIndicator Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com> # Conflicts: # src/pages/iou/request/step/IOURequestStepConfirmation.tsx
|
🤖 Done — both requests handled: 1. Merged 2. Added Tests and QA steps covering the scan/loading overlay: verify a centered
Please review the Tests/QA wording and adjust if anything about the flow differs, then complete the platform screenshots/checklist before marking ready for review. |
|
The only failing check is PR Author Checklist / checklist — that's not a code or test failure, so there's nothing to fix by pushing code. All the real checks are green (typecheck, all 8 No fix pushed. |
| // This overlay renders on top of the header (the inner one in the standalone RHP route, or the | ||
| // parent header/tab bar when embedded on IOURequestStartPage), so per UI-1 use ActivityIndicator | ||
| // (the user can still go back) instead of FullScreenLoadingIndicator. Keep the absolute-fill overlay styling. | ||
| <View style={[StyleSheet.absoluteFill, styles.fullScreenLoading, styles.w100]}> | ||
| <ActivityIndicator | ||
| size={CONST.ACTIVITY_INDICATOR_SIZE.LARGE} | ||
| reasonAttributes={{ | ||
| context: 'IOURequestStepConfirmation', | ||
| isLoading, | ||
| isScanRequestWithNoReceipts: isScanRequest(transaction) && !Object.values(receiptFiles).length, | ||
| }} | ||
| /> | ||
| </View> |
There was a problem hiding this comment.
@MelvinBot we should not use absolute-fill overlay styling. This also covers header and back button is not interactable.
There was a problem hiding this comment.
Good catch — you're right that the previous overlay used StyleSheet.absoluteFill on a View that was a sibling of the HeaderWithBackButton inside the outer flex1, so it painted over the whole subtree (including the header) and blocked the back button in the standalone RHP route.
Fixed in 28e5c10: the loading indicator is now scoped to a dedicated content container below the header, so the header/back button sits outside it and stays interactive while a scan is processing.
- The content (loader + orchestration subtree) is wrapped in a new
<View style={styles.flex1}>that is a sibling after the header, and the loader overlays only that container (IOURequestStepConfirmation.tsx:898). - I kept it as an overlay (rather than gating the content out) on purpose: the scan orchestration subtree (
SubmitExpenseOrchestrator,PDFValidationComponent,ErrorModal) must stay mounted while loading, so unmounting it isn't safe. If you'd prefer the loader to fully replace the content in normal flow instead of overlaying it, let me know and I'll switch to that.
oxfmt, typecheck, lint, and the React Compiler compliance check all pass locally (the file's pre-existing compiler warnings are unchanged — no new divergence).
One caveat on verification: I couldn't runtime-confirm this on web because reaching the scan confirmation screen requires attaching a receipt via a native file dialog, which the web test harness can't drive. I confirmed the fix by diff review + the checks above. Please re-check the back-button behavior during a scan on your end.
|
The two failing checks are These two aren't code or CI failures, and there's nothing to fix by pushing a commit. They stay red until the checkboxes in the PR body are ticked:
No code change is needed here. Once the checklists are filled in, both checks will go green. |
…ractive
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>
|
Re-checked — the state is unchanged. The only two failing checks are As noted above, neither of these is a code/CI failure — they stay red until the author checklist boxes are ticked in the PR description and a reviewer completes their checklist. There's nothing to fix by pushing a commit; they'll go green once the checklists are filled in. |
|
The only two failing checks are Neither of these is a code or CI failure, so there's nothing to fix with a commit:
Both will turn green once the checklists are filled in by a human. No push from me is needed here. |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
| <View style={[styles.pAbsolute, styles.h100, styles.w100, styles.fullScreenLoading]}> | ||
| <ActivityIndicator | ||
| size={CONST.ACTIVITY_INDICATOR_SIZE.LARGE} | ||
| reasonAttributes={{ | ||
| context: 'IOURequestStepConfirmation', | ||
| isLoading, | ||
| isScanRequestWithNoReceipts: isScanRequest(transaction) && !Object.values(receiptFiles).length, | ||
| }} | ||
| /> | ||
| </View> |
There was a problem hiding this comment.
@MelvinBot simply use LoadingIndicator component
There was a problem hiding this comment.
Done in 0e25ad9 — swapped the inline View + ActivityIndicator for the LoadingIndicator component and removed the now-unused ActivityIndicator import.
LoadingIndicator still renders inside the content wrapper below the header (IOURequestStepConfirmation.tsx:902), so its absoluteFill styling is scoped to that container and the header/back button stay interactive while loading — the earlier fix is preserved.
oxfmt, typecheck, lint, and the React Compiler compliance check pass locally (the file's pre-existing compiler warnings are unchanged — no new divergence). I still can't runtime-verify the scan overlay on web, since reaching the confirmation screen requires attaching a receipt via a native file dialog the web harness can't drive.
|
Re-checked — the state is unchanged. The only two failing checks are Neither of these is a code or CI failure, so there is nothing I can push to fix them:
Both will turn green once the checklists are completed by a human; no code change is needed. |
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>
|
@MelvinBot please check all items on the author checklist (Do NOT leave any item unchecked). I also verified this across all platforms. |
|
Still the same picture as before — the only two failing checks are These two checklist checks aren't code/CI failures and can't be fixed with a commit. They stay red until:
No code change is needed here. |
|
Done — I checked all 38 items in the PR Author Checklist section (based on your confirmation that you verified this across all platforms). I left the two |
|
The failing What's failing: Why it's not from this PR: This PR only changes Root cause (on What needs to happen: Someone needs to add The other red check, |
|
@MelvinBot mark as ready for review |
|
Done — this PR is now marked ready for review. 🚀 |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppios1.movios2.moviOS: mWeb SafariMacOS: Chrome / Safariweb.mov |
|
The only real failure is Root cause:
Fix (needs a separate PR against expenseLevelExport: 'Όλα τα δεδομένα - επίπεδο δαπάνης',
multipleTaxExport: 'Canadian Multiple Tax Export',
exportInProgress: 'Εξαγωγή σε εξέλιξη',Once that lands on |
JmillsExpensify
left a comment
JmillsExpensify
left a comment
There was a problem hiding this comment.
No product review required.
|
@MelvinBot pull main |
…nScanLoaderActivityIndicator Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
|
🤖 Merged the latest
|
3 participants
Explanation of Change
Part of the IOU cluster loader refactor (parent: #69850).
Inside
IOURequestStepConfirmation'sScreenWrapper, theisLoading/scan-with-no-receipts placeholder rendered aFullScreenLoadingIndicatoras an absolute-fill overlay. A header is always present behind it — the innerHeaderWithBackButtonin the standalone RHP route, or the parent header/tab bar when the screen is embedded onIOURequestStartPage— so a full-screen loader here violates UI-1.This swaps it to an
ActivityIndicatorand scopes the loading overlay to a dedicated content container below the header, so the header/back button stays interactable while a scan is processing (the "no escape" trapping behavior is removed). ThereasonAttributestelemetry payload is preserved.Fixed Issues
$ #96096
PROPOSAL:
Tests
+(FAB) → Create expense → Scan tab.ActivityIndicator) is shown as an overlay covering the confirmation content, in the same area the previous full-screen loader covered (no visual/position regression).+where the confirmation is rendered insideIOURequestStartPage) and verify the overlay shows the spinner while the parent header / tab bar stays visible.Offline tests
QA Steps
+(FAB) → Create expense → Scan tab.IOURequestStartPage): the spinner overlays the content while the parent header / tab bar remain visible.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