-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Refactor keyboard observer on iOS #4360
Conversation
object:nil]; | ||
} | ||
|
||
[self->_listeners setObject:listener forKey:listenerId]; | ||
if (self->_state == UNKNOWN) { | ||
[self recognizeInitialKeyboardState]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the initial keyboard state correct after these changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always override the state in keyboardWillChangeFrame before sending any event
This is awesome @kmagiera! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I can confirm this PR fixes the issue with missing height update on keyboard change. Here are the logs from toggling between English and Emoji keyboard. Note that for each change, there are exactly two updates in an interval of one frame (16 ms):
- As for the issue with opening a modal, it looks like after these changes useAnimatedKeyboard-based KeyboardAvoidingView behaves identically to the one from React Native so I guess this is also solved. Also, here are the logs from opening a modal while keyboard is open:
- I can also confirm this PR fixes the issue with non-zero height after keyboard hides completely. Previously, I was able to reproduce it consistently using the following conditional breakpoint:
- Because of the shape of the animation curve, sometimes (in particular, at the beginning and at the end of animation) as well as the precision of the measurement, keyboard height doesn't change in up to 5 subsequent frames. In such case (when both height and state don't change), we could skip calling listeners on the native side.
Nagranie.z.ekranu.2023-04-17.o.12.57.38.mov
- I know this is outside of scope of this PR but why we need to have two identical listeners in
useAnimatedKeyboard.ts
?
react-native-reanimated/src/reanimated2/hook/useAnimatedKeyboard.ts
Lines 25 to 29 in 2e47c4a
listenerId.current = subscribeForKeyboardEvents((state, height) => { | |
'worklet'; | |
keyboardEventData.state.value = state; | |
keyboardEventData.height.value = height; | |
}, options); |
react-native-reanimated/src/reanimated2/hook/useAnimatedKeyboard.ts
Lines 37 to 41 in 2e47c4a
listenerId.current = subscribeForKeyboardEvents((state, height) => { | |
'worklet'; | |
keyboardEventData.state.value = state; | |
keyboardEventData.height.value = height; | |
}, options); |
- Unfortunately, KeyboardAvoidingViewExample crashes on reload (Example app, iOS). It's not a regression as it also does crash on the main branch but it's certainly worth looking into, especially since reloading works fine on other examples. Here's the stack trace:
## Summary This PR refactors the logic in REAKeyboardEventObserver to address some number of different issues: 1) no change notification when keyboard size changes due to change in language or showing suggestions bar 2) issue with keyboard height notification when opening modal (see software-mansion#4339) 3) issue with non-zero height after keyboard hides completely. Here is the list of the main thing that this refactor changes: 1) We no longer use willShow/willHide/didShow/didHide notifications for keyboard and only use willChangeFrame which is the most reliable one and triggers also when the keyboard updates 2) We no longer use a technique where the keyboard size is determined based on the frame of the keyboard view. This was problematic as that frame would sometimes reflect a one-frame delayed size which resulted in the animation not coming down to 0 or not updating at all when immediately hidden or appear (by a modal). Instead, we use UIView animation to animate a complementary view that is placed outside of the screen visible area. We use UIView animation to animate height of that invisible view and then use display link to access presentation layer in order to read up-to-date height. There are few gatchas related to the new technique used: 1) Because we only look at frame change notification we need to properly determine the state of the keyboard. We do it based on the target height. If it is 0 then we consider the state to be either closing or closed – we select the final state based on whether there is animation running (closed state when no animation, or closing when we are animating). This prevents us from changing state to closing when keyboard is only becoming shorter (e.g. when text suggestion bar is closed). We handle opening/open state in similar way. 2) Due to the UIView animation technique we need to check presentationLayer.animationKeys to determine if the layer runs the animation. If it doesn't then we extract height from the view frame and not from the presentationLayer frame, as the presentationLayer may not have an up-to-date information (it will be delayed by one frame). ## Test plan Use example from software-mansion#4339 and animated keyboard screen from Example app. Open/close text suggestion bar to see how the app reacts when keyboard is changing height and not just opening/closing.
Summary
This PR refactors the logic in REAKeyboardEventObserver to address some number of different issues:
Here is the list of the main thing that this refactor changes:
There are few gatchas related to the new technique used:
Test plan
Use example from #4339 and animated keyboard screen from Example app. Open/close text suggestion bar to see how the app reacts when keyboard is changing height and not just opening/closing.