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

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

Closed
rigon opened this issue Aug 10, 2023 · 5 comments
Closed

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

rigon opened this issue Aug 10, 2023 · 5 comments
Labels
bug Something isn't working released Implemented and released

Comments

@rigon
Copy link

rigon commented Aug 10, 2023

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:

  • Allow to behave as before
  • Force rows height to never exceed targetRowHeight
  • Having another option where rows don't exceed the specified height, like maxRowHeight

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

@rigon rigon added the bug Something isn't working label Aug 10, 2023
@dawi
Copy link

dawi commented Aug 11, 2023

It seems to be out of scope of this library.

However there is a workaround. Maybe it helps:

#67 (reply in thread)

@igordanchenko
Copy link
Owner

@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
Copy link
Owner

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

🎉 This issue has been resolved in version 2.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot added the released Implemented and released label Aug 18, 2023
@rigon
Copy link
Author

rigon commented Aug 21, 2023

@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! 👏 👏

@github-actions github-actions bot locked and limited conversation to collaborators Sep 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working released Implemented and released
Projects
None yet
Development

No branches or pull requests

3 participants