kirillzyusko / react-native-keyboard-controller

Keyboard manager which works in identical way on both iOS and Android
https://kirillzyusko.github.io/react-native-keyboard-controller/
MIT License
1.67k stars 71 forks source link

Android: KeyboardProvider triggers rerender on open/close ? #629

Closed frozencap closed 1 week ago

frozencap commented 1 week ago

On Android, when you open or close the keyboard, KeyboardProvider's children rerender. Is this expected?

kirillzyusko commented 1 week ago

Hi @frozencap

No, it doesn't trigger:

https://github.com/user-attachments/assets/c7414153-da5e-4eae-87d1-14c2f357e76c

Code that I used for testing:

import "react-native-gesture-handler";

import * as React from "react";
import { StyleSheet, TextInput } from "react-native";
import { GestureHandlerRootView } from "react-native-gesture-handler";
import { KeyboardProvider } from "react-native-keyboard-controller";
import {
  SafeAreaProvider,
  initialWindowMetrics,
} from "react-native-safe-area-context";

const styles = StyleSheet.create({
  root: {
    flex: 1,
  },
});
const Child = () => {
  console.log('re-render');
  return <TextInput style={{ marginTop: 40, height: 50, width: 100, backgroundColor: 'yellow' }} />;
};

export default function App() {
  return (
    <SafeAreaProvider initialMetrics={initialWindowMetrics}>
      <GestureHandlerRootView style={styles.root}>
        <KeyboardProvider statusBarTranslucent>
          <Child />
        </KeyboardProvider>
      </GestureHandlerRootView>
    </SafeAreaProvider>
  );
}
ustuncem commented 1 week ago

I think this is happening in react-navigation. See the video:

https://github.com/user-attachments/assets/2dfe35f0-40a4-4603-95e3-e7be0d1bd04c

I am currently on the Change Password screen but Home is re-rendered when the keyboard is closed. I am on react-native v0.75.3 and using react-native-unistyles.

kirillzyusko commented 1 week ago

Thanks for the video @ustuncem πŸ™

I'm very picky to re-renders and I am always trying to write an optimized code that will trigger a re-render only when needed. I think KeyboardProvider may trigger a re-render only if you call setEnabled() from useKeyboardController hook. But in all other cases it should not trigger any re-renders 🀞

frozencap commented 1 week ago

Same I'm getting rerenders on another screen in react-navigation, also using unistyles!

ustuncem commented 1 week ago

Hi @kirillzyusko i think its not you that triggers a re-render but rather the screens that uses the useStyles() hook of the react-native-unistyles library. its because of the animatedInsets I guess.

UnistylesRegistry.addBreakpoints(breakpoints)
  .addThemes({light: lightTheme})
  .addConfig({initialTheme: 'light', disableAnimatedInsets: true});

I already disabled them but for some reason its always re-rendering every screen that uses this hook.

Edit: https://github.com/jpudysz/react-native-unistyles/issues/268 you can see the issue here btw. I think both libraries are manipulating something common on the runtime and overriding themselves.

Edit final: Unistyles resolved this with the version 2.10.0. I tested it and it is resolved. Just need to update. https://github.com/jpudysz/react-native-unistyles/issues/268

kirillzyusko commented 1 week ago

@frozencap I think the problem is fixed in 2.10.0 of unistyles? Can you verify and close the issue πŸ‘€

I'm sure that KeyboardProvider doesn't trigger a re-render πŸ™ƒ

frozencap commented 1 week ago

I am already on unistyles 2.10.0, the problem happens ootb

however I can confirm unistyles disableAnimatedInsets: true removes the undesirable rerender on keyboard open/close.

not sure how to proceed

(also thanks @kirillzyusko for being so responsive, you're the man)

kirillzyusko commented 1 week ago

@frozencap cool, glad that we all figured out what causes the problem!

Can I close the issue, since we figured out it's not an issue of this library and we found a problem and fix? πŸ‘€

kirillzyusko commented 1 week ago

@frozencap @ustuncem or do you think it's worth to add in documentation somewhere that you should add disableAnimatedInsets: true if use unistyles? Or maybe it should be in docs of unistyles? πŸ€”

frozencap commented 1 week ago

i'm not sure disabling animated insets is a fix, since it's still a desirable feature. it seems react-native-keyboard-controller is responsible for triggering the unistyles animation re-render, since, i suppose, it modifies the insets?

i swapped KeyboardAvoidingView to core react-native component to see if this was a platform keyboard issue and it isn't, the rerender doesn't take place.

ustuncem commented 1 week ago

In version 3 of react native unistyles, they say that this rendering problem will be completely fixed but I dont know when will they update so maybe we can add it as a warning on installation part? Version 3 is currently on pre release to subscribers.

I am not a maintainer though maybe we can open it as a recommendation

frozencap commented 1 week ago

i see, in that case yeah, maybe just add a mention here https://kirillzyusko.github.io/react-native-keyboard-controller/docs/recipes/platform-differences#android ? I don't fully understand the root cause of the issue so it's a bit difficult for me to suggest what to write, but from a lib user perspective could be something like:

kirillzyusko commented 1 week ago

@jpudysz may I ask your opinion on that? πŸ‘€

I'm not sure if I understand correctly what disableAnimatedInsets does - can you maybe explain that and the way how it conflicts with react-native-keyboard-controller? πŸ™

jpudysz commented 1 week ago

Hey @kirillzyusko !

There’s nothing wrong with the keyboard controller. Long story short, when you add your provider, you start manipulating the bottom inset with WindowInsetCompat. Unistyles listens to these changes and applies bottom inset updates. Unfortunately, it only works smoothly for basic screens. When you have multiple inputs with heavier components, it becomes laggy. This is due to the architecture of Unistyles 2.x, which is based on re-renders.

In the 3.0 version, that won’t be an issue, as I’ll target only the styles listening for inset changes and update them at the shadow tree level. This will eliminate re-renders and any performance issues.

In version 2.10.0, I was able to opt out of this feature, so I guess that’s the end of the story πŸ˜‡

Let me know if you need any other help!

kirillzyusko commented 1 week ago

@jpudysz ah, okay, it totally makes sense what you are saying.

Based on all information I think I will simply close that issue, because we have a workaround for unistyles 2.x and the problem should be fixed in 3.x

Thanks everyone for a contribution πŸ’ͺ

frozencap commented 1 week ago

boss, lfg πŸš€