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

Unable to get createCollapsibleStack working with FlatList? #126

Closed
alexpchin opened this issue Apr 19, 2020 · 30 comments
Closed

Unable to get createCollapsibleStack working with FlatList? #126

alexpchin opened this issue Apr 19, 2020 · 30 comments
Labels
bug Something isn't working

Comments

@alexpchin
Copy link
Collaborator

Information

  • react-native version: 0.62.2
  • react-navigation version: 5.1.6
  • react-navigation-collapsible version: ^5.5.0
  • Platform (iOS/Android): IOS
  • react-native init or Expo: react-native init

Detail

When using createCollapsibleStack with Animated.Flatlist, I am occasionally getting the error:

Invariant Violation: Components based on VirtualizedList must be wrapped with Animated.createAnimatedComponent to support native onScroll events with useNativeDriver

If I wrap the FlatList like this:

const AnimatedFlatList = Animated.createAnimatedComponent(FlatList);

Then I get the error:

_this.props.onScroll is not a function

Logging out onScroll, it is:

{_listeners: Array(0), _argMapping: Array(1), _attachedEvent: null, __isNative: true}

This is the case even if I set:

{createCollapsibleStack(
        <Stack.Screen
          component={User}
          name={R.PROFILE_HOME}
          options={options}
        />,
        {
          useNativeDriver: false,
        },
      )}

I can however get createCollapsibleStackSub to work...

Is it related to react-navigation warning about not passing functions to params? We are passing onScroll as a function to route.params?.collapsible?

Also, as a side point onScrollWithListener seems to return undefined?

@alexpchin alexpchin added the bug Something isn't working label Apr 19, 2020
@benevbright
Copy link
Owner

Could you check this example file if you're using it in the same way?
https://github.com/benevbright/react-navigation-collapsible/blob/master/example/src/DefaultHeaderScreen.tsx

@alexpchin
Copy link
Collaborator Author

Hi, thanks for the quick response. Yes in my stack setup I have:

{createCollapsibleStack(
        <Stack.Screen
          component={User}
          name={R.PROFILE_HOME}
          options={options}
        />,
      )}

Then in my screen I have almost exactly the same thing as your setup, with:

return (
    <Animated.FlatList
      contentContainerStyle={{ paddingTop: containerPaddingTop }}
      data={[0]}
      keyExtractor={(_, index) => index.toString()}
      onScroll={onScroll}
      renderItem={() => <View style={{ height: 6000 }} />}
      scrollIndicatorInsets={{ top: scrollIndicatorInsetTop }}
    />
  );

As soon as I scroll, I get:

_this.props.onScroll is not a function.

@benevbright
Copy link
Owner

Could you also show how you get onScroll from useCollapsibleStack()?

@alexpchin
Copy link
Collaborator Author

I have got it the same way:

const {
    containerPaddingTop,
    onScroll,
    onScrollWithListener,
    scrollIndicatorInsetTop,
  } = useCollapsibleStack();

  console.log('onScroll', onScroll);
  console.log('onScrollWithListener', onScrollWithListener);
  • onScroll: {_listeners: Array(0), _argMapping: Array(1), _attachedEvent: null, __isNative: true}
  • onScrollWithListener: undefined

@alexpchin
Copy link
Collaborator Author

OK... Sorry for wasting your time. I'll close, it seems as if there was an issue with my react-navigation caching...

Embarrassing... I blame covid-19.

@alexpchin alexpchin reopened this Apr 20, 2020
@alexpchin
Copy link
Collaborator Author

No, there is definitely an issue... But it appears only sometimes... I will investigate further.

Screenshot 2020-04-20 at 01 13 16

@alexpchin
Copy link
Collaborator Author

alexpchin commented Apr 20, 2020

Ok, looking a little more detail. I've forked the repo and have made a PR to discuss progress:
#127

First, I ran yarn-check and updated to:

@react-navigation/native 5.1.1   ❯  5.1.6
@react-navigation/stack 5.2.3   ❯  5.2.11
react 16.9.0  ❯  16.13.1
react-native-reanimated  1.7.0   ❯  1.8.0 
react-native-screens 2.4.0   ❯  2.5.0
react-native 0.61.5  ❯  0.62.2

Which required updating Podfile to:

ReactCommon/callinvoker 

Then, as I thought the issue might be the serialization of function params being passed, I looked into State persistence - so I set up state persistence in App.js as per https://reactnavigation.org/docs/state-persistence.

If I comment out all createCollapsibleStack and createCollapsibleStackSub then state persistence is working. However, adding them in gives the error:

Warning: React has detected a change in the order of Hooks called by App. This will lead to bugs and errors if not fixed. For more information, read the Rules of Hooks: https://fb.me/rules-of-hooks

   Previous render            Next render
   ------------------------------------------------------
1. useState                   useState
2. useState                   useState
3. useEffect                  useEffect
4. undefined                  useRef
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

    in App (at renderApplication.js:45)
    in RCTView (at AppContainer.js:109)
    in RCTView (at AppContainer.js:135)
    in AppContainer (at renderApplication.js:39)

This, I believe is caused by a combination of the useRefs in the various createCollapsibleStack and createCollapsibleStackSub files and.

if (!isReady) {
  return <ActivityIndicator />;
}

Splitting to a different stack file would probably resolve.

Moving on, navigating to a page causes warning:

Non-serializable values were found in the navigation state, which can break usage such as persisting and restoring state. This might happen if you passed non-serializable values such as function, class instances etc. in params. If you need to use components with callbacks in your options, you can use 'navigation.setOptions' instead. See https://reactnavigation.org/docs/troubleshooting#i-get-the-warning-non-serializable-values-were-found-in-the-navigation-state for more details.

Which relates to: #113

I updated the default screen to be pretty much the same code as mine, but I have not been able to recreate the issue above...?

@benevbright
Copy link
Owner

First of all, as the warning message says

Non-serializable values were found in the navigation state, which can break usage such as persisting and restoring state.

This module is not working with the persisting state. I should add it to README, sorry about it.

Warning: React has detected a change in the order of Hooks called by App. This will lead to bugs and errors if not fixed. For more information, read the Rules of Hooks: https://fb.me/rules-of-hooks

And this warning message is typically shown when you change your code with hot-reload. If you reload App again, this warning won't occur, no?

Do you think this issue only persists when you use the persisting state or it happens regardless of it?

@alexpchin
Copy link
Collaborator Author

alexpchin commented Apr 20, 2020

Ok, got it that this package doesn't work with persisting state.

The original problem seemed to originate from re-loading the page and getting:

Components based on VirtualizedList must be wrapped with Animated.createdComponent to support native onScoll events with useNativeDriver

My current thinking is that the potentially the persisted state is breaking the use on second load due to the [Object object] stored in the persisted params?

Emailed RE: Chat to discuss.

@Steven-Sanseau
Copy link
Contributor

Hi @benevbright, any ideas why it's not working with persisting state ? Thank you

@alexpchin
Copy link
Collaborator Author

Ok, I've managed to replicate the error:

Components based on VirtualizedList must be wrapped with Animated.createdComponent to support native onScoll events with useNativeDriver

I did this by added a bottomTabNavigator... It is caused when you reloading the app on the page with the defaultHeader AND including the persisted state.

I've not found the solution yet, but the code to replicate is in the PR #127

@alexpchin
Copy link
Collaborator Author

I'm just exploring whether we can get rid of wrapping the Stack.Screens in the navigators completely and instead put all the animation functionality into hooks that can be imported where necessary. This would, therefore, get around the issue with serializing params and also mean that it "should" work with the persisting state. What are your thoughts?

@alexpchin
Copy link
Collaborator Author

alexpchin commented Apr 22, 2020

Just a very rough version...

import * as React from 'react';
import { Animated, Dimensions, Platform, View, StatusBar } from 'react-native';
import { useRef } from 'react';
import { isIphoneX } from 'react-native-iphone-x-helper';

const getStatusBarHeight = (isLandscape) => {
  if (Platform.OS === 'ios') {
    if (isLandscape) return 0;
    return isIphoneX() ? 44 : 20;
  } else if (Platform.OS === 'android') {
    return global.Expo ? StatusBar.currentHeight : 0;
  } else return 0;
};

// const getNavigationHeight = (isLandscape, headerHeight) => {
//   return headerHeight + getStatusBarHeight(isLandscape);
// };

const getScrollIndicatorInsetTop = (isLandscape, headerHeight) => {
  if (Platform.OS === 'ios') {
    if (isIphoneX()) return getStatusBarHeight(isLandscape);
    else return headerHeight;
  }
  return headerHeight + getStatusBarHeight(isLandscape);
};

const getDefaultHeaderHeight = (isLandscape) => {
  if (Platform.OS === 'ios') {
    if (isLandscape && !Platform.isPad) {
      return 32;
    } else {
      return 44;
    }
  } else if (Platform.OS === 'android') {
    return 56;
  }
  return 0;
};

const SAFEBOUNCE_HEIGHT_IOS = 300;
const SAFEBOUNCE_HEIGHT_ANDROID = 100;

export const useCollapsibleHeader = ({
  backgroundColor = 'transparent',
  collapsedColor = 'transparent',
  safeBounceHeight = Platform.select({
    android: SAFEBOUNCE_HEIGHT_ANDROID,
    ios: SAFEBOUNCE_HEIGHT_IOS,
  }),
  height = '100%',
  width = '100%',
  position = 'absolute',
} = {}) => {
  const positionY = useRef(new Animated.Value(0)).current;

  const onScroll = Animated.event(
    [{ nativeEvent: { contentOffset: { y: positionY } } }],
    {
      useNativeDriver: true,
    },
  );

  const window = Dimensions.get('window');

  const isLandscape = window.height < window.width;

  // const headerHeight =
  //   collapsibleTarget === CollapsibleTarget.SubHeader
  //     ? route.params?.collapsibleSubHeaderHeight || 0
  //     : getDefaultHeaderHeight(isLandscape);
  const headerHeight = getDefaultHeaderHeight(isLandscape);

  const animatedDiffClampY = Animated.diffClamp(
    positionY,
    0,
    safeBounceHeight + headerHeight,
  );

  const progress = animatedDiffClampY.interpolate({
    extrapolate: 'clamp',
    inputRange: [safeBounceHeight, safeBounceHeight + headerHeight],
    outputRange: [0, 1],
  });
  const translateY = Animated.multiply(progress, -headerHeight);
  const opacity = Animated.subtract(1, progress);
  const containerPaddingTop =
    headerHeight + getScrollIndicatorInsetTop(isLandscape, headerHeight);
  // collapsibleTarget === CollapsibleTarget.SubHeader
  //   ? headerHeight
  //   : getNavigationHeight(isLandscape, headerHeight),
  const scrollIndicatorInsetTop =
    headerHeight + getScrollIndicatorInsetTop(isLandscape, headerHeight);
  // collapsibleTarget === CollapsibleTarget.SubHeader
  //   ? headerHeight
  //   : getScrollIndicatorInsetTop(isLandscape, headerHeight),

  const CollapsedHeaderBackground = ({
    backgroundColor,
    collapsedColor,
    opacity,
    translateY,
  }) => () => (
    <Animated.View style={{ flex: 1, transform: [{ translateY }] }}>
      <View
        style={{
          backgroundColor: collapsedColor || backgroundColor,
          height,
          position,
          width,
        }}
      />
      <Animated.View
        style={{
          backgroundColor,
          flex: 1,
          opacity,
        }}
      />
    </Animated.View>
  );

  return {
    containerPaddingTop,
    headerBackground: CollapsedHeaderBackground({
      backgroundColor,
      collapsedColor,
      opacity,
      translateY,
    }),
    headerTransparent: true,
    onScroll,
    opacity,
    scrollIndicatorInsetTop,
    translateY,
  };
};

And in the screen:

const {
    containerPaddingTop,
    headerBackground,
    headerTransparent,
    onScroll,
    opacity,
    scrollIndicatorInsetTop,
    translateY,
  } = useCollapsibleHeader({
    backgroundColor: C.BACKGROUND_COLOR,
    collapsedColor: C.BACKGROUND_COLOR,
  });

.
.
.

navigation.setOptions({
   headerBackground,
   headerStyle: {
     opacity,
     transform: [{ translateY }],
   },
   headerTransparent,
});

@benevbright
Copy link
Owner

benevbright commented Apr 22, 2020

@Steven-Sanseau

Hi @benevbright, any ideas why it's not working with persisting state ? Thank you

I'm investigating too. The hint would be here https://github.com/react-navigation/react-navigation/blob/de5d985f3b1272d44175f1148c1b6cffc9a2650c/packages/core/src/BaseNavigationContainer.tsx#L248 but I'm struggling to understand this code.

@alexpchin
Thanks Alex. I will find time to check your code on the weekend.

I had actually tried moving all the logic to the hooks before but I got a weird UI problem on FlatList like below.
1
I had to execute route.setParam on hooks' useEffect and it seemed that it's disturbing rendering cycle.

I haven't tried with setOptions, though. I will try that too later.

@alexpchin
Copy link
Collaborator Author

Nice one @benevbright

Yes, the issue with setOptions is that it only takes certain params.

The issues are to be able to access options inside the screen. Basically, if you want to have a dynamic headerStyle, then you have to add all of the styles in the navigation.setOptions call in the Screen itself.

I have the collapsible header working fine with the code above, albeit messy. I'm just working on a version with a sticky header to see if I can get that working... essentially to get CollapsibleStackSub to work with just hooks...

@alexpchin
Copy link
Collaborator Author

alexpchin commented Apr 23, 2020

Almost there... Just working on whether we can calculate the headerHeight and stickyHeader height dynamically rather than using the utils?

import * as React from 'react';
import { Animated, Dimensions, Platform, View, StatusBar } from 'react-native';
import { useRef } from 'react';
import { isIphoneX } from 'react-native-iphone-x-helper';

// START Utils
const getStatusBarHeight = (isLandscape) => {
  if (Platform.OS === 'ios') {
    if (isLandscape) return 0;
    return isIphoneX() ? 44 : 20;
  } else if (Platform.OS === 'android') {
    return global.Expo ? StatusBar.currentHeight : 0;
  } else return 0;
};

// Get the header height for the react-navigation header
// TODO: Check that this is correct from react-navigation
const getDefaultHeaderHeight = (isLandscape) => {
  if (Platform.OS === 'ios') {
    if (isLandscape && !Platform.isPad) {
      return 32;
    } else {
      return 44;
    }
  } else if (Platform.OS === 'android') {
    return 56;
  }
  return 0;
};

const getScrollIndicatorInsetTop = (isLandscape, headerHeight) => {
  if (Platform.OS === 'ios') {
    if (isIphoneX()) return getStatusBarHeight(isLandscape);
    else return headerHeight;
  }
  return headerHeight + getStatusBarHeight(isLandscape);
};
// END Utils

// const SAFEBOUNCE_HEIGHT_IOS = 300;
// const SAFEBOUNCE_HEIGHT_ANDROID = 100;
// const safeBounceHeight = Platform.select({
//   android: SAFEBOUNCE_HEIGHT_ANDROID,
//   ios: SAFEBOUNCE_HEIGHT_IOS,
// }),

export const useCollapsibleHeader = ({
  backgroundColor = 'transparent',
  collapsedColor = 'transparent',
  collapsibleStackSubHeight = 0,
  height = '100%',
  minScroll = 0,
  position = 'absolute',
  useNativeDriver = true,
  width = '100%',
} = {}) => {
  // Create a new reference for the vertical scroll position
  const positionY = useRef(new Animated.Value(0)).current;

  // TODO: Non-dynamic calculation of headerHeight
  // Get the height of the window
  const window = Dimensions.get('window');
  // Is the phone in landscape mode?
  const isLandscape = window.height < window.width;
  // Get the height of the react-navigation header
  const headerHeight = getDefaultHeaderHeight(isLandscape);
  // Get the insetTop
  const insetTop = getScrollIndicatorInsetTop(isLandscape, headerHeight);

  // TODO: Dynamic calculation of headerHeight
  // Issue with this being calculated only after a delay
  const [dynamicHeaderHeight, setHeaderHeight] = React.useState(0);
  const handleLayoutCollapsedHeaderBackground = ({
    nativeEvent: {
      layout: { height = 0 },
    },
  }) => {
    console.log('headerHeight', height);
    setHeaderHeight(height);
  };

  // Calculate the height of the sticky header children (if present)
  const [
    dynamicCollapsibleStackSubHeight,
    setCollapsibleStackSubHeight,
  ] = React.useState(0);
  const handleLayoutCollapsibleStackSub = ({
    nativeEvent: {
      layout: { height = 0 },
    },
  }) => {
    console.log('collapsibleStackSubHeight', height);
    setCollapsibleStackSubHeight(height);
  };

  // Create scroll event to measure when the vertical scroll position changes
  const onScroll = Animated.event(
    [{ nativeEvent: { contentOffset: { y: positionY } } }],
    {
      useNativeDriver,
    },
  );

  // START V2
  // Fix issue with the pull-to-refresh hiding the header
  // When reaching the end of a FlatList or ScrollView on iOS, the screen will bounce and scroll the other way which will trigger the header to show again
  const clampedScrollY = positionY.interpolate({
    extrapolateLeft: 'clamp',
    inputRange: [minScroll + 0, minScroll + 1],
    outputRange: [0, 1],
  });

  // Creates a new Animated value composed from two Animated values multiplied together.
  // By multiplying by -1, we make the clamped (limited by range) scroll value negative
  const minusScrollY = Animated.multiply(clampedScrollY, -1);

  // Calculate how much to move the header
  // diffClamp(a, min, max)
  const translateY = Animated.diffClamp(
    minusScrollY,
    // Adding collapsibleStackSubHeight to prevent header scrolling over sticky content
    -(headerHeight + collapsibleStackSubHeight),
    0,
  );

  // Update opacity with headerHeight from 0 to 1
  const opacity = translateY.interpolate({
    extrapolate: 'clamp',
    inputRange: [-headerHeight, 0],
    outputRange: [0, 1],
  });

  // Could this be calculated using onLayout of HeaderBackground?
  // However, onLayout would cause a delay?
  const containerPaddingTop = headerHeight + insetTop;

  // If dynamic
  const containerPaddingTop = headerHeight + collapsibleStackSubHeight;

  const CollapsedHeaderBackground = ({
    backgroundColor,
    collapsedColor,
    opacity,
    translateY,
  }) => () => (
    <Animated.View
      onLayout={handleLayoutCollapsedHeaderBackground}
      style={{ flex: 1, transform: [{ translateY }] }}
    >
      <View
        style={{
          backgroundColor: collapsedColor || backgroundColor,
          height,
          position,
          width,
        }}
      />
      <Animated.View
        style={{
          backgroundColor,
          flex: 1,
          opacity,
        }}
      />
    </Animated.View>
  );

  // START
  // Logic for a re-usable sticky header

  // Interpolate a range so that the translateY stops the sticky content from moving
  const clampedScrollYSticky = positionY.interpolate({
    extrapolate: 'clamp',
    // headerHeight is added to make sure content moves together
    inputRange: [0, headerHeight + collapsibleStackSubHeight],
    outputRange: [0, -(headerHeight + collapsibleStackSubHeight)],
  });

  const translateYSticky = Animated.diffClamp(
    clampedScrollYSticky,
    -(collapsibleStackSubHeight - headerHeight),
    0,
  );

  // ALSO works...
  // const translateYSticky = positionY.interpolate({
  //   extrapolate: 'clamp',
  //   inputRange: [0, collapsibleStackSubHeight],
  //   outputRange: [0, -collapsibleStackSubHeight],
  // });

  // Create a Component to wrap the sticky header content
  const CollapsibleStackSub = ({ children }) => (
    <Animated.View
      onLayout={handleLayoutCollapsibleStackSub}
      style={{
        backgroundColor: collapsedColor,
        left: 0,
        position: 'absolute',
        right: 0,
        top: containerPaddingTop,
        transform: [{ translateY: translateYSticky }],
        zIndex: 1,
      }}
    >
      {children}
    </Animated.View>
  );
  // END

  // Take into account a sticky header for the scrollIndicatorInsetTop
  const scrollIndicatorInsetTop =
    containerPaddingTop + collapsibleStackSubHeight;

  return {
    CollapsibleStackSub,
    containerPaddingTop,
    headerBackground: CollapsedHeaderBackground({
      backgroundColor,
      collapsedColor,
      opacity,
      translateY,
    }),
    headerTransparent: true,
    onScroll,
    opacity,
    scrollIndicatorInsetTop,
    translateY,
  };
};

@alexpchin
Copy link
Collaborator Author

Where I'm at now...

import * as React from 'react';
import { Animated, View } from 'react-native';
import { useRef } from 'react';
import { useNavigation, useSafeArea } from 'src/hooks';

export const useCollapsibleHeader = ({
  backgroundColor = 'transparent',
  collapsedColor = 'transparent',
  collapsibleStackListKey = 'collapsibleStackListKey',
  collapsibleStackOpacityDuration = 200,
  headerStyle = {},
  height = '100%',
  minScroll = 0,
  position = 'absolute',
  useNativeDriver = true,
  width = '100%',
  headerTransparent = true,
  collapsibleStackSub = false,
  showsHorizontalScrollIndicator = false,
  showsVerticalScrollIndicator = false,
} = {}) => {
  const navigation = useNavigation();
  const insets = useSafeArea();

  // Create a new reference for the vertical scroll position
  const positionY = useRef(new Animated.Value(0)).current;

  // Height of `headerBackground`
  const [headerHeight, setHeaderHeight] = React.useState(0);

  // Calculate the height of the sticky header children (if present)
  const [
    collapsibleStackSubHeight,
    setCollapsibleStackSubHeight,
  ] = React.useState(0);

  // Initialize variables
  const [translateY, setTranslateY] = React.useState(new Animated.Value(0));
  let opacity = 1;
  const containerPaddingTop = headerHeight;
  const [translateYSticky, setTranslateYSticky] = React.useState(
    new Animated.Value(0),
  );
  const scrollIndicatorInsetTop = headerHeight + collapsibleStackSubHeight;

  const handleLayoutCollapsedHeaderBackground = React.useCallback((event) => {
    const { height } = event.nativeEvent.layout;
    setHeaderHeight(height);
  }, []);

  const handleLayoutCollapsibleStackSub = React.useCallback((event) => {
    const { height } = event.nativeEvent.layout;
    setCollapsibleStackSubHeight(height);
  }, []);

  // Create scroll event to measure when the vertical scroll position changes
  const onScroll = React.useCallback(
    Animated.event([{ nativeEvent: { contentOffset: { y: positionY } } }], {
      listener: () => {},
      useNativeDriver,
    }),
    [],
  );

  const CollapsedHeaderBackground = ({
    backgroundColor,
    collapsedColor,
    opacity,
    translateY,
  }) => () => (
    <Animated.View
      onLayout={handleLayoutCollapsedHeaderBackground}
      style={{
        flex: 1,
        transform: [{ translateY }],
      }}
    >
      <View
        style={{
          backgroundColor: collapsedColor || backgroundColor,
          height,
          position,
          width,
        }}
      />
      <Animated.View
        style={{
          backgroundColor,
          flex: 1,
          opacity,
        }}
      />
    </Animated.View>
  );

  // Create a Component to wrap the sticky header content
  const CollapsibleStackSub = ({ children }) => (
    <Animated.View
      onLayout={handleLayoutCollapsibleStackSub}
      style={{
        backgroundColor: collapsedColor,
        left: 0,
        // paddingTop: insets.top,
        position: 'absolute',
        right: 0,
        top: headerHeight,
        transform: [{ translateY: translateYSticky }],
        zIndex: 1,
      }}
    >
      {children}
    </Animated.View>
  );

  const collapsibleStackOpacity = useRef(new Animated.Value(0)).current;

  // Create a Component to wrap the scrollable content
  const CollapsibleStack = ({ children }) => (
    <Animated.FlatList
      contentContainerStyle={{
        paddingTop: containerPaddingTop,
      }}
      data={[0]}
      keyExtractor={(_, index) => index.toString()}
      listKey={collapsibleStackListKey}
      nestedScrollEnabled
      onScroll={onScroll}
      renderItem={() => (
        <Animated.View
          style={{
            flex: 1,
            opacity: collapsibleStackOpacity,
          }}
        >
          {children}
        </Animated.View>
      )}
      scrollIndicatorInsets={{
        top: containerPaddingTop + scrollIndicatorInsetTop,
      }}
      showsHorizontalScrollIndicator={showsHorizontalScrollIndicator}
      showsVerticalScrollIndicator={showsVerticalScrollIndicator}
    />
  );

  // Run when the page has loaded and the onLayouts have finished
  React.useEffect(() => {
    const headerHasLoaded = !!headerHeight;

    // When reaching the end of a FlatList or ScrollView on iOS, the screen will bounce and scroll the other way which will trigger the header to show again
    const clampedScrollY = positionY.interpolate({
      extrapolateLeft: 'clamp',
      inputRange: [minScroll + 0, minScroll + 1],
      outputRange: [0, 1],
    });

    // Creates a new Animated value composed from two Animated values multiplied together.
    // By multiplying by -1, we make the clamped (limited by range) scroll value negative
    const minusScrollY = Animated.multiply(clampedScrollY, -1);

    // Calculate how much to move the header
    setTranslateY(
      Animated.diffClamp(
        minusScrollY,
        // Adding collapsibleStackSubHeight to prevent header scrolling over sticky content
        // -(headerHeight + collapsibleStackSubHeight),
        -headerHeight,
        0,
      ),
    );

    // Update opacity with headerHeight from 0 to 1
    opacity = translateY.interpolate({
      extrapolate: 'clamp',
      inputRange: [-headerHeight, 0],
      outputRange: [0, 1],
    });

    const subStackHasLoaded = !!headerHeight && !!collapsibleStackSubHeight;

    // Interpolate a range so that the translateY stops the sticky content from moving
    const clampedScrollYSticky = positionY.interpolate({
      extrapolate: 'clamp',
      // headerHeight is added to make sure content moves together
      inputRange: [0, headerHeight + collapsibleStackSubHeight],
      outputRange: [0, -(headerHeight + collapsibleStackSubHeight)],
    });

    // Calculate how much to move the CollapsibleSubStack
    setTranslateYSticky(
      Animated.diffClamp(
        clampedScrollYSticky,
        // Fold with CollapsibleSubStack with Header
        // -(collapsibleStackSubHeight - headerHeight),
        -collapsibleStackSubHeight,
        0,
      ),
    );

    // ALSO works to stop, but needs to move down immediately with header
    // setTranslateYSticky(
    //   positionY.interpolate({
    //     extrapolate: 'clamp',
    //     inputRange: [0, collapsibleStackSubHeight],
    //     outputRange: [0, -collapsibleStackSubHeight],
    //   }),
    // );

    // Fade in the content on the page to prevent jumping
    // If the `collapsibleStackSub` option has been provided, check if it has loaded
    if (
      headerHasLoaded &&
      (!collapsibleStackSub || (collapsibleStackSub && subStackHasLoaded))
    ) {
      Animated.timing(collapsibleStackOpacity, {
        duration: collapsibleStackOpacityDuration,
        toValue: 1,
        useNativeDriver: true,
      }).start();
    }

    // Load header, this will first load once to load the headerBackground
    // then, once the `headerHeight` has been calculated it will update with
    // computed values for `opacity` and `translateY`
    navigation.setOptions({
      headerBackground: CollapsedHeaderBackground({
        backgroundColor,
        collapsedColor,
        opacity,
        translateY,
      }),
      headerStyle: {
        ...headerStyle,
        opacity,
        transform: [{ translateY }],
      },
      headerTransparent,
    });
  }, [headerHeight, collapsibleStackSubHeight]);

  return {
    CollapsibleStack,
    CollapsibleStackSub,
    collapsibleStackSubHeight,
    containerPaddingTop,
    handleLayoutCollapsibleStackSub,
    onScroll,
    scrollIndicatorInsetTop,
    translateY,
    translateYSticky,
  };
};

@benevbright
Copy link
Owner

benevbright commented Apr 26, 2020

Wow. Stunning work. Could you create a new branch and push it to your repo so I can test it too?
And I don't think we should put Flatlist under our control, devs use Flatlist in various ways.
And I have two questions

  • You don't see the indicator problem that I posted with GIF when you use setOptions instead setParam?
  • Non-serializable values warning is gone with this approach?

@alexpchin
Copy link
Collaborator Author

Hi @benevbright,

  • I don't see the indicator problem with this approach
  • Non-serializable values is gone with this, as we are not passing the onScroll as a function to setParams

RE: putting the FlatList inside the library. If we export both the CollapsibleStack and the individual bits used to make it, then potentially you have an option to use it "as-is" or alternatively to use all the bits yourself.

Calculating the dynamic height with onLayout unfortunately takes a little time.

Let me fork the repo again and add to the example. My other branch is v cluttered with working out what was going wrong with the tabs.

@alexpchin
Copy link
Collaborator Author

I think on headerStyle in App.tsx, the backgroundColor: '#f002' may have been causing rendering issues? Could actually be an interesting bug for react-navigation?

Screenshot 2020-04-28 at 12 08 33

When I change the color to #fddbdb which is the lighter color on the right:

Screenshot 2020-04-28 at 12 10 06

I'm still facing a little issue with the loading order of things... but I'll push what I have so far to a new branch.

@alexpchin
Copy link
Collaborator Author

I have now seen the indicator problem...

@benevbright When you get a chance, let me know what you think of the progress so far and if you have any ideas of improvements and I'll jump back on if you think I'm on the right track.

@alexpchin
Copy link
Collaborator Author

Ok, the indicator problem is fixed with this weird hack: facebook/react-native#26610 (comment)

  • We found that adding scrollIndicatorInsets={{ right: 1 }} will force the scrollbar to be rendered correctly
  • Additionally, adding scrollIndicatorInsets={{ right: 0 }} does nothing. So we guess there's buggy code in iOS 13 that overrides contentInsets default since we didn't use SafeAreaView in our app.

I will update the example...

@benevbright
Copy link
Owner

Hey, @alexpchin

I still can't find a time to check your PR. I will review this weekend.
Let me share some thoughts here first.

Regarding including Flatlist, we shouldn't put it under our control because the way people use it too much varies. Someone just wants to use ScrollView and someone wants to use customized FlatList. The idea is putting things in our control as less as possible. I think it's important.

Non-serializable values is gone with this

Great 👍

The problem of alpha in header

Yes, it's a bug from react-navigation. We can ignore it.

indicator problem

Cool. Looks like it's a bug from react-native. And there is a workaround 👍. However, I guess we need to investigate it more since the issue is closed there without an actual fix. And I'm worried the fact that it's not happening in a normal way. I will check that too.

@benevbright
Copy link
Owner

Hi, @alexpchin.
I've checked the codes in your PR.
First of all, it's a great job. Thanks a lot.

I'm still not convinced about the use of FlatList inside the module because IMO it will make unexpected results on some users. And the important point I want to make is that this module should be easy to be opted out. Because this library is actually a problematic library. It can be broken anytime depends on how they use react-navigation since the library doesn't cover all the cases. It will never be production-ready, tbh.

And while it already looks great, I don't think we can merge your PR because it's very big PR changing every parts and I want to do it carefully with some time. But when I do it, I will be using most of your codes. So thanks a lot for your work.

@alexpchin
Copy link
Collaborator Author

Hey @benevbright ,

I do agree with your thinking about including FlatList. I wasn't really proposing that the PR should be merged, it was more a discussion point on how we could pivot the library to be collection of re-useable hooks.

I think there are still a few issues, how should we move this forward? My proposal would be that a second pair of eyes (yours probably) would be a good way to maintain an objective opinion on the code. What is your bandwidth for this?

Alternatively, I can strip back the library to remove unused and deprecated code ready for an updated version PR.

@benevbright
Copy link
Owner

@alexpchin Sure. Yeap, I actually knew it was more like POC.

I also think moving logic to the hooks is such a right direction. It would give more opportunities to the library.
I'm a bit concerned about the fact we can't access the user's option and can't merge it with our options when we're doing setOptions. (Fortunately, react-navigation will do a shallow merge, though)

Alternatively, I can strip back the library to remove unused and deprecated code ready for an updated version PR.

Sounds very cool. I appreciate it. Let me add you.

@alexpchin
Copy link
Collaborator Author

alexpchin commented May 3, 2020 via email

@ozerty
Copy link

ozerty commented May 20, 2020

Hi guys, any news about this one ?
This should remove the warning : "Non-serializable values were found in the navigation state,...".

Thanks guys

@alexpchin
Copy link
Collaborator Author

Sorry, I haven't had a chance to get around to making the PR. I obviously want to make sure it's solid... Most of the thinking is there in the PR but it just needs all the files updating etc.

@benevbright
Copy link
Owner

#155

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants