feat: distribute children by height

cedricdelpoux / react-responsive-masonry

React responsive masonry component built with css flexbox

https://cedricdelpoux.github.io/react-responsive-masonry/

MIT License

381 stars 37 forks source link

feat: distribute children by height #123

Closed yangchristina closed 1 month ago

yangchristina commented 4 months ago

Resolves #117

Before:

After:

Uses a greedy algorithm - this keeps items in a similar order as what was passed in. For the best column distribution you probably need some form of sorting the children like here. You could also use something like MUI's masonry's sequential prop to use the old distribution algorithm.

This is my first time really working with react class components, so I'm not sure if this is a good method. I couldn't really figure out how to get the demo running (had some errors), so I just tested on my own project instead.

Sorry the commit name is misleading, this is not sequential

cedricdelpoux commented 3 months ago

Hello @yangchristina . Thanks for your help! I tried you branch but the ResponsiveMasonry demo is broken. localhost_1190_

yangchristina commented 3 months ago

@cedricdelpoux I've fixed it. I was able to run the demo before merging with master, so I've confirmed it works, but I wasn't able to figure out running it after merging, so I can't confirm whether it works or not with the new updates.

piszczu4 commented 3 months ago

Can we merge it?

cedricdelpoux commented 2 months ago

Sorry for the delay. @yangchristina can you resolve conflicts? After that I will merge. Thank you

yangchristina commented 2 months ago

@cedricdelpoux I don't see any conflicts?

cedricdelpoux commented 2 months ago

@yangchristina

Screenshot 2024-09-16 at 11 14 36

cedricdelpoux commented 2 months ago

@yangchristina I think it's because your first commit Merge remote-tracking branch 'upstream/master'.

Could you rebase your branch on master and apply only your 3 last commits? Or better squash your 3 last commit in just one?

Thank you

fostergn commented 2 months ago

Looking forward to this so I wanted to bump. Happy to help rebase

yangchristina commented 2 months ago

@fostergn thanks for the reminder, I'd forgotten about this. @cedricdelpoux I've rebased it, is this okay? I still can't run it, so would you mind checking if this is still fine?

cedricdelpoux commented 2 months ago

@yangchristina Sorry I just tried your branch and the ResponsiveMasonry demo is still broken. The second and third column are in the DOM but empty. Screenshot 2024-09-23 at 12 17 35

The same demo on master works as expected like here.

you need to run yarn start to run the demo locally

yangchristina commented 2 months ago

@cedricdelpoux That's what I've been trying but it doesn't work. It just terminates immediately

Before, I got it to run by using node 16, but now even that isn't working.

What node and yarn versions are you using?

cedricdelpoux commented 1 month ago

node v22.3.0 yarn 1.22.21

nathandaven commented 1 month ago

any update here? Would love this feature!

yangchristina commented 1 month ago

@cedricdelpoux Should be fixed now, distributing children by height will only occur when the height of every child is > 0 The problem before was that distributeChildren was running before images had loaded, so they all had a height of 0

Not sure if this is a good solution, since it only reorders things after every image has loaded, so I added a sequential prop to use old behaviour

Running it worked when I ran yarn set version 1.22.21

cedricdelpoux commented 1 month ago

Thanks! Published in 2.4.0