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

[Masonry] Items at the bottom of component fail to be rendered on Chrome #27934

Closed
2 tasks done
hbjORbj opened this issue Aug 24, 2021 · 7 comments · Fixed by #28059
Closed
2 tasks done

[Masonry] Items at the bottom of component fail to be rendered on Chrome #27934

hbjORbj opened this issue Aug 24, 2021 · 7 comments · Fixed by #28059
Labels
bug 🐛 Something doesn't work component: masonry This is the name of the generic UI component, not the React module! waiting for 👍 Waiting for upvotes

Comments

@hbjORbj
Copy link
Member

hbjORbj commented Aug 24, 2021

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

Chrome limits the number of grid rows to 1000 at the maximum. Overflowing items beyond the 1000th row are simply squashed. A row size of 2px is used for Masonry component. Hence, the maximum height of the masonry is 2000px, and items that go beyond this height fail to be rendered correctly. It is worth noting that this limitation does not exist in Firefox or Safari.

Expected Behavior 🤔

Masonry should be able to render all items correctly on Chrome.

Steps to Reproduce 🕹

https://codesandbox.io/s/imagemasonry-material-demo-forked-x4fyc?file=/demo.js

Steps:

  1. Change the number of columns, columns, to 3 (where masonry's height doesn't exceed 2000px). All items are rendered correctly.
  2. Now, change the number of columns, columns, to 2 (which causes masonry's height to exceed 2000px). The items beyond the maximum height (hence at the bottom) fail to be rendered but rather are squashed.

Context 🔦

The 2,000px height limit is insufficient for dashboards with charts, stats, tables, or images. With the current design (using a row gap of 2px), virtualisation is a good candidate to fix this issue, or there could be other smart workarounds.

Your Environment 🌎

Chrome browser

`npx @material-ui/envinfo` ``` Don't forget to mention which browser you used. Output from `npx @material-ui/envinfo` goes here. ```
@hbjORbj hbjORbj added status: waiting for maintainer These issues haven't been looked at yet by a maintainer component: masonry This is the name of the generic UI component, not the React module! labels Aug 24, 2021
@oliviertassinari oliviertassinari removed the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 24, 2021
@oliviertassinari
Copy link
Member

Possible solutions:

  1. Add a new prop to allow developers to configure the tradeoff. They could trade less item height precision for a higher max-height or the opposite.
  2. Replace the CSS grid implementation with a flex: order, based on https://github.com/gilbitron/flexmasonry. Discussed a bit from [Masonry] Add new component #27439 (comment)
  3. Move to a absolute positioning implementation
  4. Proposals?

@oliviertassinari oliviertassinari added waiting for 👍 Waiting for upvotes bug 🐛 Something doesn't work labels Aug 24, 2021
@Primajin
Copy link
Contributor

Could it be an option to have a virtualized content so that when you scroll down only X need to be rendered?

@oliviertassinari
Copy link
Member

@Primajin Feel free to open a new issue for virtulization support with Masonry.

@Primajin
Copy link
Contributor

#28586 👍🏻

@hbjORbj
Copy link
Member Author

hbjORbj commented Sep 27, 2021

@oliviertassinari @mnajdova @eps1lon The first usage of masonry component within the team is rejected #28485 (review) because of its known limitation, which is described in this issue:

Chrome limits the number of grid rows to 1000 at the maximum. Overflowing items beyond the 1000th row are simply squashed. A row size of 2px is used for Masonry component. Hence, the maximum height of the masonry is 2000px, and items that go beyond this height fail to be rendered correctly.

I think this is a clear indication that we should consider switching to the other implementation, the flexbox approach, here: #28059. Its only downside compared to the current implementation is that it doesn't support column spanning. Still, I think that the lack of column spanning support is miniscule compared to the limitation of current masonry, which fails to render items beyond a certain threshold.

If we decide to switch to the other implementation, I will modify the masonry blog post accordingly.

@YuriGor
Copy link

YuriGor commented Oct 24, 2021

Here is a good implementation
I guess it's based on absolute positioning and that's why it has nice animation support.
But it's not maintained currently and needs to be upgraded to use recent react,
so maybe mui team will consider to adopt it.
I am using it currently in mui based UI, render mui cards as items, looks good.

@hbjORbj
Copy link
Member Author

hbjORbj commented Oct 24, 2021

@YuriGor Thanks for your opinion. Currently, we took the direction of using css flexbox. You can find the details here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: masonry This is the name of the generic UI component, not the React module! waiting for 👍 Waiting for upvotes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants