Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: set shared values in layout effects #838

Merged

Conversation

QichenZhu
Copy link
Contributor

@QichenZhu QichenZhu commented Feb 26, 2025

📜 Description

Fixed KeyboardAvoidingView not updating if the keyboard closes before binding event handlers.

💡 Motivation and Context

There’s a short timing gap after KeyboardAvoidingView initializes and before it listens for keyboard changes. If the keyboard closes during this gap, it isn’t aware that the keyboard has already closed.

Closes #836.

Related to:
$ Expensify/App#56156
PROPOSAL: Expensify/App#56156 (comment)

📢 Changelog

JS

  • set shared values in useLayoutEffect

🤔 How Has This Been Tested?

Tested locally on OnePlus 8 Pro Android 13.

📸 Screenshots (if appropriate):

Before After
react-native-keyboard-controller-android.mp4
after.mp4

📝 Checklist

  • CI successfully passed
  • I added new mocks and corresponding unit-tests if library API was changed

@kirillzyusko kirillzyusko self-assigned this Feb 26, 2025
@kirillzyusko kirillzyusko added 🐛 bug Something isn't working 🤖 android Android specific KeyboardAvoidingView 🧪 Anything related to KeyboardAvoidingView component labels Feb 26, 2025
Copy link
Contributor

📊 Package size report

Current size Target Size Difference
169866 bytes 169847 bytes 19 bytes 📈

@QichenZhu
Copy link
Contributor Author

Checks have passed, but I noticed two e2e tests are flaky on iOS 16:

They failed on the first try and passed when reran.

Are they known issues or related to this PR?

@kirillzyusko
Copy link
Owner

Are they known issues or related to this PR?

This is a known issue. Simulator on first boot works really slow, but after 15-20 minutes becomes working normally (this is why I added test repeat). So yeah, ignore it 😅

May I ask you to attach videos from your reproduction app, how it was before and how it looks after?

@QichenZhu QichenZhu marked this pull request as ready for review February 27, 2025 07:37
@kirillzyusko kirillzyusko merged commit 2b21634 into kirillzyusko:main Feb 27, 2025
12 checks passed
@kirillzyusko
Copy link
Owner

@QichenZhu do you need a new release? Asking because in 1.16 release I did some modification to how selection coordinates get calculated on Android (and in Expensify we heavily rely on those coordinates). And if you update the lib to the latest version, then potentially you'll need to check that part as well, to be sure it works as before (but it's out of the scope of the initial issue).

So asking how do you want to fix that in Expensify? Via patch or via lib update?

@QichenZhu
Copy link
Contributor Author

Thank you so much @kirillzyusko for the quick action on this.

I believe the plan is to upgrade the library, as mentioned in @roryabraham's comment:

we'll also need to upgrade react-native-keyboard-controller in this repo.

@kirillzyusko
Copy link
Owner

Okay @QichenZhu

I'll prepare release today 👍

@kirillzyusko
Copy link
Owner

Published 1.16.6 😌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 android Android specific 🐛 bug Something isn't working KeyboardAvoidingView 🧪 Anything related to KeyboardAvoidingView component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KeyboardAvoidingView on Android keeps padding after keyboard closes
2 participants