Repository navigation
[CP Staging] Fix emoji picker bounce when keyboard opens/closes on Android - #94106
Conversation
…oard animation Co-authored-by: truph01 <truph01@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. Checks I ran locally (all passing): |
|
🤖 I analyzed the failing checks — there's no code fix needed. The only failing check is Every substantive check has passed: To get the checklist green: the human co-author should fill in the |
|
@MelvinBot Mark this PR as ready for review |
|
@shubham1206agra @truph01 One of you needs to 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] |
![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: ac438577db
ℹ️ 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".
| // on isKeyboardShown removes/re-adds the padding only after the keyboard animation finishes, which abruptly changes | ||
| // the picker height a beat behind the modal motion and makes the picker bounce. | ||
| const {isKeyboardActive} = useKeyboardState(); | ||
| const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({addBottomSafeAreaPadding: !isKeyboardActive}); |
There was a problem hiding this comment.
This now removes the emoji picker's bottom safe-area padding whenever isKeyboardActive is true, but withKeyboardState sets that flag on every keyboardWillShow/keyboardDidShow without checking whether a soft keyboard actually has a positive height. In zero-height keyboard cases such as hardware keyboards, nothing covers the bottom inset, and because this modal opts into edge-to-edge bottom safe-area handling, this inner padding is the only protection against the home/navigation indicator; the bottom row can end up underneath it while the search input is focused. Please gate this on a positive keyboard height as well as activity.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🤖 truph01 asked me to check this. The concern is valid as a code-level regression, though its real-world trigger is uncertain.
Why it's valid: the two flags come from different sources in withKeyboardState.tsx:
isKeyboardActiveis an event-driven toggle — settrueonkeyboardWillShow/keyboardDidShowwith no height check (L44-L51).isKeyboardShownis derived askeyboardHeight !== 0(L91).
So when a show event fires with zero (or, on Android, sub-inset) height, isKeyboardActive is true while isKeyboardShown is false. Since the picker opts into edge-to-edge mode (EmojiPicker.tsx:277) and the modal therefore adds no bottom padding itself (BaseModal.tsx:265), the inner padding at EmojiPicker.tsx:247 is the only protection for the home/nav indicator — and it would be dropped in that case.
The caveat: whether a hardware keyboard actually emits a show event with height: 0 is platform/react-native-keyboard-controller runtime behavior I can't confirm from code alone. On iOS, a hardware keyboard often suppresses the soft keyboard entirely (no show event → both flags false → no bug). So this is a real edge-case gap, but its actual frequency is unverified — best confirmed on a device with a hardware keyboard attached.
Minimal fix that keeps the animation-sync benefit: gate on positive height too, using keyboardActiveHeight (already set alongside isKeyboardActive on willShow — L52, exposed at L93):
const {isKeyboardActive, keyboardActiveHeight} = useKeyboardState();
const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({
addBottomSafeAreaPadding: !(isKeyboardActive && keyboardActiveHeight > 0),
});This preserves syncing the padding change with the keyboard animation (the bounce fix) while restoring the positive-height guard the old isKeyboardShown provided.
If you'd like, reply @MelvinBot implement this and I'll push the change — I'm an assignee so I can commit to this branch.
There was a problem hiding this comment.
@truph01 Let's leave this for now cause other component also relies on same logic
There was a problem hiding this comment.
@MelvinBot Yes, let's update to:
const isSoftKeyboardActive = isKeyboardActive && keyboardHeight > 0;
const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({addBottomSafeAreaPadding: !isSoftKeyboardActive});
There was a problem hiding this comment.
🤖 Done — pushed in 7582962.
The padding now keys on isSoftKeyboardActive instead of isKeyboardActive alone:
const { isKeyboardActive, keyboardHeight } = useKeyboardState();
const isSoftKeyboardActive = isKeyboardActive && keyboardHeight > 0;
const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({
addBottomSafeAreaPadding: !isSoftKeyboardActive,
});Checks: Prettier, lint-changed, and typecheck-tsgo all pass. The React Compiler check reports 3 pre-existing Cannot access refs during render errors at line 39 — these are present on main too, so this change introduces no regression.
Note:
shubham1206agramentioned other components rely on the same logic — worth a quick scan to confirm none of them need the matchingkeyboardHeight > 0guard.
There was a problem hiding this comment.
@truph01 Let's leave this for now cause other component also relies on same logic
Oh, I don't see this comment when asking Melvin to update the PR.
There was a problem hiding this comment.
@MelvinBot Let's revert the code change to something like:
const {isKeyboardActive} = useKeyboardState();
const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({addBottomSafeAreaPadding: !isKeyboardActive});
since other component also relies on same logic.
|
Android: Screen.Recording.2026-06-20.at.10.29.55.movIOS: Screen.Recording.2026-06-20.at.10.58.42.mov |
|
@mountiny Build please The behavior is still bit jumpy but it's better than previous behavior |
Co-authored-by: truph01 <truph01@users.noreply.github.com>
Co-authored-by: truph01 <truph01@users.noreply.github.com>
|
Done — reverted to the simpler form so it matches the logic other components use: const {isKeyboardActive} = useKeyboardState();
const bottomSafeAreaPaddingStyle = useBottomSafeSafeAreaPaddingStyle({addBottomSafeAreaPadding: !isKeyboardActive});Dropped the extra Verification
|
mountiny
changed the title
|
🚧 @mountiny has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
Reviewer Checklist
Screenshots/VideosXRecorder_20260621_01-2026-06-21.13_53_44.050.mp4 |
mountiny
left a comment
mountiny
left a comment
There was a problem hiding this comment.
Thank you, looks like its fixed
|
🚧 @mountiny 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! 🧪🧪
|
…dBounce [CP Staging] Fix emoji picker bounce when keyboard opens/closes on Android (cherry picked from commit 360d370) (cherry-picked to staging by mountiny)
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.4.16-2 🚀
Bundle Size Analysis (Sentry): |
|
🤖 No help site changes required. This PR is a pure visual/animation fix. The only change is in There is no change to any user-facing feature, setting, tab, label, button, or documented workflow — only the smoothness of an existing interaction. Nothing under If you believe a help article should be touched anyway, let me know which one and I'll draft it. |
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.4.16-5 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.4.17-0 🚀
Bundle Size Analysis (Sentry): |
|
🤖 No help site changes required. This PR is a purely internal bug fix for an Android UI animation-timing regression in the emoji picker. It swaps the keyboard signal that drives the picker's bottom safe-area padding ( There is no change to any user-facing feature, flow, setting, label, or behavior that is documented on the help site ( @truph01, if you believe a help site article does need updating here, let me know which behavior changed from a user's perspective and I'll draft the changes. |
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.4.17-3 🚀
|
1 similar comment
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.4.17-3 🚀
|
Explanation of Change
The emoji picker bounced up/down on Android (3-button navigation mode) when the search keyboard opened and closed. This was a regression from #88578, which added a keyboard-conditional bottom safe-area padding to the picker's inner
View:isKeyboardShownis derived fromkeyboardHeight !== 0, which is only set by thekeyboardDidShow/keyboardDidHidelisteners — i.e. it flips after the keyboard animation finishes. Meanwhile the modal slides smoothly with the keyboard viaavoidKeyboard(KeyboardAvoidingView). So the padding was added/removed in one discrete step a beat after the smooth modal motion, abruptly changing the picker's content height → the visible "jump then settle" in both directions.This PR keys the inner padding on
isKeyboardActiveinstead.isKeyboardActivetoggles onkeyboardWillShow/keyboardWillHide(animation start), the same signal the modal layer itself uses for its safe-area handling (BaseModal.tsx). The padding now changes in sync with the keyboard-avoidance animation, eliminating the out-of-sync height change that caused the bounce while preserving the original intent (no gap above the keyboard when it is shown).Fixed Issues
$ #94086
PROPOSAL: #94086 (comment)
Tests
// TODO: The human co-author must fill out the tests you ran before marking this PR as "ready for review"
// Please describe what tests you performed that validated your changes worked.
Offline tests
Same as Tests.
QA Steps
// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
Same as Tests.
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))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