Skip to content

Commit

Permalink
fix: race condition in useSmoothKeyboardHandler (#284)
Browse files Browse the repository at this point in the history
## 馃摐 Description

Use `evt` instead of `animatedKeyboardHeight` before dispatching `onEnd`
event in `useSmoothKeyboardHandler` hook.

## 馃挕 Motivation and Context

It turns out, that sometimes this code:

```ts
(evt) => {
      if (!evt) {
        return;
      }
      handler.onMove?.(evt);

      console.log("reaction", evt, animatedKeyboardHeight.value, persistedHeight.value);

      // dispatch `onEnd`
      if (animatedKeyboardHeight.value === persistedHeight.value) {

```

is evaluated as:

```
'reaction', { 
  duration: 250,
  target: 397,
  height: 294.95096567560637,
  progress: 0.9990274545503124 }, 295.23809814453125, 295.23809814453125
```

It seems like `prepare` and `react` functions can be executed
asynchronously and shared value from a closure can have a different
value. In this PR I'm using `evt` in comparison statement because it'll
have a correct value and correct events will be dispatched.

Without this fix `onEnd` handler was called two time, as a result
`scrollPosition` value was updated and then again combination of
`onMove`/`onEnd` was called - last call caused to apply x2 scroll
(because an updated `scrollPosition` is already taken into
consideration).

Was caught here:
https://github.com/kirillzyusko/react-native-keyboard-controller/actions/runs/7037664869

## 馃摙 Changelog

### Android
- use `evt` instead of `animatedKeyboardHeight` before dispatching
`onEnd` event.

## 馃 How Has This Been Tested?

Tested manually in release build on API 28 device

## 馃摳 Screenshots (if appropriate):

|Before|After|
|-------|-----|

|![image](https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/ba5e6a30-da5e-44c6-9519-6ad125a36ab5)|![image](https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/a467c44e-216c-4b59-96a0-edad080792e6)|

## 馃摑 Checklist

- [x] CI successfully passed
  • Loading branch information
kirillzyusko committed Nov 29, 2023
1 parent c94c5e3 commit d8a312d
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const useSmoothKeyboardHandler: typeof useKeyboardHandler = (
handler.onMove?.(evt);

// dispatch `onEnd`
if (animatedKeyboardHeight.value === persistedHeight.value) {
if (evt.height === persistedHeight.value) {
handler.onEnd?.(evt);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export const useSmoothKeyboardHandler: typeof useKeyboardHandler = (
handler.onMove?.(evt);

// dispatch `onEnd`
if (animatedKeyboardHeight.value === persistedHeight.value) {
if (evt.height === persistedHeight.value) {
handler.onEnd?.(evt);
}
},
Expand Down

0 comments on commit d8a312d

Please sign in to comment.