Skip to content

Commit d77e9eb

Browse files
authored
fix: trigger full event lifecycle when keyboard is syncing on Android (#1503)
## 📜 Description Fixed an issue when closing a `Modal` triggers undesired scrolling in `KeyboardAwareScrollView`. ## 💡 Motivation and Context The issue is because when `Modal` gets closed we dispatch only `onMove`/`onEnd` events. In `KeyboardAwareScrollView` in `onStart` method we have a lot of logic that computes current input position before keyboard animation, calculates offset etc. If we skip this stage, then `onMove`/`onEnd` may use values from previous keyboard interaction (so if keyboard was closed before and we scrolled for ~100px) we may scroll again, because values from `onStart` were not updated. Technically that problem could be fixed in `KeyboardAwareScrollView` with additional conditions, but we use `useKeyboardHandler` hook in so many places/components, so if we would use this approach we potentially might need to replicate this fix in `KeyboardAvoidingView` and other components (and lib users would also need to do that if they use this hook in their projects). So I think the correct fix is to fix inconsistency in native code. Funny enough, but the new `syncKeyboardPosition` now becomes very similar to `onKeyboardResized`, especially since both functions are used in `onApplyWindowInsets` and I think that in the future these methods can be unified. Moreover the condition with detection keyboard resize or double keyboard position synchronization also can be unified and as a result we can make the codebase simpler. But this is the next step. First of all we need to merge/release this PR and keep an eye on possible bug reports about new bugs that caused by these changes 👀 Closes #1501 #1502 ## 📢 Changelog <!-- High level overview of important changes --> <!-- For example: fixed status bar manipulation; added new types declarations; --> <!-- If your changes don't affect one of platform/language below - then remove this platform/language --> ### Android - send `onStart` event too from `syncKeyboardPosition` function; ## 🤔 How Has This Been Tested? Tested manually on Pixel 7 Pro (API 36, real device). ## 📸 Screenshots (if appropriate): |Before|After| |-------|-----| |<video src="https://github.com/user-attachments/assets/0a08edd8-ed27-4ee4-9b77-c469516d9e85">|<video src="https://github.com/user-attachments/assets/ab430e99-202f-4595-b8c1-225fc7378b11">| ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
1 parent a2f7e7e commit d77e9eb

1 file changed

Lines changed: 7 additions & 2 deletions

File tree

android/src/main/java/com/reactnativekeyboardcontroller/listeners/KeyboardAnimationCallback.kt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -393,8 +393,13 @@ class KeyboardAnimationCallback(
393393
"KeyboardController::" + if (!isKeyboardVisible) "keyboardDidHide" else "keyboardDidShow",
394394
getEventParams(keyboardHeight),
395395
)
396-
// dispatch `onMove` to update RN animated value and `onEnd` to indicate that transition finished
397-
listOf(KeyboardTransitionEvent.Move, KeyboardTransitionEvent.End).forEach { eventName ->
396+
// dispatch full lifecycle of onStart/onMove/onEnd events to update RN animated value and
397+
// keep imperative hooks like `useKeyboardHandler` in sync
398+
listOf(
399+
KeyboardTransitionEvent.Start,
400+
KeyboardTransitionEvent.Move,
401+
KeyboardTransitionEvent.End,
402+
).forEach { eventName ->
398403
context.dispatchEvent(
399404
eventPropagationView.id,
400405
KeyboardTransitionEvent(

0 commit comments

Comments
 (0)