Repository navigation
Keep Plaid in portrait on Android phones so its inputs stay visible - #105598
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] |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
|
Plaid's LinkActivity locks itself to portrait. We override that to UNSPECIFIED so Plaid follows the device orientation, but on a phone in landscape the keyboard leaves Plaid's WebView about 126dp tall, which Plaid's own header fills, so the login inputs sit under the keyboard. Apply the override only when smallestScreenWidthDp is 600 or more, the same tablet threshold DeviceInfo.isTablet() uses on Android. Phones fall back to Plaid's portrait lock, tablets keep landscape Plaid.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f15de4bf37
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
LinkActivity declares smallestScreenSize in configChanges, so folding or unfolding across the 600dp threshold while Plaid is open does not recreate it and onActivityCreated never runs again. Keep a weak reference to the live LinkActivity and re-apply the orientation from Application.onConfigurationChanged. Phones now get an explicit PORTRAIT instead of relying on Plaid's own lock, so a session opened unfolded locks when the device is folded. Android 8.0 is skipped because a translucent activity cannot request a fixed orientation there, which is why Plaid skips it as well.
|
@hoangzinh PR is ready |
| } | ||
| // Android 8.0 throws when a translucent activity requests a fixed orientation; Plaid skips it there too. | ||
| if (Build.VERSION.SDK_INT != Build.VERSION_CODES.O) { | ||
| activity.requestedOrientation = ActivityInfo.SCREEN_ORIENTATION_PORTRAIT |
There was a problem hiding this comment.
@mukhrr why don't we use Plaid's requestedOrientation? (I recall Plaid already set ActivityInfo.SCREEN_ORIENTATION_PORTRAIT)
There was a problem hiding this comment.
hmm yeah, it makes sense to me
| override fun onActivityDestroyed(activity: Activity) { | ||
| if (plaidLinkActivity?.get() === activity) { | ||
| plaidLinkActivity = null | ||
| } | ||
| } |
There was a problem hiding this comment.
Those LOC doesn't have in your proposal. Can you explain it a bit here?
There was a problem hiding this comment.
these are from Codex comment above and i think it's valid concern. LinkActivity has smallestScreenSize in configChanges, so fold/unfold doesn't recreate it and onActivityCreated won't run again. that's why we keep ref and re-apply in onConfigurationChanged
Plaid sets its orientation before super.onCreate(), so LinkActivity already holds Plaid's value when onActivityCreated runs. Save it there and restore it below 600dp instead of hardcoding portrait and duplicating Plaid's API 26 check.
|
|
||
| private fun applyPlaidOrientation(activity: Activity, config: Configuration) { | ||
| activity.requestedOrientation = | ||
| if (config.smallestScreenWidthDp >= 600) ActivityInfo.SCREEN_ORIENTATION_UNSPECIFIED else plaidRequestedOrientation |
There was a problem hiding this comment.
Should we do nothing in the else clause? @mukhrr
There was a problem hiding this comment.
@hoangzinh no, I think it's needed for the fold case. on first open it's no-op, value is already Plaid's. but if user unfolds (we set UNSPECIFIED) and then folds back, without else it stays UNSPECIFIED and we get original bug again on phone
There was a problem hiding this comment.
@mukhrr I got this AI review on hybrid repo
On Android 16+ (Android/build.gradle:226 targets SDK 36), when LinkActivity is first created in an sw600dp+ window, the platform ignores Plaid's portrait request and getRequestedOrientation() is normalized to SCREEN_ORIENTATION_UNSPECIFIED. If an unfolded foldable is then folded below 600dp, LinkActivity is not recreated because Android/AndroidManifest.xml:277 handles smallestScreenSize, so this branch reapplies UNSPECIFIED; Plaid remains rotatable in the phone-sized window and the landscape-keyboard regression returns. The small-screen branch should request portrait explicitly rather than replaying the value captured while the request was ignored.
It seems we need to revert to your original version.
There was a problem hiding this comment.
I agree with the review. I’ll hold the merge until we have a follow-up for it.
|
Opening PR in Hybrid repo |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-11-05.at.21.13.09.movAndroid: mWeb ChromeiOS: HybridAppScreen.Recording.2026-11-05.at.21.33.22.moviOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-11-05.at.21.26.40.mov |
|
@mollfpr the hybrid PR is here https://github.com/Expensify/Mobile-Expensify/pull/14216 |
mollfpr
left a comment
mollfpr
left a comment
There was a problem hiding this comment.
Looks good to me! Thanks!
This reverts commit bd672b6.
|
@mollfpr I thnk we are good to merge now. thanks |
|
🚧 mollfpr has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mollfpr in version: 9.5.34-0 🚀
|
|
This PR failing because of the issue #106342 |
@hoangzinh is hybrid app PR https://github.com/Expensify/Mobile-Expensify/pull/14216 merged yet? |
|
ah nope. @mollfpr could you help to review and merge that hybrid PR? |
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.5.34-1 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
On Android phones we override Plaid's own portrait lock, and in landscape the keyboard leaves Plaid's WebView too short to show its login inputs. The override now applies to tablets only, so phones keep Plaid in portrait and the inputs stay visible.
Fixed Issues
$ #102562
PROPOSAL: #102562 (comment)
Tests
Android phone, account without a wallet bank account.
Offline tests
Plaid needs a connection to load, so the flow cannot run offline. The change only sets the orientation of Plaid's activity when it is created and has no network surface.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged theExpensify/designteam so 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
102562-android-app.mp4
Android: mWeb Chrome
102562-android-web.mp4
iOS: Native
102562-ios-app.mp4
iOS: mWeb Safari
102562-ios-web.mp4
MacOS: Chrome / Safari
102562-after-web.mp4