Unable to get `createCollapsibleStack` working with FlatList?
Closed this issue · 30 comments
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
?
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
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
.
Could you also show how you get onScroll
from useCollapsibleStack()
?
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
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.
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...?
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?
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.
Hi @benevbright, any ideas why it's not working with persisting state ? Thank you
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
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?
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,
});
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.
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.
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...
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,
};
};
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,
};
};
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
insteadsetParam
? Non-serializable values
warning is gone with this approach?
Hi @benevbright,
- I don't see the indicator problem with this approach
Non-serializable values
is gone with this, as we are not passing theonScroll
as a function tosetParams
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.
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
?
When I change the color to #fddbdb
which is the lighter color on the right:
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.
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.
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...
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.
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.
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.
@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.
Hi guys, any news about this one ?
This should remove the warning : "Non-serializable values were found in the navigation state,...".
Thanks guys
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.