Skip to content

Commit 9b5510d

Browse files
authored
fix: properly report height for didShow event happening during animation (#557)
## 📜 Description Fixed a case when `onEnd`/`keyboardDidShow` reports incorrect (previous frame) keyboard height. ## 💡 Motivation and Context Fixes regressions introduced in #539 Initially I thought it's safe to read current keyboard frame in `didAppear` lifecycle (I thought it'll be always dispatched after animation finish, so why not to read it). But it was a fatal mistake because: - when keyboard changes its size it changes it immediately and in `didAppear` we'll read old (previous) value ⛔ - on iOS 15 when modal gets hidden, then keyboard appears instantly and iOS 15 dispatches only one event, so we'll read old frame (i. e. `0`); 🔴 - when iOS 17 with `secureTextEntry` randomly attaches/detaches `inputAccessoryView`, then we may also encounter a situation, when we read old frame and we will avoid keyboard incorrectly ❌ So to sum up all previous approaches: - relying on `duration === 0` is not reliable because keyboard can be hidden immediately but with `duration === 250` 🤯 - reading current frame in `keyboardDidShow` is not correct approach because in some situations we may read old values 😔 This PR is a third revision of the solution 😂 Now I learned all lessons (at least I hope so) and invented a better mechanism. Let's go back to original problem. The original problem was in the fact, that a resize event can be dispatched during animation AND it'll contain incorrect information about keyboard size (it'll have final destination, though animation hasn't finished yet). I decided to catch that situation in the code - I added `didShowDeadline` variable and in `keyboardWillAppear` I set it as `timestamp + duration`. In `keyboardDidAppear` I verify, that this event arrived later, and if so, then we read `keyboardHeight` value from `notification`. Otherwise, if event `didShow` was received before deadline (i. e. resize event) we can not read data from notification (as it may not reflect a real state of the things) and instead we are reading it from keyboard view layer (falling back to what is actually shown on the screen). Schematically pipeline looks like: <img width="687" alt="image" src="https://github.com/user-attachments/assets/a5fd7b2f-ee8c-4c86-82fc-d017bb01f363"> I think such approach is safer and it looks like it will fix all known issues and at the same time it doesn't introduce additional complexity to the JS codebase. Fixes #327 (comment) Expensify/App#47679 ## 📢 Changelog ### iOS - added `didShowDeadline` variable; - initialize `didShowDeadline` in `willAppear`; - check if `didShowDeadline` is bigger than current timestamp, and if yes, then only then read values from layer. Otherwise as before - from a notification ## 🤔 How Has This Been Tested? Tested manually on many devices. ## 📸 Screenshots (if appropriate): ### iOS 15 |Before|After| |-------|-----| |<video src="https://github.com/user-attachments/assets/8253efaa-a20e-4566-b747-7676b6c6cd1e">|<video src="https://github.com/user-attachments/assets/988340c6-da7e-40f5-bcd2-6ec95e7d1ea4">| ### Keyboard resize |Before|After| |-------|-----| |<video src="https://github.com/user-attachments/assets/6d008543-38b1-46e3-b792-b4ef813fea77">|<video src="https://github.com/user-attachments/assets/fc7108d7-e996-4568-8f6e-1cefdf7a3e9a">| ### `KeyboardAvoidingView` |Before|After| |-------|-----| |<video src="https://github.com/user-attachments/assets/d43f8b53-d442-4a42-a573-d3a2e30a04ad">|<video src="https://github.com/user-attachments/assets/c7cfe29e-7467-497f-9f4d-b824956f50d6">| ## 📝 Checklist - [x] CI successfully passed - [x] I added new mocks and corresponding unit-tests if library API was changed
1 parent 1c58299 commit 9b5510d

File tree

1 file changed

+9
-3
lines changed

1 file changed

+9
-3
lines changed

ios/observers/KeyboardMovementObserver.swift

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public class KeyboardMovementObserver: NSObject {
4040
private var duration = 0
4141
private var tag: NSNumber = -1
4242
private var animation: KeyboardAnimation?
43+
private var didShowDeadline: Int64 = 0
4344

4445
@objc public init(
4546
handler: @escaping (NSString, NSNumber, NSNumber, NSNumber, NSNumber) -> Void,
@@ -168,6 +169,7 @@ public class KeyboardMovementObserver: NSObject {
168169
let keyboardHeight = keyboardFrame.cgRectValue.size.height
169170
self.keyboardHeight = keyboardHeight
170171
self.duration = duration
172+
didShowDeadline = Date.currentTimeStamp + Int64(duration)
171173

172174
onRequestAnimation()
173175
onEvent("onKeyboardMoveStart", Float(keyboardHeight) as NSNumber, 1, duration as NSNumber, tag)
@@ -193,18 +195,22 @@ public class KeyboardMovementObserver: NSObject {
193195
}
194196

195197
@objc func keyboardDidAppear(_ notification: Notification) {
198+
let timestamp = Date.currentTimeStamp
196199
let (duration, frame) = metaDataFromNotification(notification)
197200
if let keyboardFrame = frame {
198201
let (position, _) = keyboardView.framePositionInWindow
199202
let keyboardHeight = keyboardFrame.cgRectValue.size.height
200203
tag = UIResponder.current.reactViewTag
201204
self.keyboardHeight = keyboardHeight
205+
// if the event is caught in between it's highly likely that it could be a "resize" event
206+
// so we just read actual keyboard frame value in this case
207+
let height = timestamp >= didShowDeadline ? keyboardHeight : position
202208
// always limit progress to the maximum possible value
203-
let progress = min(position / self.keyboardHeight, 1.0)
209+
let progress = min(height / self.keyboardHeight, 1.0)
204210

205211
onCancelAnimation()
206-
onEvent("onKeyboardMoveEnd", position as NSNumber, progress as NSNumber, duration as NSNumber, tag)
207-
onNotify("KeyboardController::keyboardDidShow", getEventParams(position, duration))
212+
onEvent("onKeyboardMoveEnd", height as NSNumber, progress as NSNumber, duration as NSNumber, tag)
213+
onNotify("KeyboardController::keyboardDidShow", getEventParams(height, duration))
208214

209215
removeKeyboardWatcher()
210216
setupKVObserver()

0 commit comments

Comments
 (0)