kirillzyusko / react-native-keyboard-controller

Keyboard manager which works in identical way on both iOS and Android
https://kirillzyusko.github.io/react-native-keyboard-controller/
MIT License
1.79k stars 81 forks source link

iOS crash when native alert is shown while keyboard is visible #175

Closed owinter86 closed 1 year ago

owinter86 commented 1 year ago

Describe the bug iOS crashes when using a native alert while the keyboard is shown, I can confirm this only happens if the app is wrapped in the KeyboardProvider, removing the Keyboard provider from the snippet below will avoid the crash on ios.

Code snippet

import { Alert, Button, TextInput, View } from "react-native";
import { KeyboardProvider } from "react-native-keyboard-controller";

export function App() {
  return (
    <KeyboardProvider>
      <TestView />
    </KeyboardProvider>
  );
}

function TestView() {
  return (
    <View style={{ flex: 1, alignItems: "center", justifyContent: "center" }}>
      <TextInput placeholder="PLACHOLDER TEXT" />
      <Button
        onPress={() => {
          Alert.alert("Hello");
        }}
        title="Show Alert"
      />
    </View>
  );
}

Repo for reproducing No repo provided as the code sample is simple enough to paste into a running env without having to checkout and build another repo.

To Reproduce Steps to reproduce the behavior:

  1. Select placeholder text to expose keyboard
  2. press show alert button
  3. see crash

Expected behavior Alerts should not crash the app when the keyboard is shown

Screenshots If applicable, add screenshots to help explain your problem.

Smartphone (please complete the following information):

kirillzyusko commented 1 year ago

Thanks @owinter86!

I'll have a look on that!

kirillzyusko commented 1 year ago

@owinter86 it seems like I can not reproduce it 😔

https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/e08ab307-637a-458f-a1e4-c3c4591c65ed

I've tried to reproduce it on simulator. Should I try to use a real device?

P. S. I also tested app on iOS 16.2. Maybe 16.5 affects somehow? 🤔

owinter86 commented 1 year ago

I have created a new project and its not crashing, but is crashing with a specific project with just that simple example as the entry point. So I guess there is a third party package that is conflicting, lets close this for now and I will look into it more and give a more detailed issue if I find the clash/cause.

The errors I get in sentry are not helpful either C++ Exception - UISystemKeyboardDockController

kirillzyusko commented 1 year ago

@owinter86 One thing that I can suggest is to try to call setupKVObserver/removeKVObserver in different lifecycle methods. Try to replace KeyboardMovementObserver with this content:

//
//  KeyboardMovementObserver.swift
//  KeyboardController
//
//  Created by Kiryl Ziusko on 2.08.22.
//  Copyright © 2022 Facebook. All rights reserved.
//

import Foundation

@objc(KeyboardMovementObserver)
public class KeyboardMovementObserver: NSObject {
  // class members
  var onEvent: (NSString, NSNumber, NSNumber) -> Void
  var onNotify: (String, Any) -> Void
  // progress tracker
  private var _keyboardView: UIView?
  private var keyboardView: UIView? {
    let windowsCount = UIApplication.shared.windows.count

    if _keyboardView == nil || windowsCount != _windowsCount {
      _keyboardView = findKeyboardView()
      _windowsCount = windowsCount
    }

    return _keyboardView
  }

  private var _windowsCount: Int = 0
  private var prevKeyboardPosition = 0.0
  private var displayLink: CADisplayLink?
  private var keyboardHeight: CGFloat = 0.0
  private var hasKVObserver = false

  @objc public init(
    handler: @escaping (NSString, NSNumber, NSNumber) -> Void,
    onNotify: @escaping (String, Any) -> Void
  ) {
    onEvent = handler
    self.onNotify = onNotify
  }

  @objc public func mount() {
    NotificationCenter.default.addObserver(
      self,
      selector: #selector(keyboardWillDisappear),
      name: UIResponder.keyboardWillHideNotification,
      object: nil
    )
    NotificationCenter.default.addObserver(
      self,
      selector: #selector(keyboardWillAppear),
      name: UIResponder.keyboardWillShowNotification,
      object: nil
    )
    NotificationCenter.default.addObserver(
      self,
      selector: #selector(keyboardDidAppear),
      name: UIResponder.keyboardDidShowNotification,
      object: nil
    )
    NotificationCenter.default.addObserver(
      self,
      selector: #selector(keyboardDidDisappear),
      name: UIResponder.keyboardDidHideNotification,
      object: nil
    )
  }

  private func setupKVObserver() {
    if hasKVObserver {
      return
    }

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

  private func removeKVObserver() {
    if !hasKVObserver {
      return
    }

    hasKVObserver = false
    _keyboardView?.removeObserver(self, forKeyPath: "center", context: nil)
  }

  // swiftlint:disable:next block_based_kvo
  @objc override public func observeValue(
    forKeyPath keyPath: String?,
    of object: Any?,
    change: [NSKeyValueChangeKey: Any]?,
    context _: UnsafeMutableRawPointer?
  ) {
    // swiftlint:disable:next force_cast
    if keyPath == "center", object as! NSObject == _keyboardView! {
      // if we are currently animating keyboard or keyboard is not shown yet -> we need to ignore values from KVO
      if displayLink != nil || keyboardHeight == 0.0 {
        return
      }

      // swiftlint:disable:next force_cast
      let keyboardFrameY = (change?[.newKey] as! NSValue).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
      )

      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
      }

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

  @objc public func unmount() {
    // swiftlint:disable:next notification_center_detachment
    NotificationCenter.default.removeObserver(self)
  }

  @objc func keyboardWillAppear(_ notification: Notification) {
    if let keyboardFrame: NSValue = notification.userInfo?[UIResponder.keyboardFrameEndUserInfoKey] as? NSValue {
      let keyboardHeight = keyboardFrame.cgRectValue.size.height
      self.keyboardHeight = keyboardHeight

      var data = [AnyHashable: Any]()
      data["height"] = keyboardHeight

      onEvent("onKeyboardMoveStart", Float(keyboardHeight) as NSNumber, 1)
      onNotify("KeyboardController::keyboardWillShow", data)

      setupKeyboardWatcher()
    }
  }

  @objc func keyboardWillDisappear() {
    var data = [AnyHashable: Any]()
    data["height"] = 0

    onEvent("onKeyboardMoveStart", 0, 0)
    onNotify("KeyboardController::keyboardWillHide", data)

    setupKeyboardWatcher()
    removeKVObserver()
  }

  @objc func keyboardDidAppear(_ notification: Notification) {
    if let keyboardFrame: NSValue = notification.userInfo?[UIResponder.keyboardFrameEndUserInfoKey] as? NSValue {
      let keyboardHeight = keyboardFrame.cgRectValue.size.height
      self.keyboardHeight = keyboardHeight

      var data = [AnyHashable: Any]()
      data["height"] = keyboardHeight

      onEvent("onKeyboardMoveEnd", keyboardHeight as NSNumber, 1)
      onNotify("KeyboardController::keyboardDidShow", data)

      removeKeyboardWatcher()
      setupKVObserver()
    }
  }

  @objc func keyboardDidDisappear() {
    var data = [AnyHashable: Any]()
    data["height"] = 0

    keyboardHeight = 0.0

    onEvent("onKeyboardMoveEnd", 0 as NSNumber, 0)
    onNotify("KeyboardController::keyboardDidHide", data)

    removeKeyboardWatcher()
  }

  @objc func setupKeyboardWatcher() {
    // sometimes `will` events can be called multiple times.
    // To avoid double re-creation of listener we are adding this condition
    // (if active link is present, then no need to re-setup a listener)
    if displayLink != nil {
      return
    }

    displayLink = CADisplayLink(target: self, selector: #selector(updateKeyboardFrame))
    displayLink?.preferredFramesPerSecond = 120 // will fallback to 60 fps for devices without Pro Motion display
    displayLink?.add(to: .main, forMode: .common)
  }

  @objc func removeKeyboardWatcher() {
    displayLink?.invalidate()
    displayLink = nil
  }

  // https://stackoverflow.com/questions/32598490/show-uiview-with-buttons-over-keyboard-like-in-skype-viber-messengers-swift-i
  func findKeyboardView() -> UIView? {
    var result: UIView?

    let windows = UIApplication.shared.windows
    for window in windows {
      if window.description.hasPrefix("<UITextEffectsWindow") {
        for subview in window.subviews {
          if subview.description.hasPrefix("<UIInputSetContainerView") {
            for hostView in subview.subviews {
              if hostView.description.hasPrefix("<UIInputSetHostView") {
                result = hostView as? UIView
                break
              }
            }
            break
          }
        }
        break
      }
    }
    return result
  }

  @objc func updateKeyboardFrame() {
    if keyboardView == nil {
      return
    }

    let keyboardFrameY = keyboardView?.layer.presentation()?.frame.origin.y ?? 0
    let keyboardWindowH = keyboardView?.window?.bounds.size.height ?? 0
    let keyboardPosition = keyboardWindowH - keyboardFrameY

    if keyboardPosition == prevKeyboardPosition || keyboardFrameY == 0 {
      return
    }

    prevKeyboardPosition = keyboardPosition
    onEvent("onKeyboardMove", keyboardPosition as NSNumber, keyboardPosition / CGFloat(keyboardHeight) as NSNumber)
  }
}

Maybe it'll help 🙂

owinter86 commented 1 year ago

So it seems like if "@shopify/react-native-skia": "0.1.183", is installed it causes the crash, even though that snippet is all the app is loading.

kirillzyusko commented 1 year ago

@owinter86 I've installed skia, installed pods, re-assembled project, and still didn't get a crash 😐

owinter86 commented 1 year ago

I can reproduce on a new expo app, and new react native cli app with just these three dependencies on both sim and device, you need to make sure that if you are on the sim that the actual keyboard is visible though, so you may need to cmd + K to see the keyboard after the input has been focussed.

    "@shopify/react-native-skia": "0.1.183",
    "react-native-keyboard-controller": "1.5.6",
    "react-native-reanimated": "~2.14.4"

edit: upgrading reanimated and skia seems to fix the issue, though. Some others may run into this issue also as those are the "recommended" versions for the current Expo sdk 48 release.

edit2: seems like the current solution is to either remove skia, or to upgrade to reanimated V3

kirillzyusko commented 1 year ago

@owinter86 that's very weird. I have following in yarn.lock:

image image

And the issue is not reproducible 🤯 Would you mind to push your repo to GitHub? I'd love to check it out and see what is the problem there 👀

owinter86 commented 1 year ago

sure here is the repo, and the video below showing the crash only when the keyboard is visible.

https://github.com/owinter86/keyboard-crash-rn

https://github.com/kirillzyusko/react-native-keyboard-controller/assets/2920419/f3dab2de-6561-4de9-b6b4-4216dc4fd0c2

kirillzyusko commented 1 year ago

@owinter86 I can confirm, the crash happens on iOS 16.5, but is not happening on iOS 16.2 🤷‍♂️

I'll investigate it more 👍

kirillzyusko commented 1 year ago

I found out a root problem. Need a little bit time to try various solutions to understand how would be better to fix it 👀

kirillzyusko commented 1 year ago

@owinter86 may I kindly ask you to test https://github.com/kirillzyusko/react-native-keyboard-controller/pull/178 and say whether it fixes the problem or not?

owinter86 commented 1 year ago

@kirillzyusko I can confirm this fixes the issue, nice work.

kirillzyusko commented 1 year ago

Thanks @owinter86

I'm going to do more tests on more devices and if everything is alright (which is most likely), then I'll merge this PR and prepare a new release quite soon (want to include some other fixes as well).

In a meantime you can patch-package your dependency or use library from the branch of this PR 🙂

kirillzyusko commented 1 year ago

Merged and published under 1.5.7 version 😊

In case if issue re-occurs - feel free to submit a new issue 👀