Skip to content

Issues with two providers and React Native Navigation (wix) #130

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

Closed
Omelyan opened this issue Mar 26, 2023 · 14 comments · Fixed by #149
Closed

Issues with two providers and React Native Navigation (wix) #130

Omelyan opened this issue Mar 26, 2023 · 14 comments · Fixed by #149
Assignees
Labels
🤖 android Android specific 🐛 bug Something isn't working

Comments

@Omelyan
Copy link

Omelyan commented Mar 26, 2023

Describe the bug
Everything is fine with the first screen shown. We can receive keyboard events as usual. But as soon as the second screen pushed and then popped out, the first screen didn't receive any keyboard events anymore.
RNN needs declared providers for every screen. It seems that when two KeyboardProvider's are rendered with RNN the first one becomes unresponsive.
Reproducible in a clean project with the following requirements:

  • OS: Android 10 API 29 (with API 30+ everything works fine; in iOS it's ok too)
  • RN version: 0.71.2
  • RN architecture: old
  • RNN version: 7.32.1
  • JS engine: Hermes
  • Library version: 1.4.4
@Omelyan Omelyan added the 🐛 bug Something isn't working label Mar 26, 2023
@kirillzyusko
Copy link
Owner

Hi @Omelyan
Thank you for reporting the issue. Could you please create a small reproducible example? 👀

@kirillzyusko kirillzyusko added the 🤖 android Android specific label Mar 27, 2023
@Omelyan Omelyan changed the title Doesn't work with React Native Navigation (WIX) Issues with two providers and React Native Navigation (wix) Mar 27, 2023
@Omelyan
Copy link
Author

Omelyan commented Mar 27, 2023

Hi @Omelyan Thank you for reporting the issue. Could you please create a small reproducible example? 👀

Hi, thanks for fast response. Yeap I did some tests and found how this can be solved. This is interesting...
https://github.com/Omelyan/RNNKeyboardController-Test

Here's the starting point:
RNNKC1.webm
(you can see that after the second screen will be pushed and popped no keyboard events firing)

Interestingly, when you set https://github.com/Omelyan/RNNKeyboardController-Test/blob/6a4b942737ce1803f0ded0e727f29d1416f68f31/index.js#L14 to false (RNN can wait for initial screen rendering before push animation), then the first keyboard event will be skipped, but the next ones will work:
RNNKC2.webm

And finally, once navigationBarTranslucent is set for KeyboardProvider everything is fine:
RNNKC3.webm

@Omelyan
Copy link
Author

Omelyan commented Mar 27, 2023

@kirillzyusko sadly, but it seems not all are so good :(
Check this video. While keyboard is shown, I push a new screen. Keyboard hides while switching screens. Then on a new screen I focus input and keyboard showing again. But after closing current screen (keyboard hides) the first screen thinks that keyboard is still on. And the worst thing is that it happens from time to time, not always.
RNNKC4.webm

@kirillzyusko
Copy link
Owner

Well, thank you @Omelyan for providing repro and describing all your findings. It's very helpful!
I'm going to clone your repo and see what's going wrong with the app 👀

@Omelyan
Copy link
Author

Omelyan commented Mar 27, 2023

Well, thank you @Omelyan for providing repro and describing all your findings. It's very helpful! I'm going to clone your repo and see what's going wrong with the app 👀

that's great, you're the most responsive lib maint on my mind)) and the lib is great and really helpful.

@kirillzyusko
Copy link
Owner

kirillzyusko commented Mar 27, 2023

What I've found today is:

  1. The lifecycle methods onStart/onProgress/onEnd are not triggered because of this:

image

For some reasons after going back callback.mDispachedInsets and insets are equal 🤷‍♂️

  1. I've tried to re-create next example:
/* eslint-disable react-native/no-inline-styles */
import React, {useState} from 'react';
import {
  Button,
  Keyboard,
  ScrollView,
  TextInput,
  TouchableOpacity,
  View,
} from 'react-native';
import {
  KeyboardProvider,
  useKeyboardHandler,
} from 'react-native-keyboard-controller';
import {Navigation} from 'react-native-navigation';
import Animated, {
  useAnimatedStyle,
  useSharedValue,
  withTiming,
} from 'react-native-reanimated';
import {SecondScreen} from './SecondScreen';

const FirstScreenA = ({componentId}) => {
  const y = useSharedValue(0);

  const style = useAnimatedStyle(() => ({
    height: withTiming(y.value),
  }));

  useKeyboardHandler({
    onStart(e) {
      'worklet';
      y.value = 40 * e.progress;
    },
  });

  return (
    <ScrollView
      style={{
        backgroundColor: 'blue',
      }}
      contentContainerStyle={{
        flexGrow: 1,
        paddingHorizontal: 20,
        paddingVertical: 30,
      }}>
      <View
        style={{
          alignSelf: 'center',
          width: 70,
          aspectRatio: 0.6,
          borderWidth: 5,
          borderRadius: 10,
          marginBottom: 20,
          borderColor: 'white',
        }}>
        <Animated.View
          style={[
            {
              position: 'absolute',
              left: -1,
              right: -1,
              bottom: -1,
              backgroundColor: 'white',
            },
            style,
          ]}
        />
      </View>

      <View
        style={{
          height: 200,
          borderWidth: 10,
          marginBottom: 20,
          padding: 10,
          backgroundColor: 'yellow',
        }}>
        <TouchableOpacity
          style={{height: 50, backgroundColor: 'tomato'}}
          onPress={() => {
            Navigation.push(componentId, {
              component: {name: 'Second screen'},
            });
          }}
        />
      </View>

      <View
        style={{
          height: 200,
          borderWidth: 10,
          marginBottom: 20,
          backgroundColor: 'yellow',
        }}>
        <TextInput style={{height: 40, backgroundColor: 'white'}} />
      </View>

      <View
        style={{
          height: 200,
          borderWidth: 10,
          marginBottom: 20,
          backgroundColor: 'yellow',
        }}>
        <TextInput style={{height: 40, backgroundColor: 'white'}} />
      </View>
    </ScrollView>
  );
};

export const FirstScreen = ({componentId}) => {
  const [show, setShow] = useState(false);

  return (
    <View style={{flex: 1}}>
      <View style={{flex: 0.5}}>
        <KeyboardProvider>
          <FirstScreenA componentId={1} />
        </KeyboardProvider>
      </View>
      <View style={{flex: 0.5}}>
        <Button title="toggle" onPress={() => setShow(c => !c)} />
        {show && (
          <KeyboardProvider>
            <SecondScreen componentId={2} />
          </KeyboardProvider>
        )}
      </View>
    </View>
  );
};

To verify the idea that unmounting of Provider can cause unresponsiveness of first mounted provider. But the idea was wrong - everything works fine, i. e. that react-native-navigation also has an influence on this.

I'll continue my research tomorrow.

P. S. you wrote:

then the first keyboard event will be skipped, but the next ones will work

The similar issue was reported here #113 (comment) I believe issues are similar and most likely fix also will be a similar 🙂 Just an assumption that potential fix could be just calling requestApplyInsets when screen becomes active again, though need to verify it 🤔

P. P. S. it seems like I found a fix. We just need to remove callbacks when the view is detached from window and set new callback when view is attached again. I'm going to do more tests today at evening. If everything is okay then I'll open a PR and ask you to verify my changes, if you don't mind 🙃

Screen.Recording.2023-03-28.at.11.48.51.mov

@Omelyan
Copy link
Author

Omelyan commented Mar 28, 2023

it seems like I found a fix. We just need to remove callbacks when the view is detached from window and set new callback when view is attached again. I'm going to do more tests today at evening. If everything is okay then I'll open a PR and ask you to verify my changes, if you don't mind

sounds great, will be waiting, thanks

@kirillzyusko
Copy link
Owner

@Omelyan could you install a version from this branch: https://github.com/kirillzyusko/react-native-keyboard-controller/tree/fix/invalidate-callbacks-when-view-is-detached and verify whether it fixes the problem or not?

To be honestly I've discovered another problem and applied ugly fix that I don't like. Since I'm handling status/navigation bar paddings like in onApplyWindowInsetsListener -> this listener can be called many times and as a result if you open a second screen, then return back to a first one - happens too many calls to methods which are changing layout and as a result view is getting not laid out properly and first keyboard appearance is getting ignored. I've fixed the problem by calling setPaddings only one time 😅 But in my opinion such fix can break something else in the future, so in a meantime while you are testing I'll try to come up with a better fix.

@Omelyan
Copy link
Author

Omelyan commented Mar 28, 2023

verify whether it fixes the problem or not?

unfortunately nothing seems changed :(

RNNKC5.mov

@kirillzyusko
Copy link
Owner

kirillzyusko commented Mar 28, 2023

@Omelyan thanks, seems like I need to dive into the issue deeper 👍

@Omelyan
Copy link
Author

Omelyan commented Mar 28, 2023

@kirillzyusko btw, events are called several times for some reason on switching screens, while the keyboard is hidden all the time. I mean more than the number of providers...

@Omelyan
Copy link
Author

Omelyan commented Mar 29, 2023

@kirillzyusko Hello. One more thing noticed: screen resize events are fired, causing different layouts before and after the new screen is pushed:

RNNKC6.mov

I think these things are related

@kirillzyusko
Copy link
Owner

Well, I think yes, there are many related things...
Yesterday I've tried to attach a single listener to decorView but even in this case the listener is also getting broken 🤯
(Here is a patch in case if you are interested: react-native-keyboard-controller+1.4.4.patch)

@Omelyan maybe you have TG/Discord - we could have a conversation there and share all findings there in order to fix the issue faster 🙂 You can post your contact here (and the edit a message later in order not to share it for everyone) or send me a message on gmail if you want

@Omelyan
Copy link
Author

Omelyan commented Mar 29, 2023

@kirillzyusko good idea)

kirillzyusko added a commit that referenced this issue Sep 5, 2023
## 📜 Description

Fixed an integration with `react-native-navigation` without hacks.

## 💡 Motivation and Context

To overcome the initial problem I've decided to re-create callback
between `onAttachedToWindow`/`onDetachFromWindow` lifecycles. It
improves the situation, but at some point of time you still may
inconsistent values returned by hook and actual keyboard position.

For me it seems like if the `WindowInsetsAnimationCompat.Callback` is
attached to any parent view of the current screen (i. e. `decorView`,
`rootView`, etc.), then it can be broken after some navigation cycles.

This solution was inspired by
Omelyan/RNNKeyboardController-Test@d5ee7ea.
If we send events through overlay, then it's never broken. I had a look
on a view hierarchy and realised, that overlays are not attached to
parent views or to navigation tree:

<img width="672" alt="image"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/629002ec-7b41-479e-83b0-6122e75c5827">

I got an inspiration from this idea and decided to replicate this
mechanism in native code. I've decided to attach a view as a child to
`@id/action_bar_root` and setup callback on this view. In this case this
view will not be a parent of navigation tree and will not be inside of
navigation tree:

<img width="675" alt="image"
src="https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/18fc5714-6260-4b8f-b66a-7cee7ff3bc29">

However such approach didn't work out - it seems like if current view
has `onAttachedToWindow`/`onDetachedFromWindow` calls, then Keyboard
insets detection in the end will be broken on API < 30.

I've tried to remove this `eventView` in `onDetachedFromWindow` and
re-create it again in `onAttachedToWindow` - and such combination worked
out.

Fixes
#130

## 📢 Changelog

### Android
- setup callbacks in `onAttachedToWindow`;
- add `eventView` as a child of `@id/action_bar_root` view (in
`onAttachedToWindow`);
- attach a callback to `eventView` rather than
`EdgeToEdgeReactViewGroup` (but still send events through
`EdgeToEdgeReactViewGroup` id)
- added `removeSelf` to `ViewGroup` extensions;
- remove `eventView` in `onDetachedFromWindow`;
- change `inputMode` only if new mode is not equal to current one;

### Docs
- update `README.md`;

## 🤔 How Has This Been Tested?

Tested on Pixel 6 Pro (API 28), emulator.

## 📸 Screenshots (if appropriate):


https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/923fe0b6-cdb3-4bca-92e2-44a2c7fe678d

## 📝 Checklist

- [x] CI successfully passed
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants