Always use correct image meta dimensions #17258
Conversation
|
PRbuilds results: Screenshots Exceptions (0) A11y validation (0) Apache Benchmark Load Testing --automated message |
| Json.obj( | ||
| "@type" -> "ImageObject", | ||
| "url" -> JsString(Item700.bestSrcFor(picture.images).getOrElse("")), | ||
| "height" -> asset.fold(0)(_.height), |
There was a problem hiding this comment.
Technically this isn't the true width even - it will just be Item700.width when imgix in use.
|
💎!
Just a question, as nothing is ever clear for me from looking at the code... Does "always" means "whenever we do currently" (there are places when we should but don't now)? Or does it truly mean "always" now? Almost totally unrelatedly, e.g. Lightbox and other places could gain from height-bound image sizes (instead of width-bound only). Maybe as slightly cheekily suggested by @rich-nguyen? Maybe by using the trick Oliver has found? |
| @@ -231,38 +222,25 @@ object ImgSrc extends Logging with implicits.Strings { | |||
| .mkString(", ") | |||
| } | |||
|
|
|||
| def srcsetForProfile(profile: Profile, imageContainer: ImageMedia, hidpi: Boolean): String = { | |||
| if(ImageServerSwitch.isSwitchedOn) { | |||
There was a problem hiding this comment.
Why don't you have to check for this anymore?
There was a problem hiding this comment.
It has been moved into bestFor
Sadly not, only in the case of image metadata urls. And due to another issue, they were already masters anyway - but by accident, not design. |
|
Seen on PROD (merged by @jfsoul 22 minutes and 55 seconds ago)
|
What does this change?
The original motivation for this PR is to change the metadata in both
structuredData/Image.scalaandimageFigure.scala.html. The image dimensions are now for the same image as the url - whereas before they could have been different (in practice they never would be due to another bug, which will be fixed in another PR).In doing this I wanted to simplify
Profile.scalaa bit so that the logic for whether we serve largest or best image for a profile is in one place (and callers of the class cannot pick and choose whether to follow that logic). That required a bit of renaming for clarity which is what the majority of changes here are.What is the value of this and can you measure success?
We always provide correct image metadata dimensions and serve masters if using imgix and
something elseif not. Thesomething elsewill be solidified in another PR, but it is intended to be the best crop from the grid for the provided profile size.Does this affect other platforms - Amp, Apps, etc?
Yes, AMP
Screenshots
Tested in CODE?
Will do