[Masonry] Observe every masonry child to trigger computation when needed #29896

hbjORbj · 2021-11-25T15:30:52Z

I have followed (at least) the PR section of the contributing guide.

Closes #29692
Closes #30453

Before:

ResizeObserver API was only observing the masonry container and the first child (which we expected will cause issues in the future). As a result, changes in the dimension of children other than the first do not trigger computation (needed for correct layout of masonry). Code sandbox
Why does observing the container not detect changes in dimensions of children? Because masonry container is set to a fixed height after every computation. As a result, except the first mounting, when masonry container doesn't have a fixed height yet, changes in dimensions of children do not get detected.

After:

ResizeObserver API no longer observes the container but now it observes every children. Code sandbox

Preview: https://deploy-preview-29896--material-ui.netlify.app/components/masonry/#main-content

mui-pr-bot · 2021-11-25T15:33:23Z

Details of bundle changes

@material-ui/lab: parsed: -0.09% 😍, gzip: -0.12% 😍

Generated by 🚫 dangerJS against 1f39589

siriwatknp · 2021-11-26T07:14:52Z

Even though it fixes the issue but I am not sure if this is the experience most people want because the masonry item shifted when there are spaces available.

@danilo-leal Do you think this is weird from a designer's perspective?

Screen.Recording.2564-11-26.at.14.14.17.mov

siriwatknp · 2021-11-26T07:15:52Z

Also, another demo should be added to the docs. 😊

hbjORbj · 2021-11-26T09:19:45Z

Even though it fixes the issue but I am not sure if this is the experience most people want because the masonry item shifted when there are spaces available.

@danilo-leal Do you think this is weird from a designer's perspective?

Screen.Recording.2564-11-26.at.14.14.17.mov

That is the property of masonry though. The next item is pushed to the shortest column.

Also, another demo should be added to the docs. 😊

Ah, good call :)

mnajdova · 2021-11-26T09:45:58Z

Even though it fixes the issue but I am not sure if this is the experience most people want because the masonry item shifted when there are spaces available.

@danilo-leal Do you think this is weird from a designer's perspective?

That is the property of masonry though. The next item is pushed to the shortest column.

I agree it looks a bit odd :\ @hbjORbj could you link how other Masonry implementations behave in this scenario? Just to do some benchmarking

hbjORbj · 2021-11-26T10:23:08Z

@mnajdova @siriwatknp

In fact, I think I may be able to come up with a solution so that those masonry items can maintain the original order. I can figure out if computation is running due to a change in masonry item's "height" and then do not set new order for each item in such case. I will update you.

could you link how other Masonry implementations behave in this scenario? Just to do some benchmarking

Sure I will look.

danilo-leal · 2021-11-29T20:20:14Z

@danilo-leal Do you think this is weird from a designer's perspective?

The behavior is weird to me too but after quickly looking at other Masonries out there, it seems that this is indeed how they behave if the column max height size is filled?! (As Benny mentioned). What made it confusing though was that the cards moved to different columns without any transition/animation whatsoever, so I felt lost.

I know it's a different interaction (drag and drop), but I think we could draw some inspiration from https://muuri.dev/. Check their Grid Demo — how each tile moves smoothly whenever you change some of them from their place. It's just an idea. I haven't interacted with a Masonry with Accordions out on the wild yet to be able to contribute with confidence, so I'm just thinking out loud...

hbjORbj · 2021-11-30T09:23:31Z

The behavior is weird to me too but after quickly looking at other Masonries out there, it seems that this is indeed how they behave if the column max height size is filled?! (As Benny mentioned). What made it confusing though was that the cards moved to different columns without any transition/animation whatsoever, so I felt lost.

I know it's a different interaction (drag and drop), but I think we could draw some inspiration from https://muuri.dev/. Check their Grid Demo — how each tile moves smoothly whenever you change some of them from their place. It's just an idea. I haven't interacted with a Masonry with Accordions out on the wild yet to be able to contribute with confidence, so I'm just thinking out loud...

@danilo-leal I agree with you. I will definitely check it out!!

mnajdova · 2022-01-13T12:01:48Z

packages/mui-lab/src/Masonry/Masonry.js

-        resizeObserver.observe(container.firstChild);
-      }
+    if (masonryRef.current) {
+      masonryRef.current.childNodes.forEach((childNode) => {


Shouldn't we still observe the masonry container?

Ok, I got it from the PR description, nevermind :)

mnajdova

Changes look good, I've tested the new sandbox, looks much better. Before we merge, let's add new demo illustrating this (something similar to the codesandbox could be beneficial).

Also, please add some unit tests, to make sure we don't break this again in the future.

hbjORbj · 2022-01-14T13:01:02Z

I added a demo and a test :)

Since a number of issues need this PR to be merged, as @mnajdova suggested, I will go ahead and merge this once approved, and then look into the animation effect @danilo-leal suggested in a separate PR.

cc @siriwatknp

mui-bot · 2022-01-14T13:01:49Z

Details of bundle changes

@material-ui/lab: parsed: -0.09% 😍, gzip: -0.12% 😍

Generated by 🚫 dangerJS against 120dc4b

…ded (mui#29896)

hbjORbj added bug 🐛 Something doesn't work package: lab Specific to @mui/lab component: masonry This is the name of the generic UI component, not the React module! labels Nov 25, 2021

hbjORbj changed the title ~~[WIP] [Masonry] Observe every masonry child~~ [Masonry] Observe every masonry child to trigger computation when needed Nov 25, 2021

hbjORbj requested review from mnajdova and siriwatknp November 25, 2021 16:00

hbjORbj mentioned this pull request Dec 31, 2021

[Masonry] error when replacing content #30453

Closed

2 tasks

mnajdova reviewed Jan 13, 2022

View reviewed changes

hbjORbj requested a review from mnajdova January 14, 2022 12:59

hbjORbj added 3 commits January 14, 2022 13:18

[Masonry] Observe every masonry child

f004bbc

[Masonry] Add demo for masonry with variable heights of items

0d0df3a

[Masonry] Add test

120dc4b

hbjORbj force-pushed the fix-masonry branch from 236cb31 to 120dc4b Compare January 14, 2022 13:18

mnajdova approved these changes Jan 14, 2022

View reviewed changes

hbjORbj merged commit 8e4c482 into mui:master Jan 14, 2022

wladimirguerra pushed a commit to wladimirguerra/material-ui that referenced this pull request Feb 2, 2022

[Masonry] Observe every masonry child to trigger computation when nee…

d094172

…ded (mui#29896)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Masonry] Observe every masonry child to trigger computation when needed #29896

[Masonry] Observe every masonry child to trigger computation when needed #29896

hbjORbj commented Nov 25, 2021 •

edited

mui-pr-bot commented Nov 25, 2021 •

edited

siriwatknp commented Nov 26, 2021

siriwatknp commented Nov 26, 2021

hbjORbj commented Nov 26, 2021 •

edited

mnajdova commented Nov 26, 2021

hbjORbj commented Nov 26, 2021

danilo-leal commented Nov 29, 2021

hbjORbj commented Nov 30, 2021

mnajdova Jan 13, 2022

mnajdova Jan 13, 2022

mnajdova left a comment

hbjORbj commented Jan 14, 2022 •

edited

mui-bot commented Jan 14, 2022 •

edited

[Masonry] Observe every masonry child to trigger computation when needed #29896

[Masonry] Observe every masonry child to trigger computation when needed #29896

Conversation

hbjORbj commented Nov 25, 2021 • edited

mui-pr-bot commented Nov 25, 2021 • edited

siriwatknp commented Nov 26, 2021

siriwatknp commented Nov 26, 2021

hbjORbj commented Nov 26, 2021 • edited

mnajdova commented Nov 26, 2021

hbjORbj commented Nov 26, 2021

danilo-leal commented Nov 29, 2021

hbjORbj commented Nov 30, 2021

mnajdova Jan 13, 2022

Choose a reason for hiding this comment

mnajdova Jan 13, 2022

Choose a reason for hiding this comment

mnajdova left a comment

Choose a reason for hiding this comment

hbjORbj commented Jan 14, 2022 • edited

mui-bot commented Jan 14, 2022 • edited

hbjORbj commented Nov 25, 2021 •

edited

mui-pr-bot commented Nov 25, 2021 •

edited

hbjORbj commented Nov 26, 2021 •

edited

hbjORbj commented Jan 14, 2022 •

edited

mui-bot commented Jan 14, 2022 •

edited