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

Add ratio breakpoint functionality #32

Closed
wants to merge 10 commits into from

Conversation

aerni
Copy link
Contributor

@aerni aerni commented Jan 23, 2021

This PR implements issue #31.

You can now set different ratios for different breakpoints like so:

{{ responsive:image ratio="1/1" lg:ratio="16/9" 2xl:ratio="2/1" }}

Each breakpoint adds a source element with a media attribute containing a min-width media query. No longer do you have to hide/show images with CSS if you want to use different aspect ratios depending on screen size.

If you don't provide a default ratio, the image ratio falls back to the ratio of the original image.

{{ responsive:image lg:ratio="16/9" }}

This PR ended up in a much bigger refactor than I first anticipated. Please let me know what you think.

@aerni
Copy link
Contributor Author

aerni commented Jan 25, 2021

There's one drawback to note with this approach. As of now, it's not possible to set the width and height attributes on the individual source element, resulting in layout shifts for any breakpoint other than the default one. Adding these has been proposed but until then, this is currently a limitation of the solution.

@riasvdv
Copy link
Member

riasvdv commented Jan 25, 2021

Thanks a lot for all the work! I've merged your changes, but did a big refactor and added tests, so Github doesn't recognize the merge anymore.

@riasvdv riasvdv closed this Jan 25, 2021
@aerni
Copy link
Contributor Author

aerni commented Jan 25, 2021

Awesome and great refactor too! Excited about the new feature 🎉

@aerni aerni deleted the feature/art-direction branch February 2, 2021 08:55
@ncla ncla mentioned this pull request Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants