Closed levic92 closed 2 years ago
I have created a PR to fix this in both FlatGrid and SectionGrid. The issue appears when you specify a maxDimension
greater than the screen dimension. Since the current code favoured maxDimension
it would take an extra render to get the proper totalDimension
. These PRs fix the existing behaviour so I wanted to submit them now with the minimal amount of changes.
I am also working on another addition to the library (related to to this PR) that will require some extra modification to this code, let me explain here and get your input and you can determine if you want me to submit a PR for it:
In my use case I want this grid to be used from small mobile screens to large desktop screens. I also want to limit the content to around 1000px with an inner padding to match the rest of my scrollable content. I can accomplish this by using contentContainerStyle
prop and styling maxWidth
, padding
, etc. So the main issue for my use case is that the library won't take into account the padding when determining the totalDimension
. So my proposed changed (for the next potential PR) is to also look at the contentContainerStyle
for maxWidth
, and also take the padding
and subtract that from totalDimension
. There will be some overlap with the maxDimension
prop but I don't see an issue with that to maintain backwards compatibility. The docs can be made clear that using maxWidth
in contentContainerStyle
is an alternative to using the maxDimension
prop. There is no technical problem using both but it is best just to set one or the other.
See #185 for new code to use contentContainerStyle
Hi @levic92
I am yet to look at the other PR, but quick question: For your use case can you not use maxDimension
and spacing
prop? When calculating items per row we account for spacing
between the items.
If you want to have more spacing on the left and right that that of between the items, you can embed FlatGrid inside a parent View and add padding to that? In this case you would need to pass maxDimension = Min(windowWidth, 1000) - extraPadding * 2)
. You would also need to add style in parent view to align the grid in center.
Of course this is a bit extra code on the consumer side, but the library can be kept simple. Thoughts?
The problem with adding the padding in a wrapper around the FlatGrid is that then the scroll is inset in the middle of the screen. I'd like to maintain the scroll indicator to be at the edge of the screen which also means that the touches (or mouse scrolling) is registered across the entire width of the screen/window.
Having the library automatically adjust to the padding specified in the styles actually isn't that complicated. Let's continue the chat on the other PR once you have time to review it. If you are happy with the idea of making the library more dynamic to these styles then maybe the thing to discuss is whether or not you add a config option like adjustGridToStyles
that enables or disables this new behaviour
defaultTotalDimension should use same logic as onLocalLayout