Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: incorrect first frame when keyboard was resized #316

Merged
merged 2 commits into from
Jan 5, 2024

Conversation

kirillzyusko
Copy link
Owner

@kirillzyusko kirillzyusko commented Jan 4, 2024

πŸ“œ Description

Fixed incorrect first frame (when keyboard was resized).

πŸ’‘ Motivation and Context

If we use progress directly for interpolation we may encounter a situation when animation looks junky.

It happens because first frame is not calculated properly when we interpolate value.

First of all let's define how progress value is calculated across platforms when keyboard is resized:

  • on iOS it'll be always 1 because keyboard changes size immediately;
  • on Android prior to Android 11 we'll have intermediate values;
  • on Android 11+ we don't have intermediate values, but to make it more consistent across platforms I added a transition;

So to sum it up: on iOS progress is always 1 when keyboard gets resized. On Android it will change the value, and the calculation algorithm looks like: take latest keyboard frame and new final frame -> divide current to new (in this case we always assure that final progress value will be 1.

However if we do interpolation directly on progress value we may encounter some issues. Let's say keyboard changes size from 0 to 200 and we do interpolation from 0 to 230 (+30 to final keyboard frame). Then keyboard gets resized from 200 to 220. In this case we'll interpolate from 0 to 250. And initial frame will be 200 / 220 * 250 = 227 (but previous value was 220, so we will see a jump from 220 and 227).

To overcome this problem I added useKeyboardInterpolation hook. It fixes this problem by changing interpolation approach. Basically when keyboard appears/disappears it uses the same approach as progress-based interpolation.

However when it comes to keyboard resizing it detects this moment and changes interpolation rules: instead of using a provided range it will use latest interpolated value as the beginning of inputRange, so in our case discussed above new interpolation will be:

{
  inputRange: [200, 220]
  outputRange: [230, 250]
}

In this case we will not have a jump and animation will be smooth.

Closes #315

πŸ“’ Changelog

JS

  • added useKeyboardInterpolation hook;
  • use useKeyboardInterpolation hook in KeyboardAvoidingView/KeyboardStickyView;

πŸ€” How Has This Been Tested?

Tested manually on Pixel 7 Pro.

πŸ“Έ Screenshots (if appropriate):

Before After
resize-jump.mp4
resize-jump-fixed.mp4
bug-jump.mp4
bug-fixed.mp4

πŸ“ Checklist

  • CI successfully passed

@kirillzyusko kirillzyusko added πŸ› bug Something isn't working πŸ€– android Android specific πŸ“š components Anything related to the exported components of this library labels Jan 4, 2024
@kirillzyusko kirillzyusko self-assigned this Jan 4, 2024
Copy link
Contributor

github-actions bot commented Jan 4, 2024

πŸ“Š Package size report

Current size Target Size Difference
112200 bytes 108176 bytes 4024 bytes πŸ“ˆ

@kirillzyusko kirillzyusko marked this pull request as ready for review January 5, 2024 09:05
@kirillzyusko kirillzyusko merged commit 133bfd8 into main Jan 5, 2024
6 checks passed
@kirillzyusko kirillzyusko deleted the fix/315-interpoaltion-incorrect-frame branch January 5, 2024 09:14
kirillzyusko added a commit that referenced this pull request Mar 10, 2024
## πŸ“œ Description

Removed animated keyboard resize transitions on Android. Now it matches
iOS and is also instant.

## πŸ’‘ Motivation and Context

### Problem description

First of all I'd like to give a little bit context:
- transition prior to Android 11 are simulated (compat library listens
to `onAppyWindowInsets` and dispatches the animation that looks kind of
similar to keyboard show/hide);
- when keyboard is resized -> compat library still dispatches an
animated transition (but in fact keyboard frame is changed instantly);
- after Android 11 `onStart`/`onMove`/`onEnd` events are not dispatching
if keyboard is resized;
- instead we should listen to `onApplyWindowInsets` and do a
corresponding reactions on our own;
- in
#62
I decided to simulate behavior for Android < 11 and also do an animated
transition.

Initially for me it looked like a correct option. But devil is in
details and this approach brought new issues:

#### 1️⃣ Incorrect `progress` value when keyboard gets resized

The `progress` value should be only between 0 (closed) and 1 (opened).
If we do an animated keyboard resize, then we need to change `progress`
value accordingly. It would be strange to see `progress` as `1.3` if
keyboard gets bigger, so we always keep it in range of [0, 1].

> [!WARNING]
> The only one exception is when keyboard gets smaller, in this case for
first frame we increased instantly progress to >1 value (let's say
`1.2`) and then changed a progress to `1` (finish of animation), thus
when keyboard is not animated it'll always have a `progress` as `0` or
`1`.

In
#316
I fixed the problem by introducing additional hook, but solution wasn't
perfect, because:
- it's additional code that we need to support;
- people are not aware about this hook;
- it wasn't exported from the package (I knew I'm going to remove it
eventually) and it wasn't documented.

So animated transition for resize is really headache 🀯 

#### 2️⃣ Strange animation with `KeyboardStickyView` + switching between
different TextInput fields with different input types (i. e. text to
numeric)

The undesired animated resize effect comes into play when we have
various simultaneous animations. For example using `KeyboardToolbar`
when you press `next` button and keyboard changes it size from `text` to
`numeric` (becomes smaller). If everything wrapped in
`KeyboardAwareScrollView`, then we have several animations:
- keyboard toolbar is moving because keyboard was resized;
- `KeyboardAwareScrollView` is scrolling because a new input got a
focus.

And it looks very strange. You can see it in
#374

### Solution

#### 1️⃣ Overview

The solution is quite simple - react on keyboard resize instantly (i. e.
it shouldn't be an animated transition). In this case we can assure that
we match iOS behavior and we reduce API fragmentation across platforms.

So in this PR I reworked code and made resize transition instant one:
- first of all I deleted an animated transition from
`onKeyboardResized`, but it helps only for Android 11+;
- secondly I re-wrote `onStart` method for Android < 11 (I detect if
keyboard was resized and keep animation reference that should be ignored
- in `onEnd` I reset that value);
- it worked, but I discovered, that on Android < 11 we may have a
situation, where we dispatch many `onStart` animations (go to emoji ->
start to search them). If we keep only last reference - then other
(previously) started animations affect position and transition is very
janky:


https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/9c311ed7-6196-4aed-957a-bd04e3c73ebf

If we take only latest value - we'll e always one frame behind. It's not
perfect, but still better than race conditions:


https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/add753e7-42f9-41bb-a019-3659e07ecfca

That's the purpose behind `animationsToSkip` HashSet.

#### 2️⃣ new `onResize` handler

Initially I though to add additional `onResize` handler, but after some
research I thought that it could make API of library more complex
without explicit benefits. So I decided to use existing methods. However
it's a bit challenging, because by default on Android we listen only to
`onMove` events. Theoretically we can add a listener and update a value
on `onEnd` - I did it and on one of my projects it resulted to jumpy
behavior.

So for sake of backward compatibility I'm dispatching 3 events on
Android (onStart/onMove/onEnd) and two events on iOS (onStart/onEnd).

#### 3️⃣ One frame lag

After implementation I discovered that there is a one frame dealy
between actual keyboard size changes and the emitted event. After
digging into native code I found that it suffers from the same problem:

|1st frame|2nd frame|3rd frame|4th frame|
|---------|----------|----------|----------|
|<img width="473" alt="image"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/6d4767f7-6ba0-4c92-a92f-9d51c046d32a">|<img
width="475" alt="image"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/111e6465-df66-40d7-bded-cbd873809375">|<img
width="475" alt="image"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/7e8fcced-cf2a-4e4a-ae6b-8c72b283c6af">|<img
width="474" alt="image"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/97beaa36-5fb8-4e63-b3a4-e90ed03b29aa">|

And backward transition:

|1st frame|2nd frame|3rd frame|
|---------|----------|----------|
|<img width="472" alt="image"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/84b36c0d-d23a-4b62-a6d2-d19879511dd3">|<img
width="474" alt="image"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/84af0178-4541-432f-bf78-48449c28100a">|<img
width="475" alt="image"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/d92e6a82-352b-41ba-9572-7d249b6986a7">|

#### 4️⃣ An ability to have an animated transitions

If for some reasons you still prefer to have an animated transition,
then you still can achieve it using `useKeyboardHandler` hook:

```tsx
const useKeyboardAnimation = () => {
  const height = useSharedValue(0);
  const shouldUpdateFromLifecycle = useSharedValue(true);

  useKeyboardHandler({
    onStart: (e) => {
      "worklet";

      // keyboard gets resized
      if (height.value !== 0 && e.height !== 0) {
        shouldUpdateFromLifecycle.value = false;
        height.value = withTiming(e.height, { duration: 250 }, () => {
          shouldUpdateFromLifecycle.value = true;
        });
      }
    },
    onMove: (e) => {
      "worklet";
      
      if (shouldUpdateFromLifecycle.value) {
        height.value = e.height;
      }
    },
  }, []);

  return { height };
}
```

Closes
#374

## πŸ“’ 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 `useKeyboardInterpolation` hook;
- use `progress` based interpolation in
`KeyboardStickyView`/`KeyboardAvoidingView`;

### Android

- removed `DEFAULT_ANIMATION_TIME`;
- do an instant transition if keyboard was resized;
- store running animations for resize in `HashSet` (`animationsToSkip`);
- ignore animations that are in `animationsToSkip` HashSet;
- dispatch instant `onStart`/`onMove`/`onEnd` events if keyboard was
resized;

## πŸ€” How Has This Been Tested?

Tested manually on:
- Xiaomi Redmi Note 5 Pro (Android 9, real device);
- Pixel 7 Pro (Android 14, real device);
- Pixel 7 Pro (Android 14, emulator);
- Pixel 6 Pro (Android 9, emulator);

## πŸ“Έ Screenshots (if appropriate):

### Keyboard toolbar

|Before|After|
|-------|-----|
|<video
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/25e270ca-11dd-41fd-8cd8-404ae1e91eb0">|<video
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/ee8fefcd-14a9-4df2-bf24-a9f447ebab1b">|

### Keyboard Avoiding View

|Before|After|
|------|-----|
|<video
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/aa5989d4-97d4-400d-b40f-862ab882ad59">|<video
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/2016c5c6-2b05-4a5f-9492-f88db2462ad9">|

## πŸ“ Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
@kirillzyusko kirillzyusko added KeyboardAvoidingView πŸ§ͺ Anything related to KeyboardAvoidingView component KeyboardStickyView 🩹 Anything related to KeyboardStickyView component labels May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
πŸ€– android Android specific πŸ› bug Something isn't working πŸ“š components Anything related to the exported components of this library KeyboardAvoidingView πŸ§ͺ Anything related to KeyboardAvoidingView component KeyboardStickyView 🩹 Anything related to KeyboardStickyView component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect first frame when using KeyboardAvoidingView/KeyboardStickyView+offset and keyboard gets resized
1 participant