Fix login page shows briefly when open public room as anon user#65225
Conversation
|
@hoangzinh 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 ChromeScreen.Recording.2025-07-01.at.22.50.11.android.chrome.moviOS: HybridAppiOS: mWeb SafariScreen.Recording.2025-07-01.at.22.52.09.ios.safari.movMacOS: Chrome / SafariScreen.Recording.2025-07-01.at.22.16.12.web.movMacOS: DesktopCan't test because we're unable to open non-login URL in Desktop |
|
Note: We're unable to open public links in Hybrid App after signed out. This issue will be fixed here #64071 |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
The iOS HybridApp hangs on the splash screen. Seems related to this PR. |
|
@QichenZhu do you have reproducible steps? It works fine on my local Screen.Recording.2025-07-02.at.20.13.01.mov |
|
@hoangzinh, just fresh install the HybridApp and open it. The standalone app works fine. |
|
Thank you @QichenZhu |
|
In another PR, we recently added this new logic Lines 208 to 211 in 32f0755 That mean in HybridApp, it won't trigger this line Line 213 in 32f0755 Hence, Line 118 in 32f0755 |
|
To fix this issue we can either:
|
|
Hi all 👋 I also noticed that the current main HybridApp is unusable due to the hanging splashscreen Also I think this PR should be tested on HybridApp too to check if it's working fine 🙏 |
|
I'm going to revert as it's blocking HybridApp testing and development. Please test against HybridApp when preparing to ship it again. |
|
@war-in @Julesssss Just wanna clarify again, this issue was not reproducible in this git branch previously. And only reproducible after merging this PR #64786 as I found here #65225 (comment). => It's the reason why we couldn't catch this issue while working on this PR #65225 (comment) |
|
Yeah makes sense. I had to revert in this case as it was the much simpler option at the time. |
|
Agreed @Julesssss @bernhardoj, please create a new PR when you're available. |
|
New PR is ready |
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.1.75-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.1.76-5 🚀
|
Explanation of Change
Fixed Issues
$ #64638
PROPOSAL: #64638 (comment)
Tests
Same as QA Steps
Offline tests
Same as QA Steps
QA Steps
Prerequisite: logged out
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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.mp4
Android: mWeb Chrome
android.mweb.mp4
iOS: Native
ios.mp4
iOS: mWeb Safari
ios.mweb.mp4
MacOS: Chrome / Safari
web.mp4
MacOS: Desktop
desktop.mp4