Skip to content

Commit

Permalink
fix: instant keyboard hide causes KeyboardAvoidingView keeping bott…
Browse files Browse the repository at this point in the history
…om space (#539)

## 📜 Description

Fixed a case when `KeyboardAvoidingView` is not getting resized back
when keyboard is hidden instantly.

## 💡 Motivation and Context

The condition:

```tsx
if (e.duration === 0) {
  progress.value = e.progress;
  height.value = e.height;
}
```

prevented view to restore its initial position when keyboard is hidden
instantly. When such thing happens we have next events:

```bash
 LOG  onStart {"duration": 250, "eventName": "onKeyboardMoveStart", "height": 0, "progress": 0, "target": 7054} 1723043957135
 LOG  onEnd {"duration": 250, "eventName": "onKeyboardMoveEnd", "height": 0, "progress": 0, "target": -1} 1723043957142
```

So technically we had to set our `height`/`progress` to `0`. But we skip
it, because `event` actually has non-zero duration (it's 250ms).

We can't simply remove the code that I added before because it'll bring
old bug back, so I was thinking a lot on how to resolve the problem.

### 1️⃣ Add more conditions

The first idea was to modify condition to be like

```tsx
if (e.duration === 0 || e.target === -1) {}
```

However I don't like it because:
- it adds more complexity to the code;
- we have to remember about that case and use this condition in every
hook;
- this code doesn't feel cross-platform;
- I'm not sure if it's a correct fix, because we may have a new
situation, where this condition will lead to unexpected results;

So I started to think about other approaches.

### 2️⃣ Native code modification

At some of point of time I realized, that we expect `onEnd` to report an
actual keyboard height that is currently visible on the screen (at least
in `keyboardDidAppear` method). And if we rely on this fact then we can
remove the condition from `onEnd` handler.

So I decided to try to re-work the code and instead of re-forwarding the
data from a notification I decided to grab a real data from the
`keyboardVIew` (its real coordinates) and send them.

Turned out that this fixes old bug and doesn't introduce new one, so I
think I'll stick with this approach. Also all e2e tests passes without
any modification and I think it's a good sign that I do everything
properly 😅

> [!NOTE]
> It's also safe to read frame inside this particular handler, because
we know, that keyboard stopped its movement and has a final frame, so we
don't need to predict next frame based on a current one etc. - we can
simply read frame data and be sure it's exact number what user sees on
the screen.

A new fix for
#327

Also fixes: Expensify/App#46743 and
Expensify/App#46745

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

### JS

- removed `if (e.duration === 0)` condition in `KeyboardAvoidingView`
hook;

### iOS

- added new `framePositionInWindow` extension to `UIView`;
- take an actual keyboard height (based on `keyboardView`) in
`keyboardDidAppear` method.

## 🤔 How Has This Been Tested?

Tested manually on iPhone 15 Pro (iOS 17.4).

## 📸 Screenshots (if appropriate):

|Before|After|
|-------|-----|
|<video
src="https://github.com/user-attachments/assets/19b3ac39-bd8e-4d47-bc34-a6ba7f3478b3">|<video
src="https://github.com/user-attachments/assets/f7492e25-ccaa-42f0-870a-60eafe912a7d">|

I also checked that
#327
is not reproducible anymore.

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
  • Loading branch information
kirillzyusko authored Aug 8, 2024
1 parent 54dc6d7 commit 6861faf
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 18 deletions.
10 changes: 10 additions & 0 deletions ios/extensions/UIView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,13 @@ public extension UIView {
return superview?.convert(frame, to: rootView)
}
}

public extension Optional where Wrapped == UIView {
var framePositionInWindow: (Double, Double) {
let frameY = self?.layer.presentation()?.frame.origin.y ?? 0
let windowH = self?.window?.bounds.size.height ?? 0
let position = windowH - frameY

return (position, frameY)
}
}
12 changes: 7 additions & 5 deletions ios/observers/KeyboardMovementObserver.swift
Original file line number Diff line number Diff line change
Expand Up @@ -210,21 +210,24 @@ public class KeyboardMovementObserver: NSObject {

@objc func keyboardDidAppear(_ notification: Notification) {
if let keyboardFrame: NSValue = notification.userInfo?[UIResponder.keyboardFrameEndUserInfoKey] as? NSValue {
let (position, _) = keyboardView.framePositionInWindow
let keyboardHeight = keyboardFrame.cgRectValue.size.height
tag = UIResponder.current.reactViewTag
self.keyboardHeight = keyboardHeight
let duration = Int(
(notification.userInfo?[UIResponder.keyboardAnimationDurationUserInfoKey] as? Double ?? 0) * 1000
)
// always limit progress to the maximum possible value
let progress = min(position / self.keyboardHeight, 1.0)

var data = [AnyHashable: Any]()
data["height"] = keyboardHeight
data["height"] = position
data["duration"] = duration
data["timestamp"] = Date.currentTimeStamp
data["target"] = tag

onCancelAnimation()
onEvent("onKeyboardMoveEnd", keyboardHeight as NSNumber, 1, duration as NSNumber, tag)
onEvent("onKeyboardMoveEnd", position as NSNumber, progress as NSNumber, duration as NSNumber, tag)
onNotify("KeyboardController::keyboardDidShow", data)

removeKeyboardWatcher()
Expand Down Expand Up @@ -285,9 +288,8 @@ public class KeyboardMovementObserver: NSObject {
return
}

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

if keyboardPosition == prevKeyboardPosition || keyboardFrameY == 0 {
return
Expand Down
15 changes: 2 additions & 13 deletions src/components/KeyboardAvoidingView/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,8 @@ export const useKeyboardAnimation = () => {

isClosed.value = e.height === 0;

// `height` update happens in `onMove` handler
// in `onEnd` we need to update only if `onMove`
// wasn't called (i. e. duration === 0)
//
// we can not call this code without condition below
// because in some corner cases (iOS with `secureTextEntry`)
// `onEnd` can be emitted before `onMove` and in this case
// it may lead to choppy/glitchy/jumpy UI
// see https://github.com/kirillzyusko/react-native-keyboard-controller/issues/327
if (e.duration === 0) {
progress.value = e.progress;
height.value = e.height;
}
height.value = e.height;
progress.value = e.progress;
},
},
[],
Expand Down

0 comments on commit 6861faf

Please sign in to comment.