mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
94.05k stars 32.32k forks source link

[Masonry][material] layout flicker/shift issue, where the columns momentarily transition into rows #36673

Open surendrdhoni opened 1 year ago

surendrdhoni commented 1 year ago

Steps to reproduce 🕹

Link to live example:

Steps:

1.Create a simple Masonry component using Material UI with multiple items. 2.Observe the layout of the component as it loads/reload and adjusts to the screen size. 3.Notice that the columns momentarily transition into rows before returning to their normal state, causing a layout flicker or shift.

Current behavior 😯

The columns momentarily transition into rows before returning to their normal state, causing a layout flicker or shift.

Expected behavior 🤔

The Masonry component should maintain a consistent layout without any visual disruption or flickering.

Context 🔦

I have included the columns and spacing props in my implementation of the Masonry component,And also i have changed the defaultColumns and defaultHeight but no useful. The issue occurs consistently across multiple browsers and devices. I have not set any conflicting CSS styles that may be affecting the layout of the grid. This issue is impacting the visual experience of my application and may affect user engagement and satisfaction.

below image is the simple code using masonry componet image Current output image The above image was captured within microseconds, so it is not visible when refreshing the page a single time. It requires a fast refresh of the page to become visible. Please note that this issue may not be immediately noticeable to users, but it still impacts the visual experience of the application

Expected output Expected output

Thank you for your attention to this issue. Please let me know if there is any additional information or steps I can provide to help identify and resolve the issue.

Your environment 🌎

npx @mui/envinfo System: OS: Windows 10 10.0.19044 CPU: (8) x64 Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz Memory: 4.15 GB / 15.60 GB Binaries: Node: 16.15.0 - C:\Program Files\nodejs\node.EXE Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD npm: 7.24.2 - ~\ReactCode\node_modules\.bin\npm.CMD Managers: Composer: 2.3.5 - C:\ProgramData\ComposerSetup\bin\composer.BAT pip2: 19.2.3 - C:\Python27\Scripts\pip2.EXE pip3: 22.3.1 - C:\Program Files\Python310\Scripts\pip3.EXE Utilities: Git: 2.40.0. Virtualization: Docker: 20.10.23 - C:\Program Files\Docker\Docker\resources\bin\docker.EXE IDEs: VSCode: 1.76.2 - C:\Users\pg\AppData\Local\Programs\Microsoft VS Code\Microsoft VS Code\bin\code.CMD Languages: Java: 18.0.2.1 PHP: 7.4.29 - C:\xampp\php\php.EXE Python: 3.10.10 Databases: MySQL: undefined - C:\Program Files\MySQL\MySQL Server 8.0\bin\mysql.EXE Browsers: Edge: Spartan (44.19041.1266.0), Chromium (111.0.1661.54)
surendrdhoni commented 1 year ago

Hi @hbjORbj any update regarding the above issue??

DiegoAndai commented 1 year ago

Hey @surendrdhoni, thanks for the report!

The Masonry elements height is not known beforehand, so they must be rendered to calculate their height and with that information properly place them in columns. That’s why the elements are initially rendered in one column. Currently there's no workaround that I can think of.

There are possible fixes to this, one could be to provide a way to inform of the elements height through the Masonry’s API, another would be to hide the first render to the user. These are just ideas and have to be explored deeper.

If you wish to dig into it and open a PR, that would be great!

fuzl-llc commented 1 year ago

Anyone figured out a workaround for this? I'm having the same issue and it, to me, makes the masonry component unusable. It just looks terrible when my app is constantly flickering with the scrollbar showing and then going away and seeing a faint whisper of that initial single column layout appear and then go away.

I tried following the idea @DiegoAndai gave above to hide the first render but I can't seem to make it work. I put the masonry component in box component with sx={{ height: 100, visibility: 'hidden', overflowY: 'scroll' }} so that on first render it renders hidden inside a box with height 100 and scrolls in there, so that the main windows doesn't scroll. If I then use Chrome "inspect" tools to uncheck the three styles we get a perfect rendering of the masonry layout with no flicker.

I figured I was set, I'd just simulate the "uncheck" of those styles programmatically and be done, but I can't seem to do it. My attempt was to set a "hasRenderedBefore" state variable and then in a useEffect, I set it to true and re-render the above styles with { height: 'unset', visibility: 'visible', overflowY: 'unset' } but it doesn't work. The styles get set but it still flickers. I'm guessing "unset" isn't the same as what the browser does to uncheck those values.

I'm new to react and not great at CSS but it does seem to render twice and change the styles but I still see the flicker. Any ideas on how to make this actually work? Perhaps in some way that it could be built into a styled masonry grid that could be re-used since I use the component many places in my app? Thanks for any thoughts!

oliviertassinari commented 1 year ago

Having a closer look, the issue is clearly noticeable since #37208, released in v5.13.6 when transitioning between pages client-side. You can compare moving pages in

v5.13.5: https://648721de6dbe8f0008f3755f--material-ui-docs.netlify.app/material-ui/react-masonry/ HEAD: https://mui.com/material-ui/react-masonry/


On a different note. I landed here with a similar pain point as this issue title. I was exploring using the masonry component for https://mui.com/pricing/. I could only make it work with:

<Box
  sx={{
    columnGap: 3,
    columnCount: { sm: 1, md: 2, lg: 3 },
    '& > *': {
      breakInside: 'avoid',
      marginBottom: 2,
    },
  }}
>
Screenshot 2023-07-16 at 02 30 09

https://github.com/mui/material-ui/pull/37975

I tried https://mui.com/material-ui/react-masonry/#server-side-rendering but it wasn't cutting it, the layout is unstable, breaking the anchor link. The main downside of the above solution is that the order of the tile is "vertical based", not "horizontal based".

fuzl-llc commented 1 year ago

Thanks for the info @oliviertassinari - it is helpful. By "vertical based" layout I assume you mean items are rendered from top to bottom in the first column, then top to bottom in second column, and so on? This might be ok for some scenarios but I think for most people it needs to be true masonry style where items render in rows from left to right before going down to create new rows.

Is there any way to do something like I'm trying to do above but have one of the developers do it internally within the component code rather than us all trying to hack something on after the fact? Like, maybe they can do an internal render on a hidden element that doesn't affect layout to get the heights they need and then, knowing those heights, re-render properly without the flicker? I've seen and used other masonry style components that didn't have this problem so it must be possible one way or another...

fuzl-llc commented 1 year ago

Also, like you said @oliviertassinari it seems like this may be due in part to some relatively recent change. In fact, I just noticed the problem in dev today. I checked my prod site which is probably running on an older MUI version (last published a few months ago I think) and it doesn't seem to have the issue... though I'm not sure if there might be some speed difference in how the code runs locally and perhaps an optimized prod build.

fuzl-llc commented 1 year ago

Ok, I have something that can be cleanly added to fix the flicker for now and removed later when hopefully this gets fixed properly from inside the component itself :)

import React, { ReactNode, useEffect } from "react";
import { Box } from "@mui/material";

// This component adds a delay, 1 millisecond by default, to the visibility of all child components.
//
// Motivation:
// The reason this control was created is to wrap a masonry component that initially renders all items vertically (causing a scrollbar), before rendering them in masonry "rows," causing unacceptable flicker.
// There is a commonly known problem of FOUC (flash of unstyled content) where you see an initial rendering of your content before your stylesheets are downloaded.
// There is a similar problem that can happen with Javascript where visible elements on the page are rendered in script and take a moment to fully render.
// This can result in a flicker, sometimes causing a scrollbar to appear and go away, which then causes the whole page's width to be reduced momentarily causing further re-layout activity.
// This DelayedChild componet offres a solution that is probably not ideal but can be useful in certain circumstances.
//
// Functionality:
// This component works by rendering its child components inside a box that is initially set to be invisible and to have a small fixed height and scrollbar 
// so even if the content is "tall" it will just scroll inside this small heigh box and not cause the main browser window to need a scrollbar.
// We do this so that the fixed height box itself will be very unlikely to cause any flicker (e.g. if its content was so tall that it caused a vertical scrollbar be required).
// It is important that we make the child components "invisible" with visibility rather than display: none so that they actually render on the page, allowing the Javascript to do its rendering work.
// Once the delay has finished, and in theory all code in the child components is complete, we remove the fixed height and vertical overflow / scroll behavior and set the box to be visible again.
// 
// Limitations:
// We don't know how long of a delay is required and in theory if we don't wait long enough for the script to complete its work, our approach won't work.
// However, we only set a 1 ms delay and have not had any issues.
// 
// Future Development:
// Perhaps make the delay a property of the component that the parent can set.
function DelayedChild(props: { children: ReactNode }) {

    const delayTime: number = 1;
    const delay = (ms: number) => new Promise(res => setTimeout(res, ms));
    const [shouldRenderChild, setShouldRenderChild] = React.useState(false);

    useEffect(() => {
        delay(delayTime).then(() => {
            setShouldRenderChild(true);
        });
    });

    return (
        <>
            {(() => {
                if (!shouldRenderChild) {
                    return ( // Note: height 10 works but height 1 does not. Even 2 works but putting at 10 in case it might be some browser dependent thing where it says "well, this is too small we're not going to bother to try to render anything here."
                        <Box sx={{ visibility: 'hidden', height: 10, overflowY: 'scroll' }}>
                            {props.children}
                        </Box>
                    );
                } else {
                    return (
                        <Box sx={{ visibility: 'visible' }}>
                            {props.children}
                        </Box>
                    );
                }
            })()}
        </>
    );
}

export default DelayedChild;

You can then wrap the Masonry component like this:

<DelayedChild>
  <Masonry>
    ...your normal masonry stuff here...
  </Masonry>
</DelayedChild>

Obviously just adding a delay isn't ideal and I'm a bit nervous there may be some unintended consequences or other issues but, for me, that risk is worth it to get rid of the flicker for now.

SanNic20 commented 1 year ago

Same thing here ! @hbjORbj Could you please have a look into this ? Thank you in advance.

DiegoAndai commented 1 year ago

Taking a look into it

DiegoAndai commented 1 year ago

Summary

The "flicker" issue existed before but is more noticeable after https://github.com/mui/material-ui/pull/37208, which goal was fixing a ResizeObserver loop limit exceeded Masonry error: https://github.com/mui/material-ui/issues/36909.

Possible render delay's with the ResizeObserver loop limit exceeded error solution were discussed: https://github.com/mui/material-ui/issues/36909#issuecomment-1541542110, i.e. using requestAnimationFrame is the cause for the flickering getting more noticeable as it introduces a delay.

It was also discussed that it seems to be safe to ignore the ResizeObserver loop limit exceeded error: https://github.com/mui/material-ui/issues/36909#issuecomment-1541551496

Reverting https://github.com/mui/material-ui/pull/37208 would improve the flickering, but wouldn't get rid of it, and it would also bring back https://github.com/mui/material-ui/issues/36909.

Possible action paths

Regarding the ResizeObserver loop limit exceeded error:

  1. Not reverting #37208 and sticking with that solution
  2. Reverting #37208 and see if it's possible to catch the error and either ignore it or rethrow it as a warning

Regarding the flickering: The issue is that initially, the height of the longest column of items is not known, so an initial render is needed to obtain this longest column height. This initial render is the perceived flicker.

Possible improvements:

  1. Hide the content until the first layout is set (in practice this is when the Masonry longest column height is calculated)
  2. Enable defaultHeight to use until the actual height is obtained. This will only work if the user knows or has a way to calculate the height beforehand.

My take: My take would be going with

I would like to hear your opinion, do these options make sense to you? Maybe there are other options I'm not seeing, would love to hear those as well!

After we settle on an action path I'll start working on it.

mengnans commented 1 year ago

Experiencing the flickering issue when I am trying to work with a load more function.

tangye1234 commented 1 year ago

My own masonry dose not have flickering issues, and it takes mui's implementation as a reference, but I use styled-component. The masonry works well with next.js.

'use client'

import React, {
  startTransition,
  useEffect,
  useImperativeHandle,
  useMemo,
  useRef,
  useState
} from 'react'
import { flushSync } from 'react-dom'
import { useLayoutEffect } from '@radix-ui/react-use-layout-effect'
import styled, { css } from 'styled-components'

interface MasonryBaseProps {
  columns?: number | Breakpoint<number>
  spacing?: number | Breakpoint<number>
  defaultColumns?: number
  defaultHeight?: number
  defaultSpacing?: number
}

type MasonryInnerProps = MasonryBaseProps &
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  React.ComponentProps<'div'> & { as: any }
type MasonryRootType = ReturnType<typeof styled.div<MasonryBaseProps>>

export type Breakpoint<T> = {
  [key: number]: T
  default?: T
}

type MasonryRootState = {
  spacing: number
  columns: number
  height: number
  ssr: boolean
}

const LineBreaks = styled.span<{ $order: number }>`
  flex-basis: 100%;
  width: 0;
  margin: 0;
  padding: 0;
  order: ${props => props.$order || 'unset'};
`

const MasonryRoot = styled.div.attrs<{ $state: MasonryRootState }>(props => ({
  ...props,
  style: {
    '--masonry-height': props.$state.height ? ntp(props.$state.height) : 'auto',
    ...props.style
  }
}))<{ $state: MasonryRootState }>`
  display: flex;
  flex-flow: column wrap;
  align-content: flex-start;
  contain: strict;
  height: var(--masonry-height, 'auto');
  margin: ${({ $state }) => ntp(-$state.spacing / 2)};

  & > :not(template, ${LineBreaks}, [hidden]) {
    margin: ${({ $state }) => ntp($state.spacing / 2)};
    width: ${({ $state }) =>
      `calc(${(100 / $state.columns).toFixed(2)}% - ${ntp($state.spacing)})`};
  }

  ${({ $state }) =>
    $state.ssr &&
    new Array($state.columns).fill('').map(
      (_, idx) => css`
        &
          > :not(template, ${LineBreaks}, [hidden]):nth-of-type(
            ${$state.columns}n+${(idx + 1) % $state.columns}
          ) {
          order: ${idx + 1};
        }
      `
    )}
`

function ptn(val: string) {
  return Number(val.replace('px', ''))
}

function ntp(n = 0) {
  return `${n}px`
}

function resolveBreakpoint(value: number | Breakpoint<number>, def: number) {
  if (typeof value === 'number') {
    return value
  }
  const w = typeof window === 'undefined' ? Infinity : window.innerWidth
  const keys = Object.keys(value)
    .map(k => (k === 'default' ? Infinity : Number(k)))
    .sort((a, b) => a - b)

  const key = keys.find(k => k > w)
  return key ? (isFinite(key) ? value[key] : value['default'] || def) : def
}

const Masonry = React.forwardRef<HTMLElement, MasonryInnerProps>(
  (
    {
      children,
      className,
      as = 'div',
      columns = 4,
      spacing = 1,
      defaultColumns = typeof columns === 'number'
        ? columns
        : columns.default || 4,
      defaultSpacing = typeof spacing === 'number'
        ? spacing
        : spacing.default || 1,
      defaultHeight = 0,
      ...rest
    },
    ref
  ) => {
    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
    const masonryRef = useRef<HTMLElement>(null!)
    useImperativeHandle(ref, () => masonryRef.current)

    const [isSSR, setSSR] = useState(true)
    useEffect(() => startTransition(() => setSSR(false)), [])

    const [maxColumnHeight, setMaxColumnHeight] = useState(0)
    const [numberOfLineBreaks, setNumberOfLineBreaks] = useState(
      isSSR ? defaultColumns - 1 : 0
    )

    const breakpointSpacing = resolveBreakpoint(
      isSSR ? defaultSpacing : spacing,
      1
    )

    const breakpointColumns = resolveBreakpoint(
      isSSR ? defaultColumns : columns,
      4
    )

    const height = isSSR
      ? defaultHeight
      : Math.ceil(maxColumnHeight + breakpointSpacing / 2)

    const state = useMemo(
      () => ({
        spacing: breakpointSpacing,
        columns: breakpointColumns,
        height,
        ssr: isSSR
      }),
      [breakpointSpacing, breakpointColumns, height, isSSR]
    )

    useLayoutEffect(() => {
      if (typeof ResizeObserver === 'undefined') {
        return
      }

      if (typeof MutationObserver === 'undefined') {
        return
      }

      /**
       * FIXME safari will trigger `ResizeObserver loop completed
       * with undelivered notifications` error in console
       **/
      const resizeObserver = new ResizeObserver(() => {
        const result = handleResize(masonryRef.current, true)
        if (result) {
          flushSync(() => {
            setMaxColumnHeight(result.height)
            setNumberOfLineBreaks(result.numOfLineBreaks)
          })
        }
      })

      if (masonryRef.current) {
        masonryRef.current.childNodes.forEach(child => {
          if (child instanceof Element) {
            resizeObserver.observe(child as Element)
          }
        })
      }

      const mutationObserver = new MutationObserver(mutations => {
        mutations.forEach(mutation => {
          if (mutation.type !== 'childList') {
            return
          }
          mutation.addedNodes.forEach(node => {
            if (node instanceof Element) {
              resizeObserver.observe(node)
            }
          })
          mutation.removedNodes.forEach(node => {
            if (node instanceof Element) {
              resizeObserver.unobserve(node)
            }
          })
        })
      })

      mutationObserver.observe(masonryRef.current, {
        childList: true,
        subtree: false,
        attributes: false,
        characterData: false
      })

      return () => {
        resizeObserver.disconnect()
        mutationObserver.disconnect()
      }
      // eslint-disable-next-line react-hooks/exhaustive-deps
    }, [])

    return (
      <MasonryRoot
        {...rest}
        as={as}
        ref={masonryRef as React.Ref<HTMLDivElement>}
        className={className}
        $state={state}
      >
        {children}
        {new Array(numberOfLineBreaks).fill('').map((_, index) => (
          <LineBreaks key={index} data-class="line-break" $order={index + 1} />
        ))}
      </MasonryRoot>
    )
  }
) as MasonryRootType

Masonry.displayName = 'Masonry'

function handleResize(masonry: HTMLElement | undefined, isResize = false) {
  if (!masonry || masonry.childElementCount === 0) {
    return
  }

  const masonryFirstChild = masonry.firstElementChild
  const parentWidth = masonry.clientWidth
  const firstChildWidth = masonryFirstChild?.clientWidth || 0

  if (parentWidth === 0 || firstChildWidth === 0 || !masonryFirstChild) {
    return
  }

  const firstChildComputedStyle = getComputedStyle(masonryFirstChild)
  const firstChildMarginLeft = ptn(firstChildComputedStyle.marginLeft)
  const firstChildMarginRight = ptn(firstChildComputedStyle.marginRight)

  const currentNumberOfColumns = Math.round(
    parentWidth /
      (firstChildWidth + firstChildMarginLeft + firstChildMarginRight)
  )

  const columnHeights = new Array(currentNumberOfColumns).fill(0) as number[]
  let skip = false

  masonry.childNodes.forEach(child => {
    if (
      !(child instanceof HTMLElement) ||
      child.dataset.class === 'line-break' ||
      skip
    ) {
      return
    }

    const childComputedStyle = getComputedStyle(child)
    const childMarginTop = ptn(childComputedStyle.marginTop)
    const childMarginBottom = ptn(childComputedStyle.marginBottom)
    const parsedChildHeight = ptn(childComputedStyle.height)
    const childHeight = parsedChildHeight
      ? Math.ceil(parsedChildHeight) + childMarginTop + childMarginBottom
      : 0

    if (childHeight === 0) {
      // if any one of children isn't rendered yet, masonry's height shouldn't be computed yet
      skip = true
      return
    }

    // if there is a nested image that isn't rendered yet, masonry's height shouldn't be computed yet
    for (let i = 0; i < child.childNodes.length; i += 1) {
      const nestedChild = child.childNodes[i] as Element
      if (nestedChild.tagName === 'IMG' && nestedChild.clientHeight === 0) {
        skip = true
        break
      }
    }

    if (!skip) {
      // find the current shortest column (where the current item will be placed)
      const currentMinColumnIndex = columnHeights.indexOf(
        Math.min(...columnHeights)
      )

      if (isResize) {
        const oldOrder = Number(child.style.order)
        const newOrder = currentMinColumnIndex + 1
        if (isFinite(oldOrder) && oldOrder !== newOrder) {
          /** debounce order change for 5px difference */
          if (
            Math.abs(
              columnHeights[oldOrder - 1] - columnHeights[newOrder - 1]
            ) < 5
          ) {
            columnHeights[oldOrder - 1] += childHeight
            return
          }
        }
      }

      columnHeights[currentMinColumnIndex] += childHeight
      const order = currentMinColumnIndex + 1
      child.style.order = String(order)
    }
  })

  if (!skip) {
    const numOfLineBreaks =
      currentNumberOfColumns > 0 ? currentNumberOfColumns - 1 : 0
    return {
      height: Math.max(...columnHeights),
      numOfLineBreaks
    }
  }
}

export { Masonry }

This implementation's core difference from mui masonry is:

  1. ignore the warnings from ResizeObserver's callback function
  2. use MutationObserver in order to register ResizeObserver. It will be more efficient when changes happened for children.
  3. Skip the order variation if the height change is almost the same, in order to avoid flickering.
carloscheddar commented 1 year ago

I'm seeing this issue as well when I upgraded to alpha.137 from alpha.131. I'm going to revert that upgrade and go back to the ResizeObserver loop errors since those can be ignored and won't be perceived by users unlike the flickering.

That said, the ResizeObserver errors are quite annoying if they can be safely ignored maybe the component should catch those instead of showing errors on the dev console.

oliviertassinari commented 1 year ago

The Masonry elements height is not known beforehand, so they must be rendered to calculate their height and with that information properly place them in columns. That’s why the elements are initially rendered in one column.

@DiegoAndai I think that we could render, measure, repositioning, all before any frame. The component is not usable in production in its current state IMHO.

DiegoAndai commented 1 year ago

Thanks everyone for the feedback, I'll start working on a PR

DiegoAndai commented 1 year ago

Hey everyone! I've opened https://github.com/mui/material-ui/pull/38427, aiming to solve this pain point without regressing https://github.com/mui/material-ui/issues/36909. An explanation of the fixes is provided in the PR's description.

If anyone has time to test it out, you can do so by linking to the CI build in your package.json:

"@mui/lab": "https://pkg.csb.dev/mui/material-ui/commit/d0a33227/@mui/lab",
"@mui/material": "https://pkg.csb.dev/mui/material-ui/commit/d0a33227/@mui/material",

I would appreciate it 😊

brayamcorral commented 1 year ago

@DiegoAndai I tested this and the flickering no longer appears. thank you

Current: "react": "^17.0.2", "react-dom": "^17.0.2",

tangye1234 commented 1 year ago

I provide another implementation as below: https://stackblitz.com/edit/stackblitz-starters-ktbdbe?file=components%2Fmasonry.tsx

The component depends on styled-components, so is can be planted to mui.

  1. no flicker issues with good performance
  2. reduced state in masonry, so the virtual dom variation is reduced too.
  3. Instead of react useState I use css variable
  4. columns and spacing responsiveness is purely static css-in-js, not recaculated every time resize happened.
  5. good support for ssr / non-ssr
  6. no limit to children component tree
  7. good support for server streaming of children component
  8. good support for infinite loader
DiegoAndai commented 1 year ago

Thanks for sharing your code @tangye1234! Using CSS variables seems interesting.

A significant rewrite of the Masonry component is not planned, but I'll keep this in mind. You can open a PR or issue proposing these changes so we can discuss whether we should implement them.

DiegoAndai commented 1 year ago

Hey everyone! The solution on PR https://github.com/mui/material-ui/pull/38427 is not enough to solve this issue. It solves most of the flickering except on the initial render, which is the most important one.

Removing requestAnimationFrame (which causes the initial flicker) is not an option because that would bring back the ResizeObserver loop error. I also couldn't find a way to catch the ResizeObserver loop error on Material UI's side. Finally, using useLayoutEffect does not give the expected results to achieve setting the layout before the initial paint.

I cannot allocate more time to keep working on this right now. I'll add the ready to take label in case anyone from the community wants to continue this work, either by taking over my PR or proposing a new one. Hopefully, we will find a solution that is right soon.

jasondainter commented 1 year ago

Is anyone able to work on this quickly? If so I am happy to pay them...

Im having this issue also and desperately need a solution.

Ive spent a lot of time implementing masonry only to find the items all stack up on the left in one column as outlined here, before the masonry grid "kicks in". On slow connections this looks very glitchy. In user testing it was the first thing people pointed out calling my product "broken.

If theres anyone willing to work on this quickly I would be happy to pay for it if its fast as I see this first was raised in April so cant wait another 7 months!

Ahead, @DiegoAndai do you happen to know (or perhaps a faster fix) is there is any way to be able to hook into some kind of "hasFullyLoaded" boolean? If so I'm confident i can find the issue on my side then displaying a loading spinner until the grid has been fully loaded, but without knowing this its hard and hacky to do (eg time delays etc).

drichar commented 1 year ago

@jasondainter I ended up here today as well, after several frustrating hours trying to get a satisfactory masonry solution working. I've finally got something I'm happy with, based on @tangye1234's implementation above, with some adjustments for my use case.

The example uses styled components and Tanstack Query in a Next.js app, which luckily aligns well with my project.

Instead of useInfiniteQuery it has a self-referencing/recursive component containing a useQuery hook that fetches a single page of results, triggered by an intersection observer as you scroll to the end of the page. It's a clever, tidy solution imho. Perhaps this is a common pattern for infinite scroll (using recursion)? I've never seen it before.

So far it seems to be working as advertised: performant, with fewer renders, and no flicker as new items are added. 👍

jasondainter commented 1 year ago

Thanks for this @drichar.

I had a previous version of my masonry grid using an intersection observer to create an infinity scroll so i will play a bit with that and see if I have any luck (I turned it off due to some other issues that was causing).

With your solution, are you sure its removed the "1 column" stack issue or could it be its happening so fast you cant see in?

In google chrome, if you inspect element, click the toggle device toolbar then in the top throttle the page down to a low-end mobile this is a good way to replicate and see the problem in slow motion.

I would think if you are using an intersection observer to load a page, it may simple show that 1 column glitch "faster" (eg only 1 page to load) but still actually go through that cycle of stacking up a column and then showing the grid after.

Curious to hear if that is the case?

Its not an issue on fast browsers but as soon as a user gets a slow connection, eg on a mobile device, it becomes a real issue.

Screenshot 2023-11-09 at 09 59 03
drichar commented 1 year ago

The child nodes are arranged in columns inside a useLayoutEffect, so it happens synchronously before the first browser paint. Even when throttled, you get columns in the first render.

jasondainter commented 1 year ago

Thanks. The issue for me with above is I dont really want to use Tanstack Query, nor styled components. If I understood this solution is a total rebuild not actually using MUI Masonry (please correct me if I misunderstood that).

Have sunk quite a bit of development time into using MUI's masonry grid already and it all works except for this one issue.

@DiegoAndai is there no further plans to revisit this and to get it working?

DiegoAndai commented 1 year ago

Hey @jasondainter, thanks for the interest. Sadly, there's no plan to revisit it in the short term.

If anyone wants to work on this, I'll gladly help guide that effort.

jasondainter commented 1 year ago

OK thanks for the update. I ditched this in the end and built a custom solution that is working well for me in react using a flex container set to columns and some javascript to figure out the colums. Every library I tried came with so many complications that it made sense in the end to go that route. Masonry grids are the most trivial looking things that are apparently the least trivial to pull off!

nikhilrp18 commented 8 months ago

@DiegoAndai @tangye1234 @jasondainter flickering issue in mui's masonry while load more data. Is there any new solutions released for the same? can anyone give solutions...

nikhilrp18 commented 8 months ago

OK thanks for the update. I ditched this in the end and built a custom solution that is working well for me in react using a flex container set to columns and some javascript to figure out the colums. Every library I tried came with so many complications that it made sense in the end to go that route. Masonry grids are the most trivial looking things that are apparently the least trivial to pull off!

hi @jasondainter could you please provide the custom implementation for avoid flickering of mui component when load more.. ?

DiegoAndai commented 8 months ago

Hey @nikhilrp18, sadly, there's no solution yet. The issue is ready for anyone who wants to work on it. The solution is complicated and might need a rewrite of some mechanics of the component.

nikhilrp18 commented 8 months ago

ok @DiegoAndai thank you for your response .Could you please provide any insights into when I might expect a solution for this?

Rishi556 commented 8 months ago

@nikhilrp18 I was able to get rid of the flickering in my instance by patching in the changes in https://github.com/mui/material-ui/pull/38427. I know it says that there's still issues in that PR, but in our use case, those issues aren't present, possibly the same in yours?

Rishi556 commented 7 months ago

This might be useful for anyone following this issue: https://github.com/w3c/csswg-drafts/issues/10233 as well as this blog post: https://www.webkit.org/blog/15269/help-us-invent-masonry-layouts-for-css-grid-level-3/.

abriginets commented 6 months ago

@Rishi556 looks very promising. Unfortunately it's going to take a LOT of time before it's well-supported by at least a couple of latest versions of every other popular browsers

https://caniuse.com/?search=grid-template-rows%3Amasonry

NawarA commented 4 months ago

https://www.npmjs.com/package/react-responsive-masonry

I switched to this project for now, as it has 0 issues.

I recommending leveraging / borrowing from this code.

Pure CSS > JS, where possible

jlarmstrongiv commented 4 months ago

I have instances where the masonry grid gives up and displays in a single column:

CleanShot 2024-07-23 at 20 35 56

And where the masonry grid flickers (warning: flashing lights):

https://github.com/user-attachments/assets/fe174a90-e8d7-434f-957d-c9fbc5d984ce

Jun-Murakami commented 3 months ago

The code provided by tangye1234 at https://github.com/mui/material-ui/issues/36673#issuecomment-1692785487 has made my site's Masonry layout work smoothly. https://erisasaki.net/ It's functioning well with the latest Next.js 14, and there are no issues even when nesting SSG content or server components. Thank you very much!

For my use, I rewrote the code to use @emotion/styled. While it's not directly related to MUI, I'm sharing the modified code in case it might be helpful to someone else.

/** @jsxImportSource @emotion/react */
'use client';
import { useEffect, useImperativeHandle, useMemo, useRef, useState, forwardRef, useLayoutEffect } from 'react';
import { css } from '@emotion/react';
import styled from '@emotion/styled';

/**
 * This code is based on the implementation by tangye1234 shared in the MUI GitHub Issues:
 * https://github.com/mui/material-ui/issues/36673#issuecomment-1692785487
 *
 * Original implementation used styled-components.
 * Adapted to use Emotion by Jun Murakami.
 */

export type Breakpoint<T> = {
  [key: number]: T;
  default?: T;
};

interface MasonryBaseProps {
  columns?: number | Breakpoint<number>;
  spacing?: number | Breakpoint<number>;
  defaultHeight?: number;
  disableSSR?: boolean;
}

type MasonryInnerProps = MasonryBaseProps & React.ComponentProps<'div'> & { as: any };
type MasonryRootType = ReturnType<typeof styled.div<MasonryBaseProps>>;

type MasonryRootState = {
  spacing: (readonly [number, number])[];
  columns: (readonly [number, number])[];
  ssr: boolean;
};

const LineBreaks = styled.span<{ $order: number }>`
  flex-basis: 100%;
  width: 0;
  margin: 0;
  padding: 0;
  order: ${(props) => props.$order || 'unset'};
`;

const masonryRootStyles = (state: MasonryRootState) => css`
  display: flex;
  flex-flow: column wrap;
  align-content: flex-start;
  contain: ${state.ssr ? 'none' : 'strict'};
  height: var(--masonry-height, 'auto');

  ${state.spacing.map(([breakpoint, spacing]) =>
    breakpoint === -1
      ? css`
          --masonry-spacing: ${spacing}px;
        `
      : css`
          @media screen and (max-width: ${breakpoint}px) {
            --masonry-spacing: ${spacing}px;
          }
        `
  )}

  ${state.columns.map(([breakpoint, column]) =>
    breakpoint === -1
      ? css`
          --masonry-column: ${column};
        `
      : css`
          @media screen and (max-width: ${breakpoint}px) {
            --masonry-column: ${column};
          }
        `
  )}

  margin: calc(var(--masonry-spacing, 0px) / -2);

  & > :not(template, ${LineBreaks}, [hidden]) {
    margin: calc(var(--masonry-spacing, 0px) / 2);
    width: calc((1 / var(--masonry-column, 1)) * 100% - var(--masonry-spacing, 0px));
    order: calc(1 + var(--masonry-column, 1));
    contain: layout style paint;
  }

  ${state.ssr &&
  css`
    & > :not(template, ${LineBreaks}, [hidden]) {
      display: none;
    }
  `}
`;

function ptn(val: string) {
  return Number(val.replace('px', ''));
}

export const Masonry = forwardRef<HTMLElement, MasonryInnerProps>(function Masonry(
  { children, className, as = 'div', columns = 1, spacing = 0, defaultHeight = 0, disableSSR = false, ...rest },
  ref
) {
  const masonryRef = useRef<HTMLElement>(null!);
  useImperativeHandle(ref, () => masonryRef.current);

  const [isSSR, setSSR] = useState(!disableSSR);
  useEffect(() => () => setSSR(false), []);

  const maxColumnHeightRef = useRef(defaultHeight);
  const maxColumnHeight = maxColumnHeightRef.current;

  const maxColumnCount = typeof columns === 'number' ? columns : Math.max(...Object.values(columns));

  const state = useMemo<MasonryRootState>(
    () => ({
      ssr: isSSR,
      columns: _entries(columns),
      spacing: _entries(spacing),
    }),
    [isSSR, columns, spacing]
  );

  useLayoutEffect(() => {
    if (typeof ResizeObserver === 'undefined') {
      return;
    }

    if (typeof MutationObserver === 'undefined') {
      return;
    }

    /**
     * FIXME safari will trigger `ResizeObserver loop completed
     * with undelivered notifications` error in console
     **/
    const resizeObserver = new ResizeObserver(() => {
      resizeObserver.unobserve(masonryRef.current);
      const result = handleResize(masonryRef.current, true);
      const { height = 0 } = result || {};
      maxColumnHeightRef.current = height;
      masonryRef.current.style.setProperty('--masonry-height', height ? `${height}px` : 'auto');
    });

    if (masonryRef.current) {
      masonryRef.current.childNodes.forEach((child) => {
        if (child instanceof HTMLElement && child.dataset.class !== 'line-break') {
          resizeObserver.observe(child);
        }
      });
    }

    const mutationObserver = new MutationObserver((mutations) => {
      mutations.forEach((mutation) => {
        if (mutation.type !== 'childList') {
          return;
        }
        mutation.addedNodes.forEach((node) => {
          if (node instanceof HTMLElement && node.dataset.class !== 'line-break') {
            resizeObserver.observe(node);
          }
        });
        mutation.removedNodes.forEach((node) => {
          if (node instanceof HTMLElement && node.dataset.class !== 'line-break') {
            resizeObserver.unobserve(node);
          }
        });
        if (mutation.addedNodes.length === 0 && mutation.removedNodes.length > 0) {
          // this situation won't trigger resizeObserver callback, so
          // manually trigger it here
          resizeObserver.observe(masonryRef.current);
        }
      });
    });

    mutationObserver.observe(masonryRef.current, {
      childList: true,
      subtree: false,
      attributes: false,
      characterData: false,
    });

    return () => {
      resizeObserver.disconnect();
      mutationObserver.disconnect();
    };
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, []);

  return (
    <div
      {...rest}
      ref={masonryRef as React.Ref<HTMLDivElement>}
      className={className}
      css={masonryRootStyles(state)}
      style={
        {
          ...rest.style,
          '--masonry-height': maxColumnHeight ? `${maxColumnHeight}px` : 'auto',
        } as React.CSSProperties
      }
    >
      {children}
      {new Array(maxColumnCount - 1).fill('').map((_, index) => (
        <LineBreaks key={index} data-class='line-break' $order={index + 1} />
      ))}
    </div>
  );
}) as unknown as MasonryRootType;

function handleResize(masonry: HTMLElement | undefined, isResize = false) {
  if (!masonry || masonry.childElementCount === 0) {
    return;
  }

  const masonryFirstChild = masonry.firstElementChild;
  const parentWidth = masonry.clientWidth;
  const firstChildWidth = masonryFirstChild?.clientWidth || 0;

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

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

  const currentNumberOfColumns = Math.round(parentWidth / (firstChildWidth + firstChildMarginLeft + firstChildMarginRight));

  const columnHeights = new Array(currentNumberOfColumns).fill(0) as number[];
  let skip = false;

  masonry.childNodes;

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

    const childComputedStyle = getComputedStyle(child);
    if (childComputedStyle.display === 'none') {
      return; // display: noneのアイテムをスキップ
    }

    const childMarginTop = ptn(childComputedStyle.marginTop);
    const childMarginBottom = ptn(childComputedStyle.marginBottom);
    const parsedChildHeight = ptn(childComputedStyle.height);
    const childHeight = parsedChildHeight ? Math.ceil(parsedChildHeight) + childMarginTop + childMarginBottom : 0;

    if (childHeight === 0) {
      // if any one of children isn't rendered yet, masonry's height shouldn't be computed yet
      skip = true;
      return;
    }

    // if there is a nested image that isn't rendered yet, masonry's height shouldn't be computed yet
    for (let i = 0; i < child.childNodes.length; i += 1) {
      const nestedChild = child.childNodes[i] as Element;
      if (nestedChild.tagName === 'IMG' && nestedChild.clientHeight === 0) {
        skip = true;
        break;
      }
    }

    if (!skip) {
      // find the current shortest column (where the current item will be placed)
      const currentMinColumnIndex = columnHeights.indexOf(Math.min(...columnHeights));

      if (isResize) {
        const oldOrder = Number(child.style.order);
        const newOrder = currentMinColumnIndex + 1;
        if (isFinite(oldOrder) && oldOrder !== newOrder) {
          /** debounce order change for 5px difference */
          if (Math.abs(columnHeights[oldOrder - 1] - columnHeights[newOrder - 1]) < 5) {
            columnHeights[oldOrder - 1] += childHeight;
            return;
          }
        }
      }

      columnHeights[currentMinColumnIndex] += childHeight;
      const order = currentMinColumnIndex + 1;
      child.style.order = String(order);
    }
  });

  if (!skip) {
    const numOfLineBreaks = currentNumberOfColumns > 0 ? currentNumberOfColumns - 1 : 0;
    return {
      height: Math.max(...columnHeights),
      numOfLineBreaks,
    };
  }
}

export default Masonry;

function _entries(values: Breakpoint<number> | number) {
  return Object.entries(typeof values === 'number' ? { default: values } : values)
    .reverse()
    .map(([breakpoint, column]) => [breakpoint === 'default' ? -1 : parseInt(breakpoint) - 1, column] as const);
}
KrishyV commented 2 months ago

What has personally worked for me, was to wrap my child components within the Masonry with a div with flexShrink 0.

Resize Observer loop limit no longer being hit, and the components are not stuck in a row. They overflow into columns as expected.