Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SectionList throws Warning that is only for FlatList when using flexWrap #18079

Closed
MrLoh opened this issue Feb 24, 2018 · 23 comments
Closed

SectionList throws Warning that is only for FlatList when using flexWrap #18079

MrLoh opened this issue Feb 24, 2018 · 23 comments
Labels
Component: SectionList Component: VirtualizedList Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@MrLoh
Copy link

MrLoh commented Feb 24, 2018

Reopening #17516

Is this a bug report?

yes

Have you read the Contributing Guidelines?

yes

Environment

Environment:
OS: macOS High Sierra 10.13.2
Node: 9.3.0
Yarn: 1.3.2
npm: 5.6.0
Watchman: 4.9.0
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: 3.0 AI-171.4408382

Packages: (wanted => installed)
react: 16.3.2 => 16.3.2
react-native: 0.55.4 => 0.55.4

Target Platform: iOS (11.2)

Steps to Reproduce

put flexWrap on the contentContainerStyle of a SectionList:

<SectionList 
    contentContainerStyle= {{
        flexWrap: 'wrap',
        flexDirection: 'row',
    }}
    ....
/>

Expected Behavior

It should either display an error/warning message that says that SectionList doesn't support flexWrap or just work.

Actual Behavior

It seems to work but shows a warning:

Warning: `flexWrap: `wrap`` is not supported with the `VirtualizedList` components.Consider using `numColumns` with `FlatList` instead.

simulator screen shot - iphone 6 - 2018-01-10 at 11 52 27

The warning originates here:

A warning from VirtualizedList shouldn't refer to FlatList. This issue has been reported before in #15772 and #13460. People suggest that using flexWrap works fine for them, so probably the warning should be removed. Otherwise, the warning should be thrown from SectionList and not refer to FlatList, since SectionList doesn't have a numColumns prop.

Reproducible Demo

https://snack.expo.io/rkNQldQEM

Sent with GitHawk

@vonovak
Copy link
Collaborator

vonovak commented Feb 24, 2018

Imo, the error message is fine. SectionList and FlatList use VirtualizedList under the hood, hence the warning "flexWrap: wrap is not supported with the VirtualizedList components"

A warning from VirtualizedList shouldn't refer to FlatList.

why not? You're being given an advice that might be helpful to you or other people: Consider using numColumns with FlatList instead. I find this perfect! The message tells you that something you did may not work, and even gives a suggestion what you could use!

To sum up, I do not think this is an issue.

@MrLoh
Copy link
Author

MrLoh commented Feb 25, 2018

Since there is no num-columns option for section list, I don’t know how this message helps. It is also utterly confusing since flex-wrap: wrap is supported and seems to work fine (except maybe there are performance issues, maybe not). So in summary it provides a warning that something is not supported even though it works, and suggests something that doesn’t provide an alternative. So it is certainly quite unideal. The 5x +1s also clearly show that this is confusing others as well.

@vonovak what would you consider doing with this warning? Ignoring it or does one need to understand that sections and columns are not supported by any React native component and one must find another native solution to have a high performance list with section headings and several columns, or some other workaround. Do you actually know anything about what the warning is based on?

Sent with GitHawk

@hramos hramos added the Lists label Mar 8, 2018
@react-native-bot react-native-bot added the Platform: macOS Building on macOS. label Mar 20, 2018
@hramos hramos removed the Platform: macOS Building on macOS. label Mar 29, 2018
@AliDroid
Copy link

I have the same problem

@justinsherwood
Copy link

I'm seeing this warning as well when using flexWrap: 'wrap' on the VirtualizedList contentContainerStyle. Would be great to receive some clarification as to whether or not there are any performance implications when using flexWrap: 'wrap' for a virtualized list container.

@react-native-bot
Copy link
Collaborator

Thanks for posting this! It looks like your issue may refer to an older version of React Native. Can you reproduce the issue on the latest release, v0.55?

Thank you for your contributions.

@MrLoh
Copy link
Author

MrLoh commented May 16, 2018

Yes it can still be reproduced, and it'd be awesome, if someone who wrote the code could simply explain what the warning is about. Looking at the git blame information the person who added the warning statement, doesn't seem to have a GitHub account anymore, but @sahrens wrote most of the Component, maybe he could give some input.

@ARMATAV
Copy link

ARMATAV commented May 17, 2018

flexWrap: "wrap" works just fine for FlatList, can't find a way to break it, and yet still throws this warning.

@justinsherwood
Copy link

After reviewing the code for SectionList, FlatList, and VirtualizedList, it seems that this warning is thrown to indicate that the new list components do not support the use of flexWrap: "wrap" to create a grid layout within a list. As of react-native 0.55.4, the calculation used to determine which items should be displayed on the screen is reliant on all items being in a single column. When flexWrap: "wrap" is used, the viewabilityConfig calculation is unable to accurately determine which items in the list should be displayed on the screen while scrolling.

It may appear that flexWrap: "wrap" works when ignoring this warning, but if the number of columns in a row is too high and the height of the items is too low, then the calculation used to load new items to the viewport will not be triggered when scrolling up or down. This means that the number of viewable items in the list will be limited to the initial amount rendered.

Not being able to accurately calculate the size of the viewport in a VirtualizedList using flexWrap: "wrap" seems like a pretty big limitation that should be addressed. It would be great if there was a way to calculate the size of the viewport using viewabilityConfig and getItemLayout, which should also include the width of an item to calculate the number of items in a row, to accurately determine which items should appear on the screen when scrolling using flexWrap: "wrap".

I believe the current implementation of FlatList's numColumns functionality needs to be either modified or removed and supported in the VirtualizedList component. This change should enable all components that extend the functionality of VirtualizedList, like SectionList or FlatList, to support a grid layout for items using flexWrap: "wrap", in conjunction with getItemLayout, to dynamically calculate which items that should appear on the screen.

I find the current implementation of numColumns in FlatList to be quite hacky. If the numColumns property is set for a FlatList component, a group of items the size of numColumns will be inserted into a container view to create a grid like effect. The height of the viewport is then calculated by the item height, or getItemLayout, and offset of numColumns to determine which items should appear on the screen when scrolling. It works, but does not address the root of my problem; a grid layout that supports dynamic reordering of items using LayoutAnimation. Inserting items into a subview prevents FlatList from being able to dynamically reorder items with LayoutAnimation and has forced me to revert back to using ListView, which supports flexWrap: "wrap" grid layouts, but is scheduled for depreciation. Hopefully a solution to this issue is presented before the ListView component is fully depreciated.

@stale
Copy link

stale bot commented Aug 19, 2018

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 19, 2018
@MrLoh
Copy link
Author

MrLoh commented Aug 21, 2018

This is not stale. I have already reopened it once. It still needs a response from someone who wrote the code. It’s unclear what the right way is to have several columns on a section list.

@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 21, 2018
@MrLoh
Copy link
Author

MrLoh commented Aug 21, 2018

As @justinsherwood wrote it seems that there is no proper support for several columns in a section list since numColumns is implemented on FlatList and not the underlying VirtualizedList component. If no getItemLayout is implemented everything seems to work fins on small lists at least. Not sure how to go about implementing a proper getItemLayout function with multiple columns here. In summary use of SectionList with several columns is simply not documented.

@stale
Copy link

stale bot commented Nov 19, 2018

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "For Discussion" or "Good first issue" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Nov 19, 2018
@MrLoh
Copy link
Author

MrLoh commented Nov 23, 2018

Still waiting for any reaction. Nothing about this issue is stale except that it didn’t receive any attention.

@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Nov 23, 2018
@hramos hramos removed the Bug Report label Feb 6, 2019
@steinjonathan
Copy link

Still waiting for any reaction

@sahrens
Copy link
Contributor

sahrens commented Feb 13, 2019

I think both sides of the issue have been well summarized above. This basically boils down to a feature request for better grid support (yes the current numColumns implementation is pretty hacky) - but unfortunately we don't have bandwidth to improve it right now. If the FlatList numColumns suggestion doesn't work for you, then the recommended alternative for now is to just use a ScrollView and build up the functionality you need manually.

@diecastro
Copy link

Hi @MrLoh have you tried adding the props: numColumns={2}, horizontal={false} i did it and was able to get rid of the warning

@MrLoh
Copy link
Author

MrLoh commented Mar 31, 2019

It’s no problem to get rid of the warning, but it’s an confusing warning and it’s unclear what impact it has.

@cpojer
Copy link
Contributor

cpojer commented May 9, 2019

As @sahrens says, it is unlikely we'll be able to spend time on this. Is there anybody who'd like to work on a PR with a fix, cc @MrLoh?

@MrLoh
Copy link
Author

MrLoh commented May 9, 2019

I don't even understand what really needs to be done here. If the rendering is broken, that I have to Idea how to fix it. If the rendering actually works, I can commit a PR to simply adjust the error messages.

@smuxx
Copy link

smuxx commented May 31, 2019

Rendering works fine with FlatList and contentContainerStyle props set with flexWrap. Maybe the message should be adjusted...

@MrLoh
Copy link
Author

MrLoh commented May 31, 2019

Well there probably are some issues #18079 (comment)

@stale
Copy link

stale bot commented Aug 29, 2019

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Aug 29, 2019
@stale
Copy link

stale bot commented Sep 5, 2019

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.

@stale stale bot closed this as completed Sep 5, 2019
@facebook facebook locked as resolved and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Component: SectionList Component: VirtualizedList Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

No branches or pull requests

13 participants