Keep SSO button loading until AuthScreens mounts#90092
Conversation
|
@marufsharifi 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] |
|
🚧 @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! 🧪🧪
|
|
@Julesssss, Just to confirm, the SSO buttons aren’t displayed in development builds. Is there a way to test them, or is it okay to review this PR without adding any test records? Thanks. |
|
I commented this condition locally, but it doesn't seem to be reproducible for me on the latest main. video_2026-075-09_15-36-45.mp4 |
* main: (287 commits) Update Mobile-Expensify submodule version to 9.3.69-18 Update version to 9.3.69-18 Stop clearing routes on success to preserve map preview Auto-tighten eslint-seatbelt baseline Update Mobile-Expensify submodule version to 9.3.69-17 Update version to 9.3.69-17 Update Mobile-Expensify submodule version to 9.3.69-16 Update version to 9.3.69-16 Auto-tighten eslint-seatbelt baseline fix seatbelt Revert SearchUIUtilsTest.ts to match main Revert test change: restore shouldRedirectToExpensifyClassic in SearchUIUtilsTest revert manually adding seatbelt update Revert isTripStopped change per review feedback chore: add USD useSubStep files to eslint-seatbelt baseline Remove stale shouldRedirectToExpensifyClassic property from test Remove unrelated change to SearchUIUtilsTest.ts fix: use named import from @libs/ErrorUtils in ConfirmationUBO Fix: remove shouldRedirectToExpensifyClassic from test call site Remove stale shouldRedirectToExpensifyClassic property from test ...
Hi @marufsharifi, a code review is totally fine here. We'll handle testing once it hits staging as SSO is tricky to verify. I have just made a simplification to the PR with a final commit |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
| isLoading={account?.isLoading} | ||
| isLoading={!!account?.isLoading || !!authToken} |
There was a problem hiding this comment.
@Julesssss, I’m not sure these are the right changes, as the issue seems to be related to the Continue button rather than the Use single sign-on. Could you let me know if I’m missing something?
There was a problem hiding this comment.
@Julesssss, have you had a chance to check this? Thanks.
There was a problem hiding this comment.
Hi @marufsharifi. Sorry, this was supposed to be a draft, didn't realise it was assigned to you and you're right about the change. My bad. Will tag you when I resubmit it.
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product
Explanation of Change
When signing in with SSO on Android, the SSO button on the sign-in page shows a loading indicator during the SAML flow, but the loading indicator was removed before the user is navigated away to the main app. This looked buggy.
Fixed Issues
$ #90089
PROPOSAL: N/A
Tests
Offline tests
N/A — SSO requires network.
QA Steps
Same as Tests.
InternalQA: We'll verify this on staging, SSO doesn't work on AdHoc builds
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