search

wix / react-native-ui-lib

UI Components Library for React Native
https://wix.github.io/react-native-ui-lib/
MIT License
6.57k stars 715 forks source link

Fix number of columns in iPad Pro 11 inch #3266

Closed M-i-k-e-l closed 1 month ago

M-i-k-e-l commented 2 months ago

DO NOT MERGE

I did not test in private yet

Description

I only managed to reproduce the bug in iPad Pro 11 inch (iOS 17.0 - did not test in other OS) I did not test in private because I was not sure what'll you'll think about this relatively large change.

What went wrong? In this scenario numberOfColumns was set to 6 (in useGridLayout) but it seems that there was no room for all the items, I think this is partly because we have more than one source-of-truth regarding itemWidth (itemSize) and itemSpacing.

I've changed the calculations (the numberOfColumns is now more complex...) to what I think should be and allowed itemSpacing to change in case maxItemWidth is passed.

The values are now also used in usePresenter instead of it recalculating the size and using the default itemSpacing.

Changelog

⚠️ Fix number of columns in iPad Pro 11 inch

Additional info

Ticket 3786

ethanshar commented 2 months ago

@M-i-k-e-l I didn't finish reviewing, but if I understand correctly from the PR description, you mean the itemSpacing is not fixed when user pass maxItemWidth? It sounds wrong, no? I mean if the user wants a specific itemSpacing, you are changing it from them?

M-i-k-e-l commented 1 month ago

@M-i-k-e-l I didn't finish reviewing, but if I understand correctly from the PR description, you mean the itemSpacing is not fixed when user pass maxItemWidth? It sounds wrong, no? I mean if the user wants a specific itemSpacing, you are changing it from them?

A little bit, I'll try to take another look later this week