[lab][Masonry] Fix hidden first item causing layout to collapse

[Fanzzzd]

Closes https://github.com/mui/material-ui/issues/42611

Description

This pull request addresses a bug in the Masonry layout component where hiding the first item causes all remaining items to collapse into a single column. The expected behavior is that the remaining items should continue to distribute evenly across the columns, maintaining the Masonry layout's intended structure and visual consistency.

Reproduction Steps

To reproduce this issue:

Create a Masonry layout using the Material UI Masonry component. Hide the first item in the Masonry layout using CSS or JavaScript. Observe the layout of the remaining items.

Current Behavior:

When the first item is hidden, the remaining items are forced into a single column rather than distributing evenly across the available columns.

Expected Behavior:

The remaining items should adjust and distribute evenly across the columns, maintaining the Masonry layout even if the first item is hidden.

You can see a live example of the issue here: StackBlitz Example

Solution

The issue is resolved by updating the Masonry layout logic to account for hidden items. The layout recalculates and redistributes the items appropriately even when the first item is hidden, ensuring that the Masonry structure remains consistent and balanced.

Changes

Modified the Masonry component to handle hidden items effectively. Updated the item positioning logic to ensure proper distribution across columns when the first item is hidden.

Related Issues

Addresses the issue described in #42611

• [x] I have followed (at least) the PR section of the contributing guide.

[mui-bot]

Netlify deploy preview

https://deploy-preview-42630--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad) No bundle size changes

Generated by :no_entry_sign: dangerJS against 44bbae163adb18febde9d906527e6fb456a3efb7

[DiegoAndai]

Hey @Fanzzzd, thanks for working on this! May I ask you to explain why the bug was occurring and how these changes fix it, please? This will help me understand the solution to review it properly 😊

[Fanzzzd]

Thank you for asking for clarification, @DiegoAndai . I'll explain why the bug was occurring and how these changes fix it.

The original bug: The previous code didn't correctly handle hidden or invisible items. Here's the relevant part of the original code:

const handleResize = React.useCallback(
  (masonryChildren) => {
    if (!masonryRef.current || !masonryChildren || masonryChildren.length === 0) {
      return;
    }

    const masonry = masonryRef.current;
    const masonryFirstChild = masonryRef.current.firstChild;
    const parentWidth = masonry.clientWidth;
    const firstChildWidth = masonryFirstChild.clientWidth;

    if (parentWidth === 0 || firstChildWidth === 0) {
      return;
    }

    const firstChildComputedStyle = window.getComputedStyle(masonryFirstChild);
    const firstChildMarginLeft = parseToNumber(firstChildComputedStyle.marginLeft);
    const firstChildMarginRight = parseToNumber(firstChildComputedStyle.marginRight);

    // ... (rest of the code)

    masonry.childNodes.forEach((child) => {
      if (child.nodeType !== Node.ELEMENT_NODE || child.dataset.class === 'line-break' || skip) {
        return;
      }
      const childComputedStyle = window.getComputedStyle(child);
      const childMarginTop = parseToNumber(childComputedStyle.marginTop);
      const childMarginBottom = parseToNumber(childComputedStyle.marginBottom);

      const childHeight = parseToNumber(childComputedStyle.height)
        ? Math.ceil(parseToNumber(childComputedStyle.height)) + childMarginTop + childMarginBottom
        : 0;
      if (childHeight === 0) {
        skip = true;
        return;
      }
      // ... (rest of the code)
    });
  },
  [sequential],
);

This code had two main issues:

1. It always used the first child (masonryFirstChild) for initial calculations, regardless of whether it was visible or not.
2. It used height detection to determine if items were fully loaded, but this method couldn't distinguish between truly unloaded items and hidden items (both have a height of 0).

How the changes fix it:

1. Correctly selecting the first visible child: Instead of always using the first child, we now find the first visible child:

const masonryFirstVisibleChild = Array.from(masonry.childNodes).find((child) => {
  const childStyle = window.getComputedStyle(child);
  return childStyle.display !== 'none' && childStyle.visibility !== 'hidden';
});

if (!masonryFirstVisibleChild) {
  return;
}

const parentWidth = masonry.clientWidth;
const firstChildWidth = masonryFirstVisibleChild.clientWidth;

if (parentWidth === 0 || firstChildWidth === 0) {
  return;
}

const firstChildComputedStyle = window.getComputedStyle(masonryFirstVisibleChild);
const firstChildMarginLeft = parseToNumber(firstChildComputedStyle.marginLeft);
const firstChildMarginRight = parseToNumber(firstChildComputedStyle.marginRight);

This ensures that even if the first element is hidden, we use an appropriate visible element for layout calculations.

2. Distinguishing between unloaded and hidden items: The new code checks if an element is visible before calculating its height:

masonry.childNodes.forEach((child) => {
  if (child.nodeType !== Node.ELEMENT_NODE || child.dataset.class === 'line-break' || skip) {
    return;
  }

  // Skip if the child is not visible
  const childStyle = window.getComputedStyle(child);
  if (childStyle.display === 'none' || childStyle.visibility === 'hidden') {
    return;
  }

  const childComputedStyle = window.getComputedStyle(child);
  const childMarginTop = parseToNumber(childComputedStyle.marginTop);
  const childMarginBottom = parseToNumber(childComputedStyle.marginBottom);

  // ... (rest of the code remains the same)
});

This allows us to skip hidden items and only consider visible ones for layout.

3. Retaining the original loading detection logic: The new code still keeps the checks for zero height and unloaded nested images, but applies these checks only to visible elements:

const childHeight = parseToNumber(childComputedStyle.height)
  ? Math.ceil(parseToNumber(childComputedStyle.height)) + childMarginTop + childMarginBottom
  : 0;
if (childHeight === 0) {
  skip = true;
  return;
}
// Check nested images
for (let i = 0; i < child.childNodes.length; i += 1) {
  const nestedChild = child.childNodes[i];
  if (nestedChild.tagName === 'IMG' && nestedChild.clientHeight === 0) {
    skip = true;
    break;
  }
}

This approach correctly handles hidden items while ensuring that incompletely loaded items don't affect layout calculations.

[DiegoAndai]

Thanks for the detailed explanation @Fanzzzd. That makes it clearer 🙌🏼. May I ask you a couple of things so we can merge this?

1. As you explained, these changes fix two issues: a. Correctly identify first visible children, we should keep this change b. Skip not visible children. Seems like this change is not necessary to fix the issue. If it's not necessary, we should remove it. If we need to fix it we should create a separate issue, so it's easier to track and backtrack in the future.
2. Add tests for the fix so we can ensure no regressions in the future. There should be a test for: a. First item not visible layout should work b. First item not visible becoming visible and being considered after being visible

Does that make sense?