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

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

Open
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

Fanzzzd
Copy link

@Fanzzzd Fanzzzd commented Jun 12, 2024

Closes #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

@mui-bot
Copy link

mui-bot commented Jun 12, 2024

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 082d531

@Fanzzzd Fanzzzd changed the title [Masonry] Fix hidden first item causing layout to collapse [material-ui] Fix hidden first item causing layout to collapse Jun 12, 2024
@zannager zannager added the component: Hidden The React component. label Jun 13, 2024
@zannager zannager requested a review from siriwatknp June 13, 2024 16:26
Delete redundant code

Signed-off-by: Fanzzzd <[email protected]>
@ZeeshanTamboli ZeeshanTamboli added package: lab Specific to @mui/lab component: masonry This is the name of the generic UI component, not the React module! and removed component: Hidden The React component. labels Jun 19, 2024
@ZeeshanTamboli ZeeshanTamboli changed the title [material-ui] Fix hidden first item causing layout to collapse [lab][Masonry] Fix hidden first item causing layout to collapse Jun 19, 2024
@DiegoAndai DiegoAndai removed the request for review from siriwatknp June 19, 2024 21:11
@DiegoAndai
Copy link
Member

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
Copy link
Author

Fanzzzd commented Jun 26, 2024

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.

  1. 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.

  1. 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: masonry This is the name of the generic UI component, not the React module! package: lab Specific to @mui/lab
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[material-ui][Masonry] layout forms a single column when the first item is hidden
5 participants