Skip to content
This repository has been archived by the owner on Dec 3, 2022. It is now read-only.

Add hook for handling/preventing back behavior (Android) #39

Closed
slorber opened this issue Sep 2, 2019 · 6 comments
Closed

Add hook for handling/preventing back behavior (Android) #39

slorber opened this issue Sep 2, 2019 · 6 comments

Comments

@slorber
Copy link
Member

slorber commented Sep 2, 2019

Something similar to https://github.com/vonovak/react-navigation-backhandler with hooks

@slorber
Copy link
Member Author

slorber commented Sep 24, 2019

@satya164 I'm using this in my app and it seems to work, does it feel right to you?

export const useBackHandler = (handler: () => boolean) => {
  const focusState = useFocusState();
  const canHandleBack = (focusState.isFocusing || focusState.isFocused) && !focusState.isBlurring;

  useEffect(() => {
    if (canHandleBack) {
      // Normally, it's fast to unsub/resub
      BackHandler.addEventListener('hardwareBackPress', handler);
      return () =>
        BackHandler.removeEventListener('hardwareBackPress', handler);
    }
  }, [canHandleBack, handler]);
};

Basically, we want to handle android back hardware button, when focusing/focused, but not blurring (because the screen is already leaving anyway).

That seems pretty similar to the lib of @vonovak that handles back between willFocus and willBlur.

As far as I remember in v5 you don't have focusing/focused/blurring/blurred transition? so how do you know how to handle back during those transitions?

@slorber
Copy link
Member Author

slorber commented Sep 24, 2019

@satya164 btw that seems like a nice usecase for implementing your useFocusEffect you discussed here: #9 (comment)

Does this feel similar to what you have in v5? In my app it seems to work fine.

// Useful to access the latest user-provided value
const useGetter = <S extends {}>(value: S): (() => S) => {
  const ref = useRef(value);
  useLayoutEffect(() => {
    ref.current = value;
  });
  return useCallback(() => ref.current, [ref]);
};

export const useFocusEffect = (
  effect: () => void | (() => void | undefined),
) => {
  const getEffect = useGetter(effect);
  const focusState = useFocusState();
  const runEffect =
    (focusState.isFocusing || focusState.isFocused) && !focusState.isBlurring;

  useEffect(() => {
    if (runEffect) {
      return getEffect()();
    }
  }, [runEffect, getEffect]);
};

export const useBackHandler = (backHandler: () => boolean) => {
  const getBackHandler = useGetter(backHandler);
  useFocusEffect(() => {
    const latestBackHandler = getBackHandler();
    // Normally, it's fast to unsub/resub
    BackHandler.addEventListener('hardwareBackPress', latestBackHandler);
    return () =>
      BackHandler.removeEventListener('hardwareBackPress', latestBackHandler);
  });
};

I'm not sure what logic you have for runEffect boolean. Should we cleanup that focus effect at the beginning or end of the blur transition?

@satya164
Copy link
Member

does it feel right to you?

I think it's okay. The disadvantage here is that it adds 2 extra re-renders on mount and 1 on unmount which can affect the animation smoothness (even if it's native driver).

As far as I remember in v5 you don't have focusing/focused/blurring/blurred transition?

We don't have focusing and blurring events, but we still fire focus and blur events in addition to transitionStart and transitionEnd events which are specific to stack.

useFocusEffect subscribes to these events internally and manages running the effect: https://github.com/react-navigation/navigation-ex/blob/1153d5575b5d37b5c889158600162be8132b61eb/packages/core/src/useFocusEffect.tsx#L16

Does this feel similar to what you have in v5? In my app it seems to work fine.

The problem is extra re-renders. Even with re-animated we've found that these re-renders during animation really hurt the smoothness.

Should we cleanup that focus effect at the beginning or end of the blur transition?

In v5 the focus and blur events fire at the beginning of the transition. I think this is a good place to cleanup the listener. Since the previous screen is already losing focus, no reason it needs to keep listening to the event.

@slorber
Copy link
Member Author

slorber commented Sep 25, 2019

Yeah, I noticed too that rendering reanimated nodes can sometimes be costy (but reduced it well memoized). Also noticed that reanimated heavy things takes some time to mount, not sure if this is discussed anywhere?

So you mean I should rather, like your code, not rely at all on useState to avoid any unnecessary render right?

Also, not fan of "useFocusState()", as people will use v5 sooner or later I'd rather expose a useFocused() hook that would just return a boolean. Not really fan of "useFocusState()" and as you said it's likely to trigger unnecessary render + make v5 migration more complicated for users.

@satya164
Copy link
Member

Yeah we know about issues with reanimated perf regarding mount/re-render and trying to improve it.

So you mean I should rather, like your code, not rely at all on useState to avoid any unnecessary render right?

It would be ideal, imo

@slorber
Copy link
Member Author

slorber commented Sep 29, 2019

we agreed this backhandler should be implemented by users. This pattern has been added to doc:

const useBackHandler = (backHandler: () => boolean) => {
  useFocusEffect(() => {
    const subscription = BackHandler.addEventListener('hardwareBackPress', backHandler);
    return () => subscription.remove();
  });
};

in #43

@slorber slorber closed this as completed Sep 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants