react-native-screens icon indicating copy to clipboard operation
react-native-screens copied to clipboard

White line in the top of the screen after navigate between screens

Open RadkevichDenis opened this issue 2 years ago • 11 comments

Description

When switching between screens, a white line appears at the top of the screen and also background color is flickering. This problem is solved by adding detachInactiveScreens={false} to the props navigator. However, when calling enableScreens(false), the problem remains.

Since the detachInactiveScreens prop in CardStack(react-navigation) will always be boolean (not undefined), the check is const { enabled = ENABLE_SCREENS, hasTwoStates, ...rest } = props; will never work. Therefore, enableScreens will not work as expected.

It seems to me that the bug is very similar to https://github.com/software-mansion/react-native-screens/issues/111, but this issue was created using react-navigation v.4-5 and react-native-screens v.1.0.0-alpha.22. This issue has been closed, but a similar bug still appears on both platforms.

Is it still necessary to set detachInactiveScreens for some stacks so that the animation runs smoothly?

Screenshots

IOS

https://user-images.githubusercontent.com/44681387/151158705-9718453b-b6b9-4eca-9508-9f51806868c5.mov

Android

https://user-images.githubusercontent.com/44681387/151158804-bcac9080-7f39-4a89-b18c-d763d70dd537.mp4

Steps To Reproduce

  1. Tap on the "Go to second screen" button

Expected behavior

Animation should occur without a white line appearing at the top of the screen and changing the color of the header

Actual behavior

After transition white line presented in the top of the screen and header background color is flickering.

Reproduction

repo: https://github.com/RadkevichDenis/animation_bug

Platform

  • [x] iOS
  • [x] Android
  • [ ] Web
  • [ ] Windows
  • [ ] tvOS

Workflow

  • [ ] Managed workflow
  • [x] Bare workflow

Package versions

package version
react-native 0.66.4
@react-navigation/native ^6.0.6
@react-navigation/native-stack -
react-native-screens ~3.10.2
react-native-safe-area-context ^3.3.2
react-native-gesture-handler ^1.10.3
react-native-reanimated 2.3.1
expo -

RadkevichDenis avatar Jan 26 '22 12:01 RadkevichDenis

Have you tried to apply this suggestion: https://github.com/software-mansion/react-native-screens/issues/111#issuecomment-757518915 ? It will make the previous screen stay in the native view hierarchy after navigating forward, so the flicker should not be visible there.

WoLewicki avatar Jan 27 '22 11:01 WoLewicki

Yes this solution works, as does detachInactiveScreens={false}. However, this issue was created and closed a long time ago, hasn’t anything been added so far to fix this? In order not to resort to the use of these tricks.

RadkevichDenis avatar Jan 27 '22 18:01 RadkevichDenis

Is it happening because the previous screen is getting detached too fast? Like a frame before the animation ends?

hirbod avatar Jan 29 '22 13:01 hirbod

Yeah, the problem comes from the detaching/attaching process being connected to Animated.Value on the side of react-navigation (see how it is resolved around here: https://github.com/react-navigation/react-navigation/blob/4c5805867ce946d99ba17f71b4d6086bb8751262/packages/stack/src/views/Stack/CardStack.tsx#L580). Maybe there is a way to make it work better but we haven't found a solution for this yet. All contributions are welcome :tada:

WoLewicki avatar Jan 31 '22 11:01 WoLewicki

@WoLewicki sorry, do I correctly understand, that Animated.Value completes its execution path (from initial value to target) faster in native thread, than in JS? And because of that we see flickering? And it happens because of asynchronous bridge? If it is the case and it's a known issue, then maybe we can schedule a delayed update?

I've tried to create a draft of possible solution (don't take it seriously, I'm just trying to show the main idea 😅):

function useDelayedValue<T>(value: T) {
  const [delayedValue, setDelayedValue] = React.useState(value);

  React.useEffect(() => {
    let isMounted = true;

    setTimeout(() => {
      if (isMounted) {
        setDelayedValue(value);
      }
    }, 500);

    return () => {
      isMounted = false;
    };
  }, [value]);

  return delayedValue;
};

export const MaybeScreen = ({
  enabled,
  active,
  ...rest
}: ViewProps & {
  enabled: boolean;
  active: 0 | 1 | Animated.AnimatedInterpolation;
  children: React.ReactNode;
}) => {
  if (Screens != null) {
    const state = useDelayedValue(active);

    return (
      <Screens.Screen enabled={enabled} activityState={state} {...rest} />
    );
  }

  return <View {...rest} />;
};

I understand, that this code is not ideal, since it brings additional communication with native side involving the bridge, it brings additional re-renders, it uses hardcoded timeout etc., but using the code above I can not see any flickering. 🤷‍♂️ So just curious whether we can implement such changes in react-native-screens? Maybe in native code, if we understand, that activityState is Animated.Value, then we can schedule an update slightly later?

Apologies in advance if I'm proposing stupid idea. I'm not very familiar how Animated.Value is represented in Kotlin/ObjC code. But I think it can be a possible solution for this problem. Let me know what do you think 🙂

kirillzyusko avatar Jan 31 '22 20:01 kirillzyusko

I also thought of artificially delaying the value, but I think this will introduce other issues, specially if you rely on the transition progress (like the hopefully coming support for shared animation elements somewhere in the next 3 years, lol)

What we know now is that react-navigation is detaching the screen a little bit too early. I mean there must be a way to make sure it's not. Isn't React-Navigation relying on the transition progress api you guys introduced some time ago?

hirbod avatar Jan 31 '22 20:01 hirbod

I just realized that, regarding the issue details, this error does happen on stack but not on native-stack? If yes, thats the wrong repo, isn't it?

hirbod avatar Jan 31 '22 20:01 hirbod

I just realized that, regarding the issue details, this error does happen on stack but not on native-stack? If yes, thats the wrong repo, isn't it?

Maybe, but the boundaries of this issue are blurred 🙈 The functionality of the stack is toughly related to react-native-screens and honestly I don't know, where this issue should live. Just technically stack simply uses components from this library. If they use it in incorrect way, then of course the issue should be moved to react-navigation repo, but for me it seems like react-navigation do it right. Though it's only my opinion, feel free to correct me 🙂

kirillzyusko avatar Jan 31 '22 20:01 kirillzyusko

IIRC, the exact problem comes from this line: https://github.com/react-navigation/react-navigation/blob/4c5805867ce946d99ba17f71b4d6086bb8751262/packages/stack/src/views/Stack/CardStack.tsx#L595 where we switch the value of isScreenActive not exactly when the animation ends, but 1-EPSILON. I cannot remember why exactly was it implemented like that. The best way to check it would be to log the values on the JS side and see how they are mapped to the native activityState e.g. on Android. Here is a PR that introduced it on the react-navigation side, you can see connected PR from react-native-screens there too: https://github.com/react-navigation/react-navigation/pull/8805.

WoLewicki avatar Feb 01 '22 13:02 WoLewicki

Hi @WoLewicki

Thank you for pointing to the problem. If I decrease EPSILON to 0.001, then everything works good. It seems like it was added for the first time here and then EPSILON changed to 0.01 here.

I understand it's not part of your responsibility to know why it has been changed to bigger value, but just want to ask maybe you accidentally know why?

P. S. it seems like changing EPSILON on Android doesn't fix the problem 😔

kirillzyusko avatar Feb 01 '22 14:02 kirillzyusko

Unfortunately I don't know. If you don't spot any new issues with the decreased value, you can submit a PR with such a change.

WoLewicki avatar Feb 02 '22 13:02 WoLewicki