oblador / react-native-collapsible

Animated collapsible component for React Native, good for accordions, toggles etc
MIT License
2.45k stars 451 forks source link

When ActiveSessions property changes, all headers and content are re-rendered #293

Closed Ahnert1 closed 5 years ago

Ahnert1 commented 5 years ago

Any time I tap on an item in my <Accordion>, the renderHeader() and renderContent() methods are called for ALL items in the list, which creates a very slow/laggy animation on the accordion. This is only an issue when list has more than ~15 items or so. (Sometimes my <Accordion> has 100+ items)

Am I implementing something wrong? I assume that the renderHeader() and renderContent()methods should NOT be called when the activeSessions prop is changed.

LuizDoPc commented 5 years ago

having the same issue here. There's something that we can do to go around the problem while fix is in progress?

iRoachie commented 5 years ago

This is the expected behaviour. The renderHeader and renderContent methods each have an isActive argument passed to them that needs to be updated when activeSessions change. This is how applying active styling is done.

mahlon-gumbs commented 5 years ago

I'm running into this issue as well. This really shouldn't be expected behavior. If I have 10 items and one becomes active, there should only be two re-renders (the new item going from inactive to active and the old item going from active to inactive). The other items would not have had a change to their isActive flag and thus should not be re-rendered.

mahlon-gumbs commented 5 years ago

For anyone else running into this problem, looks like you're gonna have memoize your components in order to bypass the 'always re-render' behavior of this module. It's the only thing that worked for me as keys, etc are all ignored.

zaptrem commented 5 years ago

@iRoachie Is there any way to pass isActive to the render function so I can add an if statement and only render if that section is open? I tried this but crashed due to " Warning: Failed prop type: Invalid prop children supplied to Collapsible, expected a ReactNode."

` _renderContent = (section, isActive) => { // In the future only render if selected if (isActive) return (

{ }
);
return {};`
iRoachie commented 5 years ago

@zaptrem If you see the props in the readme, isActive is passed to the renderContent function https://github.com/oblador/react-native-collapsible#properties-1

zaptrem commented 5 years ago

@iRoachie

    _renderContent = (section, isActive) => {
        // In the future only render if selected
        console.log(isActive);
        if (isActive) return (
            <View style={styles.content}>
                {
                    <RecordingPlayer recording={section} />
                }
            </View>
        );
        return {};
    };

This crashes with "Invariant Violation: Objects are not valid as React child (found object with keys {}). If you meant to render a collection of children, use an array instead"

Only this works:

_renderContent = (section, index, isActive) => {
        // In the future only render if selected
        //console.log(Boolean(isActive));
        if (Boolean(isActive.toString())) return ...
...

Additionally, (and potentially related), whenever the component that is in here (RecordingPlayer) changes state and rerenders for the first time, the section is collapsed. In my case, that means that whenever the audio player starts playing, the section is closed. Once it's closed, it must be tapped twice; once to "close" it and once to reopen it. After that, it's the wrong size.

Edit: the above might be related to this: https://github.com/oblador/react-native-collapsible/issues/269

iRoachie commented 5 years ago

Of course it crashes, you are surrounding the jsx with object braces:

  if (isActive) return (
            <View style={styles.content}>
-                {
                    <RecordingPlayer recording={section} />
-                }
            </View>
        );
-        return {};

You are returning an object, that's why you see the error.

if (isActive) {
  return (
    <RecordingPlayer recording={section} />
  ) 
}

return null
zaptrem commented 5 years ago

You’re right, sorry about that.

zaptrem commented 5 years ago

@iRoachie However, the issue with #269 still remains and makes this unusable in many cases. Do you have a fix in progress/workaround for this?