igordanchenko / react-photo-album

Responsive photo gallery component for React
https://react-photo-album.com
MIT License
567 stars 35 forks source link

Images are too big when there aren't enough to fill the row #121

Closed rigon closed 1 year ago

rigon commented 1 year ago

Describe the bug

The images become too big when there aren't enough of them to fill the row.

Expected behavior

Having an option to configure what to do when that occurs. I see three different configurations, I think you should decide which should go forward:

How to reproduce

Create a gallery with let's say one image, but the container for react-photo-album taking all the width available.

Screenshots / Logs

Current behavior: image

Expected: image

Additional context

No response

dawi commented 1 year ago

It seems to be out of scope of this library.

However there is a workaround. Maybe it helps:

https://github.com/igordanchenko/react-photo-album/discussions/67#discussioncomment-4561261

igordanchenko commented 1 year ago

@dawi thank you for linking that discussion!

@rigon the above workaround is probably the easiest and the most elegant solution for this edge case. Unfortunately, implementing this as a feature in the library isn't as straightforward because targetRowHeight can be a function of the containerWidth, which creates chicken-and-egg problem.

I'll go ahead and close this ticket. However, if someone is willing to propose a solution, I'd be happy to review a PR.

igordanchenko commented 1 year ago

I couldn't get this edge case out of my head, so I ended up implementing this feature. It did require some refactoring, but I'm happy with the result.

Here is how you can define maximum row height when there isn't enough photos to fill more than one row:

<PhotoAlbum
    layout="rows"
    photos={photos}
    targetRowHeight={150}
    rowConstraints={{ singleRowMaxHeight: 200 }}
/>

Usage notes: 1) singleRowMaxHeight must be greater than the targetRowHeight 2) while it's possible to define both targetRowHeight and singleRowMaxHeight as responsive parameters (functions of the containerWidth), I'd recommend exercising caution in this scenario

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 2.3.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

rigon commented 1 year ago

@igordanchenko I really appreciate that you took the time to address this issue. I already integrated it in the project I'm working on and does work like a charm. Thank you! 👏 👏