Skip to content

Commit

Permalink
fix: crash in KeyboardMovementObserver due to incorrect KVO removal (
Browse files Browse the repository at this point in the history
…#787)

## 📜 Description

Fixed a crash in `KeyboardMovementObserver` due to incorrect KVO
removal.

## 💡 Motivation and Context

Similar to
#768

Instead of using plain KVO we are using block-scoped KVO, mainly to
prevent random crashes, when we try to remove a KVO from a view that
doesn't have it.

I can not 100% gurantee, that crash will not happen again, because a
crash was very random and we can not reproduce it ourself (we just see
it in Sentry) - so basically I can not test it.

But I'm pretty confident, that a new approach will fix the issue, since
we will not accidentally remove KVO where it doesn't exist (because of
block-scoped approach) 🤞

Also new approach reduces code duplication (such as `if keyPath ==
"center"`) and reduces a number of code lines 👀

Fixes
#152 (comment)

## 📢 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 -->

### iOS

- use block-scoped KVO in `KeyboardMovementObserver`;

## 🤔 How Has This Been Tested?

Tested manually on iPhone 15 Pro (iOS 17.5).

## 📸 Screenshots (if appropriate):


https://github.com/user-attachments/assets/acb009a1-8c25-44c5-93ff-cf1ad0a48dc7

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
  • Loading branch information
kirillzyusko authored Jan 29, 2025
1 parent 6553bf6 commit 49d20fc
Showing 1 changed file with 40 additions and 56 deletions.
96 changes: 40 additions & 56 deletions ios/observers/KeyboardMovementObserver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class KeyboardMovementObserver: NSObject {
private var _windowsCount: Int = 0
private var prevKeyboardPosition = 0.0
private var displayLink: CADisplayLink?
private var hasKVObserver = false
private var interactiveKeyboardObserver: NSKeyValueObservation?
private var isMounted = false
// state variables
private var _keyboardHeight: CGFloat = 0.0
Expand Down Expand Up @@ -93,73 +93,57 @@ public class KeyboardMovementObserver: NSObject {
}

private func setupKVObserver() {
if hasKVObserver {
return
}
guard interactiveKeyboardObserver == nil, let view = keyboardView else { return }

interactiveKeyboardObserver = view.observe(\.center, options: .new) { [weak self] _, change in
guard let self = self, let changeValue = change.newValue else { return }

if keyboardView != nil {
hasKVObserver = true
keyboardView?.addObserver(self, forKeyPath: "center", options: .new, context: nil)
self.keyboardDidMoveInteractively(changeValue: changeValue)
}
}

private func removeKVObserver() {
if !hasKVObserver {
interactiveKeyboardObserver?.invalidate()
interactiveKeyboardObserver = nil
}

private func keyboardDidMoveInteractively(changeValue: CGPoint) {
// if we are currently animating keyboard -> we need to ignore values from KVO
if displayLink != nil {
return
}
// if keyboard height is not equal to its bounds - we can ignore
// values, since they'll be invalid and will cause UI jumps
if floor(keyboardView?.bounds.size.height ?? 0) != floor(_keyboardHeight) {
return
}

hasKVObserver = false
_keyboardView?.removeObserver(self, forKeyPath: "center", context: nil)
}
let keyboardFrameY = changeValue.y
let keyboardWindowH = keyboardView?.window?.bounds.size.height ?? 0
let keyboardPosition = keyboardWindowH - keyboardFrameY

// swiftlint:disable:next block_based_kvo
@objc override public func observeValue(
forKeyPath keyPath: String?,
of object: Any?,
change: [NSKeyValueChangeKey: Any]?,
context _: UnsafeMutableRawPointer?
) {
if keyPath == "center", object as? NSObject == _keyboardView {
// if we are currently animating keyboard -> we need to ignore values from KVO
if displayLink != nil {
return
}
// if keyboard height is not equal to its bounds - we can ignore
// values, since they'll be invalid and will cause UI jumps
if floor(keyboardView?.bounds.size.height ?? 0) != floor(_keyboardHeight) {
return
}
let position = CGFloat.interpolate(
inputRange: [_keyboardHeight / 2, -_keyboardHeight / 2],
outputRange: [_keyboardHeight, 0],
currentValue: keyboardPosition
) - KeyboardAreaExtender.shared.offset

guard let changeValue = change?[.newKey] as? NSValue else {
return
}
let keyboardFrameY = changeValue.cgPointValue.y
let keyboardWindowH = keyboardView?.window?.bounds.size.height ?? 0
let keyboardPosition = keyboardWindowH - keyboardFrameY

let position = CGFloat.interpolate(
inputRange: [_keyboardHeight / 2, -_keyboardHeight / 2],
outputRange: [_keyboardHeight, 0],
currentValue: keyboardPosition
) - KeyboardAreaExtender.shared.offset

if position == 0 {
// it will be triggered before `keyboardWillDisappear` and
// we don't need to trigger `onInteractive` handler for that
// since it will be handled in `keyboardWillDisappear` function
return
}
if position == 0 {
// it will be triggered before `keyboardWillDisappear` and
// we don't need to trigger `onInteractive` handler for that
// since it will be handled in `keyboardWillDisappear` function
return
}

prevKeyboardPosition = position
prevKeyboardPosition = position

onEvent(
"onKeyboardMoveInteractive",
position as NSNumber,
position / CGFloat(keyboardHeight) as NSNumber,
-1,
tag
)
}
onEvent(
"onKeyboardMoveInteractive",
position as NSNumber,
position / CGFloat(keyboardHeight) as NSNumber,
-1,
tag
)
}

@objc public func unmount() {
Expand Down

0 comments on commit 49d20fc

Please sign in to comment.