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

useAnimatedKeyboard fixes (iOS) #6755

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mhoran
Copy link

@mhoran mhoran commented Nov 24, 2024

Summary

fix: return correct height for detached keyboard on iOS (fixes #5584)
fix: don't clobber keyboard height when keyboard did not animate on iOS
fix: support cross-fade transitions in useAnimatedKeyboard on iOS (fixes #6440)
fix: convert keyboard position to window coordinate space on iOS (Stage Manager compatibility)
fix: don't prematurely update keyboard height during dismissal on iOS
fix: don't jump to height when iOS keyboard is rapidly opened/closed (fixes #6727)

Test plan

Can be tested with the useAnimatedKeyboard example.

  1. Enable iPad for the example project.
  2. Launch the useAnimatedKeyboard example.
  3. Open the keyboard, then undock or float the keyboard. The view should not transform when undocked or floating.
  4. Dock the keyboard
  5. Hide the keyboard, note that the view should now be transformed by the keyboard height
  6. Enable Reduce Motion in Accessibility, and Prefers Cross-Fade Transitions
  7. Open the keyboard, note that he transform should work as expected

@mhoran mhoran changed the title Fix use animated keyboard useAnimatedKeyboard fixes (iOS) Nov 24, 2024
@mhoran mhoran force-pushed the fix-use-animated-keyboard branch 3 times, most recently from bc78384 to 606096c Compare November 26, 2024 04:54
Previously, when the keyboard was detached, useAnimatedKeyboard returned
a height corresponding to the height of a docked keyboard. However, when
the keyboard is floating, undocked or split, the height should be 0.

Listen for the UIKeyboardWillHideNotification notification to determine
if the keyboard has been undocked. We must now listen for show and hide
notifications instead of UIKeyboardWillChangeFrameNotification, as the
show/hide notifications are dispatched after the change frame
notification.

Adapted from the initial work in
software-mansion#5710.
Listening for hide is simpler than calculating whether the keyboard is
detached, compatible with Stage Manager, and maintains compatibility
with interactive keyboard dismissal.
When processing a keyboard frame, if the keyboard did not animate to the
target height (e.g. when closing the software keyboard on a device with
a physical keyboard), the height would be set to the wrong value in
updateKeyboardFrame on the subsequent update.

Instead of relying on the measuringView, which refers to the closing
animation in such cases, return _targetKeyboardHeight, which is also
returned in keyboardWillChangeFrame.
On iOS, when Prefer Cross-Fade Transitions is enabled, the keyboard
position and height is reported differently (0 instead of Y position
value matching height of the frame.) Override the initial and target
keyboard height when this preference is enabled.
The keyboard frame passed to keyboardWillChangeFrame is in the screen's
coordinate space [1]. Convert to the window's coordinate space to
support Stage Manager.

https://developer.apple.com/documentation/uikit/uikeyboardframeenduserinfokey?language=objc
@mhoran mhoran force-pushed the fix-use-animated-keyboard branch from 606096c to 3c2d160 Compare November 26, 2024 23:42
@tomekzaw
Copy link
Member

tomekzaw commented Nov 27, 2024

Hi @mhoran, thanks for another PR! Someone from Reanimated team will review this PR shortly.

Would you mind giving us a bit more context on why exactly some of the changes were made (e.g. what's the reason to change UIKeyboardWillChangeFrameNotification to UIKeyboardWillShowNotification)? This would facilitate our review process very much. Thanks in advance!

When the keyboard is dismissed via the close button or is undocked, the
interactive dismissal observer will be notified of the geometry change
before the show/hide notification is received.

Bail out before updating the keyboard height if the reported keyboard
height is 0 or if the keyboard is undocked (keyboard width != window
width.)

Note that this still allows some updates through when they shouldn't be,
but it's better than before, especially for floating keyboards.
@mhoran
Copy link
Author

mhoran commented Nov 28, 2024

Sure thing! I tried to leave detailed notes in my commit messages for posterity, but happy to provide further context as well.

Regarding UIKeyboardWillChangeFrameNotification vs show/hide notifications, initially I tried to take an approach similar to the one in #5710. While I did find a working solution (checking the endFrame only, which worked with interactive dismissal), I ran into issues with this approach in split view as well as Stage Manager. Dealing with all the different coordinate systems was a real pain, and I figured the code would be so complex that it would end up regressing in the future.

I found that the most reliable way to determine if the keyboard has been detached is to listen for UIKeyboardWillHideNotification. This required reworking things a bit to use UIKeyboardWillShowNotification and UIKeyboardWillHideNotification instead of UIKeyboardWillChangeFrameNotification, due to ordering (UIKeyboardWillChangeFrameNotification fires before the show / hide notifications.)

However, this approach also has the benefit of simplifying the logic in the change handler. I was able to remove the logic that tried to guess whether the keyboard was opening or closing -- which I found to be inaccurate at times, especially when a hardware keyboard is attached (due to the autosuggest dock.) The OPENING and CLOSING states are now set only in the appropriate notification handlers (or overridden in updateKeyboardHeightDuringInteractiveDismiss -- more on that later.)

I fixed a few more issues along the way.

  1. There was an issue with the hardware keyboard autosuggest bar not being respected, due to the notification handler expecting there to be a measurement frame with accurate height at all times. However, that's not true if the event handler doesn't happen to run at the exact moment a keyboard animation is running. I found this to be unnecessary complexity, since _targetKeyboardHeight can be used instead.
  2. I fixed an issue with the calculated height when the user has enabled Reduced Motion (Accessibility) and Prefers Cross-Fade Transitions. The implemented fix is the same that is in React Native.
  3. The coordinate space of keyboard frame notifications are in the screen's coordinate space. These need to be converted to the window's coordinate space to work correctly with Stage Manager. Without 3c2d160, the height returned by useAnimatedKeyboard will only be correct if an app is full screen when Stage Manager is enabled. Otherwise, the height will not take into consideration the position of the window within the stage.
  4. I found that sometimes the wrong height would be returned -- often a quick jump to 0, then back to the correct height -- during animations, due to an issue with updateKeyboardHeightDuringInteractiveDismiss. I added a guard to make sure that the keyboard did not immediately go to 0 (this will be handled by keyboardWillHide) and corresponds to the user dismissing the keyboard with the close button or blurring the input. I also added a check to determine if the keyboard is floating (keyboard width != window width). These are not perfect solutions, but do make the animation less janky. I think someone with more iOS experience could come up with a more bullet proof solution (e.g. check if there is a touch somewhere in the main view), but I'm not sure how to do that myself.

There is a new API, Keyboard Layout Guide, which will make this all a lot simpler in the future. However, it was introduced in iOS 15. So we will have to live with this complexity until RN 0.76 is the oldest supported version for Reanimated.

Note that there remain some issues with undocked and split keyboard modes, due to incorrect start frame positions sent in notifications. Apple has removed these modes in new devices, so I didn't try to implement workarounds.

@mhoran
Copy link
Author

mhoran commented Nov 28, 2024

Here is a screen recording showing the improvements: https://drive.google.com/file/d/1EFDhYZWNMXhJIwNlj1_ugeVwe3FqZqVF/view?usp=drivesdk.

@mhoran mhoran force-pushed the fix-use-animated-keyboard branch from ddefb46 to b6c8364 Compare November 28, 2024 05:55
@mhoran
Copy link
Author

mhoran commented Nov 28, 2024

It seems I may also have fixed #6727 as I was cleaning things up in 9f6b546. When a hardware keyboard is connected and the autocorrect bar is on screen, the keyboard is considered "open". But when you show the on screen keyboard, it rapidly hides and then shows the keyboard -- inadvertently triggering the interactive dismissal logic. Cleaning up the interactive dismissal flags fixed this for the hardware keyboard case, and I believe it should fix the scenario described in #6727 as well.

@mhoran mhoran force-pushed the fix-use-animated-keyboard branch 2 times, most recently from bc3d884 to 22aee6a Compare November 28, 2024 16:12
@janicduplessis
Copy link
Contributor

I tested this in my app and can confirm it does fix the Prefer Cross Fade Transition issue (#6440)

@mhoran mhoran force-pushed the fix-use-animated-keyboard branch from 22aee6a to bc3d884 Compare December 18, 2024 20:18
@mhoran
Copy link
Author

mhoran commented Dec 18, 2024

There is an issue with iOS 18.2 when opening the on-screen keyboard from the autosuggest bar when a hardware keyboard is attached. Opening the on-screen keyboard in this way will not properly update the height returned by useAnimatedKeyboard. However, the issue is not related to these changes (I can reproduce without them.) The keyboard handlers receive bogus end frame information in the notification, so we do not know where the keyboard actually is.

@mhoran mhoran force-pushed the fix-use-animated-keyboard branch from bc3d884 to 0c778e8 Compare December 19, 2024 03:37
The interactive dismissal logic would get tripped up if the keyboard was
rapidly opened and closed, which can happen if a hardware keyboard is
used and the autosuggest dock appears and then disappears when the on
screen keyboard is opened.

Fixes software-mansion#6727.
@mhoran mhoran force-pushed the fix-use-animated-keyboard branch from 0c778e8 to 9f6b546 Compare December 19, 2024 15:03
@mhoran
Copy link
Author

mhoran commented Dec 23, 2024

I managed to fix the iOS 18.2 hardware keyboard issue by updating the keyboard height in response to UIKeyboardDidShowNotification and UIKeyboardDidHideNotification notifications. The keyboard does jump to the target height instead of animating when opening the software keyboard with a hardware keyboard attached, so hopefully we'll get a proper fix from Apple.

This has cleaned up keyboard state handling as well. Now state changes are handled only by the corresponding notification handlers, and updateKeyboardFrame is only responsible for animation tracking. That should be a bit less error prone in the future.

On a side note, I managed to get the keyboardLayoutGuide working with React Native, sort of. Unfortunately it relies heavily on constraints, and I think this may be fundamentally incompatible with React Native's layout model. Happy to share my learnings with anyone on the Reanimated team who is interested.

UIKeyboardDidShowNotification on iOS 18.2 returns an incorrect end frame
origin when opening the on-screen keyboard with a hardware keyboard
attached.

Instead of relying on this incorrect frame, return the height of the
keyboard view in response to UIKeyboardDidShowNotification and 0 in
response to UIKeyboardDidHideNotification.

State changes are now completely handled in response to notifications,
and updateKeyboardFrame is only responsible for animation tracking.
@mhoran mhoran force-pushed the fix-use-animated-keyboard branch from 7c9a7c0 to 5a6a72f Compare January 6, 2025 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants