-
Notifications
You must be signed in to change notification settings - Fork 4
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 FixedHeight and net7.0 #12
Conversation
Thanks for contributing! Targeting .net7 has been on my todo-list for a while :-). |
No problem, took a while to figure it out myself and it's possible I've overlooked a simpler solution. Yes, for hero images at the top of articles our design simply flexes the image container in from the sides, and keeps the height, thus changing aspect ratio from ~3.5 to ~1. This works well if we're running without any image resizing, css will crop the full size image for us. But the full size image is huge, and would greatly benefit from imageresizing and server side cropping on devices thats on a slow/metered network. Without the height attibute the browser selects the smallest size in the srcset, imagesharp gives us a smaller version, but the height is smaller too because default behavior without set height is "keep aspect ratio" I guess. So to fit in our now square-ish container the height must be stretched on the client before its cropped and now its blurry :( With both width and height attributes set, imagesharp with crop it before resizing and only the useful part of the image will be sent to the client without much adjustment there. |
added to readme. If I'm not mistaken this would override AspectRatio because the height attribute would already exist in PictureUtils.AddHeightQuery in PictureRenderer? |
Ok, I get it. And I think it's a valid use case. Thanks, I really appreciate it! |
This reverts commit 442a37a.
guess the doc and net7.0 is still relevant if/when you merge core PictureRenderer, so I'll let this PR live with that for now. |
Thanks! Latest version is on it's way to the Optimizely nuget feed. |
Hi,
took the liberty to create a pull request to this very handy code! We have a case where we need the height attribute to be equal across varying screens on top hero images. This is needed for Imagesharp to crop wide images on narrow, mobile screens, which otherwise turns very blurry because the height is stretched.
If it wasn't for Optimizely and contentref we could have just added the attribute ourselves to the path, that's why I added it here, not the main PictureRenderer. We'll do as you like of course.
Also added
net7.0
TFM just because Optimizely supports that now. Not needed or could be a separate PR.