search

idibidiart / react-native-responsive-grid

Bringing the Web's Responsive Design to React Native
Other
379 stars 42 forks source link

setScreenInfo() recalculates unnecessarily #26

Closed peacechen closed 6 years ago

peacechen commented 6 years ago

setScreenInfo always calculates values for mediaSize, etc. On a layout with hundreds of columns / rows, this causes redundant recalculations for every component.

A simple optimization would be to store the previous screen Dimensions and check whether it's changed since the last call. If not, use the previously calculated values. I'll submit a PR.

idibidiart commented 6 years ago

Great catch!

Yes caching would be great. Will take a PR.

I wonder if there is a way to grant you co-maintainer status? We can then make all changes in PRs. Thanks for sharing your ideas for improvements!

peacechen commented 6 years ago

Collaborators may be added to a repo, enabling them to write commits and merge PRs. https://help.github.com/articles/inviting-collaborators-to-a-personal-repository/

Thanks for offering to add me :)

idibidiart commented 6 years ago

Will do when I get home! :)

idibidiart commented 6 years ago

Invite sent. Looking forward to your PR!

peacechen commented 6 years ago

PR #27 implements the screen info caching. It's working well on an app with hundreds of Row/Col instances. Have also tested screen rotation using <Grid>.

idibidiart commented 6 years ago

Found a problem with the logic due to the overlapping of two optimizations (onlySize and caching) Will post comment and an updated PR.

See: https://github.com/idibidiart/react-native-responsive-grid/pull/27#issuecomment-383254561