@title
} case GuVideoBlockElement(video, media, data) => { - @EmailVideoImage.bestFor(media).map { imageUrl => + @EmailVideoImage.bestSrcFor(media).map { imageUrl => @row(Seq("no-pad")) { @data.get("url").fold { @imgForArticle(imageUrl, data.get("alt")) @@ -131,7 +131,7 @@@title
case ContentAtomBlockElement(atomId) => { @page.article.content.media.find(_.id == atomId).map { atom: MediaAtom => - @atom.posterImage.flatMap(EmailVideoImage.bestFor).map { imageUrl => + @atom.posterImage.flatMap(EmailVideoImage.bestSrcFor).map { imageUrl => @atom.assets.map { asset => @row(Seq("padded")) { @asset.platform match { diff --git a/article/app/views/fragments/emailMainMedia.scala.html b/article/app/views/fragments/emailMainMedia.scala.html index 4f95f7c8f6e0..f900e99be029 100644 --- a/article/app/views/fragments/emailMainMedia.scala.html +++ b/article/app/views/fragments/emailMainMedia.scala.html @@ -6,7 +6,7 @@ @if(!article.elements.hasMainEmbed) { @article.elements.mainPicture.map { picture => - @EmailImage.bestFor(picture.images).map { url => + @EmailImage.bestSrcFor(picture.images).map { url => @row(Seq("no-pad")) { true, "vjs-big-play-button" -> !expired))" style="background-image: url(@Video700.bestFor(image))">
+
true, "vjs-big-play-button" -> !expired))" style="background-image: url(@Video700.bestSrcFor(image))">
@if(!expired) {
@if(mediaWrapper.contains(ImmersiveMainMedia)) {
diff --git a/common/app/views/fragments/containers/facia_cards/videoContainer.scala.html b/common/app/views/fragments/containers/facia_cards/videoContainer.scala.html
index 298ec1a4b8e4..3403f93e1a89 100644
--- a/common/app/views/fragments/containers/facia_cards/videoContainer.scala.html
+++ b/common/app/views/fragments/containers/facia_cards/videoContainer.scala.html
@@ -72,7 +72,7 @@
@InlineImage.fromFaciaContent(item).map { fallbackImage =>
![]()
1) {data-}src="@Video700.bestFor(fallbackImage.imageMedia)" class="js-video-playlist-image js-video-playlist-image--@{
+ @if(index > 1) {data-}src="@Video700.bestSrcFor(fallbackImage.imageMedia)" class="js-video-playlist-image js-video-playlist-image--@{
index
}" />
}
diff --git a/common/app/views/fragments/imageFigure.scala.html b/common/app/views/fragments/imageFigure.scala.html
index 2188e3cad606..41a7b8c88517 100644
--- a/common/app/views/fragments/imageFigure.scala.html
+++ b/common/app/views/fragments/imageFigure.scala.html
@@ -37,9 +37,12 @@
@if(isMain) {
}
-
-
-
+
+ @defining(Item700.bestFor(picture)) { asset =>
+
+
+
+ }
@defining(
if (isMain) {
diff --git a/common/app/views/support/EmailHelpers.scala b/common/app/views/support/EmailHelpers.scala
index 184f772b5797..d2386715e819 100644
--- a/common/app/views/support/EmailHelpers.scala
+++ b/common/app/views/support/EmailHelpers.scala
@@ -10,7 +10,7 @@ object EmailHelpers {
def imageUrlFromCard(contentCard: ContentCard, width: Int): Option[String] = {
for {
InlineImage(imageMedia) <- contentCard.displayElement
- url <- SmallFrontEmailImage(width).bestFor(imageMedia)
+ url <- SmallFrontEmailImage(width).bestSrcFor(imageMedia)
} yield url
}
diff --git a/common/app/views/support/Profile.scala b/common/app/views/support/Profile.scala
index 92d4b7995f92..6754b2ae44f3 100644
--- a/common/app/views/support/Profile.scala
+++ b/common/app/views/support/Profile.scala
@@ -20,23 +20,28 @@ sealed trait ElementProfile {
def isPng: Boolean
def autoFormat: Boolean
- def elementFor(image: ImageMedia): Option[ImageAsset] = {
- val sortedCrops = image.imageCrops.sortBy(-_.width)
- width.flatMap{ desiredWidth =>
- sortedCrops.find(_.width >= desiredWidth)
- }.orElse(image.largestImage)
+ private def toSrc(maybeAsset: Option[ImageAsset]): Option[String] =
+ maybeAsset.flatMap(_.url).map(ImgSrc(_, this))
+
+ def bestFor(image: ImageMedia): Option[ImageAsset] = {
+ if(ImageServerSwitch.isSwitchedOn) {
+ val sortedCrops = image.imageCrops.sortBy(-_.width)
+ width.flatMap{ desiredWidth =>
+ sortedCrops.find(_.width >= desiredWidth)
+ }.orElse(image.largestImage)
+ }
+ else image.largestImage
}
+ def bestSrcFor(image: ImageMedia): Option[String] = toSrc(bestFor(image))
def largestFor(image: ImageMedia): Option[ImageAsset] = image.largestImage
-
- def bestFor(image: ImageMedia): Option[String] =
- elementFor(image).flatMap(_.url).map{ url => ImgSrc(url, this) }
+ def largestSrcFor(image: ImageMedia): Option[String] = toSrc(largestFor(image))
def captionFor(image: ImageMedia): Option[String] =
- elementFor(image).flatMap(_.caption)
+ bestFor(image).flatMap(_.caption)
def altTextFor(image: ImageMedia): Option[String] =
- elementFor(image).flatMap(_.altText)
+ bestFor(image).flatMap(_.altText)
// NOTE - if you modify this in any way there is a decent chance that you decache all our images :(
val qualityparam = if (hidpi) {"q=20"} else {"q=55"}
@@ -100,8 +105,6 @@ object Item1200 extends Profile(width = Some(1200))
object Video640 extends VideoProfile(width = Some(640), height = Some(360)) // 16:9
object Video700 extends VideoProfile(width = Some(700), height = Some(394)) // 16:9
object Video1280 extends VideoProfile(width = Some(1280), height = Some(720)) // 16:9
-
-
object GoogleStructuredData extends Profile(width = Some(300), height = Some(300)) // 1:1
abstract class ShareImage(shouldIncludeOverlay: Boolean) extends Profile(width = Some(1200)) {
@@ -202,18 +205,6 @@ object ImgSrc extends Logging with implicits.Strings {
}
}
- def findNearestSrc(ImageElement: ImageMedia, profile: Profile): Option[String] = {
- profile.elementFor(ImageElement).flatMap(_.url).map{ largestImage =>
- ImgSrc(largestImage, profile)
- }
- }
-
- def findLargestSrc(ImageElement: ImageMedia, profile: Profile): Option[String] = {
- profile.largestFor(ImageElement).flatMap(_.url).map{ largestImage =>
- ImgSrc(largestImage, profile)
- }
- }
-
def srcset(imageContainer: ImageMedia, widths: WidthsByBreakpoint): String = {
widths.profiles.map { profile => srcsetForProfile(profile, imageContainer, hidpi = false) } mkString ", "
}
@@ -231,38 +222,25 @@ object ImgSrc extends Logging with implicits.Strings {
.mkString(", ")
}
- def srcsetForProfile(profile: Profile, imageContainer: ImageMedia, hidpi: Boolean): String = {
- if(ImageServerSwitch.isSwitchedOn) {
- s"${findLargestSrc(imageContainer, profile).get} ${profile.width.get * (if (hidpi) 2 else 1)}w"
- } else {
- s"${findNearestSrc(imageContainer, profile).get} ${profile.width.get * (if (hidpi) 2 else 1)}w"
- }
- }
+ def srcsetForProfile(profile: Profile, imageContainer: ImageMedia, hidpi: Boolean): String =
+ s"${profile.bestSrcFor(imageContainer).get} ${profile.width.get * (if (hidpi) 2 else 1)}w"
- def srcsetForProfile(profile: Profile, path: String, hidpi: Boolean): String = {
+ def srcsetForProfile(profile: Profile, path: String, hidpi: Boolean): String =
s"${ImgSrc(path, profile)} ${profile.width.get * (if (hidpi) 2 else 1)}w"
- }
- def getFallbackUrl(ImageElement: ImageMedia): Option[String] = {
- if(ImageServerSwitch.isSwitchedOn) {
- findLargestSrc(ImageElement, Item300)
- } else {
- findNearestSrc(ImageElement, Item300)
- }
- }
+ def getFallbackUrl(ImageElement: ImageMedia): Option[String] =
+ Item300.bestSrcFor(ImageElement)
- def getAmpImageUrl(ImageElement: ImageMedia): Option[String] = {
- findNearestSrc(ImageElement, Item620)
- }
+ def getAmpImageUrl(ImageElement: ImageMedia): Option[String] =
+ Item620.bestSrcFor(ImageElement)
- def getFallbackAsset(ImageElement: ImageMedia): Option[ImageAsset] = {
- Item300.elementFor(ImageElement)
- }
+ def getFallbackAsset(ImageElement: ImageMedia): Option[ImageAsset] =
+ Item300.bestFor(ImageElement)
}
object SeoThumbnail {
def apply(page: Page): Option[String] = page match {
- case content: ContentPage => content.item.elements.thumbnail.flatMap(thumbnail => Item620.bestFor(thumbnail.images))
+ case content: ContentPage => content.item.elements.thumbnail.flatMap(thumbnail => Item620.bestSrcFor(thumbnail.images))
case _ => None
}
}
diff --git a/common/app/views/support/cleaner/VideoEmbedCleaner.scala b/common/app/views/support/cleaner/VideoEmbedCleaner.scala
index 22c1d0f1f778..b5c572b48683 100644
--- a/common/app/views/support/cleaner/VideoEmbedCleaner.scala
+++ b/common/app/views/support/cleaner/VideoEmbedCleaner.scala
@@ -69,9 +69,7 @@ case class VideoEmbedCleaner(article: Article) extends HtmlCleaner {
element.getElementsByTag("source").remove()
// add the poster url
- video.map(_.images).flatMap(Item640.bestFor).map(_.toString()).foreach { url =>
- element.attr("poster", url)
- }
+ video.map(_.images).flatMap(Item640.bestSrcFor).foreach(element.attr("poster", _))
video.foreach { videoElement =>
videoElement.videos.encodings.map { encoding => {
diff --git a/common/test/views/support/ImgSrcTest.scala b/common/test/views/support/ImgSrcTest.scala
index 52ae5527c9ea..9f53f5691352 100644
--- a/common/test/views/support/ImgSrcTest.scala
+++ b/common/test/views/support/ImgSrcTest.scala
@@ -78,55 +78,55 @@ class ImgSrcTest extends FlatSpec with Matchers with OneAppPerSuite {
"ImgSrc" should "convert the URL of a static image to the resizing endpoint with a /static prefix" in {
ImageServerSwitch.switchOn()
- Item700.bestFor(image).get should startWith (s"$imageHost/img/static/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.jpeg?w=700&q=55&auto=format&usm=12&fit=max&s=")
+ Item700.bestSrcFor(image).get should startWith (s"$imageHost/img/static/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.jpeg?w=700&q=55&auto=format&usm=12&fit=max&s=")
}
it should "convert the URL of a media service to the resizing endpoint with a /media prefix" in {
ImageServerSwitch.switchOn()
- Item700.bestFor(mediaImage).get should startWith (s"$imageHost/img/media/knly7wcp46fuadowlsnitzpawm/437_0_3819_2291/1000.jpg?w=700&q=55&auto=format&usm=12&fit=max&s=")
+ Item700.bestSrcFor(mediaImage).get should startWith (s"$imageHost/img/media/knly7wcp46fuadowlsnitzpawm/437_0_3819_2291/1000.jpg?w=700&q=55&auto=format&usm=12&fit=max&s=")
}
it should "not convert the URL of the image if it is disabled" in {
ImageServerSwitch.switchOff()
- Item700.bestFor(image) should be (Some("http://static.guim.co.uk/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.jpeg"))
+ Item700.bestSrcFor(image) should be (Some("http://static.guim.co.uk/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.jpeg"))
}
it should "convert the URL of the image if it is a PNG (original image from static.guim.co.uk domain)" in {
ImageServerSwitch.switchOn()
val pngImage = ImageMedia.apply(Seq(ImageAsset.make(asset.copy(file = Some("http://static.guim.co.uk/sys-images/Guardian/Pix/contributor/2014/10/30/1414675415419/Jessica-Valenti-R.png")),0)))
- Item700.bestFor(pngImage).get should startWith (s"$imageHost/img/static/sys-images/Guardian/Pix/contributor/2014/10/30/1414675415419/Jessica-Valenti-R.png?w=700&q=55&auto=format&usm=12&fit=max&s=")
+ Item700.bestSrcFor(pngImage).get should startWith (s"$imageHost/img/static/sys-images/Guardian/Pix/contributor/2014/10/30/1414675415419/Jessica-Valenti-R.png?w=700&q=55&auto=format&usm=12&fit=max&s=")
}
it should "convert the URL of the image if it is a PNG (original image from static-secure.guim.co.uk domain)" in {
ImageServerSwitch.switchOn()
val pngImage = ImageMedia.apply(Seq(ImageAsset.make(asset.copy(file = Some("http://static-secure.guim.co.uk/sys-images/Guardian/Pix/contributor/2014/10/30/1414675415419/Jessica-Valenti-R.png")),0)))
- Item700.bestFor(pngImage).get should startWith (s"$imageHost/img/static/sys-images/Guardian/Pix/contributor/2014/10/30/1414675415419/Jessica-Valenti-R.png?w=700&q=55&auto=format&usm=12&fit=max&s=")
+ Item700.bestSrcFor(pngImage).get should startWith (s"$imageHost/img/static/sys-images/Guardian/Pix/contributor/2014/10/30/1414675415419/Jessica-Valenti-R.png?w=700&q=55&auto=format&usm=12&fit=max&s=")
}
it should "not convert the URL of the image if it is a GIF (we do not support animated GIF)" in {
ImageServerSwitch.switchOn()
val gifImage = ImageMedia.apply(Seq(ImageAsset.make(asset.copy(file = Some("http://static.guim.co.uk/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.gif")),0)))
- Item700.bestFor(gifImage) should be (Some("http://static.guim.co.uk/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.gif"))
+ Item700.bestSrcFor(gifImage) should be (Some("http://static.guim.co.uk/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.gif"))
}
it should "not convert the URL of the image if it is not one of ours" in {
ImageServerSwitch.switchOn()
val someoneElsesImage = ImageMedia(Seq(ImageAsset.make(asset.copy(file = Some("http://foo.co.uk/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.gif")),0)))
- Item700.bestFor(someoneElsesImage) should be (Some("http://foo.co.uk/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.gif"))
+ Item700.bestSrcFor(someoneElsesImage) should be (Some("http://foo.co.uk/sys-images/Guardian/Pix/pictures/2013/7/5/1373023097878/b6a5a492-cc18-4f30-9809-88467e07ebfa-460x276.gif"))
}
it should "convert the URL of a jpeg s3 upload to the resizing endpoint with a /uploads prefix" in {
ImageServerSwitch.switchOn()
- Item700.bestFor(s3UploadJpgImage).get should startWith (s"$imageHost/img/uploads/2016/02/10/Screen_Shot_2016-02-09_at_17.50.09.jpeg?w=700&q=55&auto=format&usm=12&fit=max&s=")
+ Item700.bestSrcFor(s3UploadJpgImage).get should startWith (s"$imageHost/img/uploads/2016/02/10/Screen_Shot_2016-02-09_at_17.50.09.jpeg?w=700&q=55&auto=format&usm=12&fit=max&s=")
}
it should "convert the URL of a png s3 upload to the resizing endpoint with a /uploads prefix" in {
ImageServerSwitch.switchOn()
- Item700.bestFor(s3UploadPNGImage).get should startWith (s"$imageHost/img/uploads/2016/02/04/gu.png?w=700&q=55&auto=format&usm=12&fit=max&s=")
+ Item700.bestSrcFor(s3UploadPNGImage).get should startWith (s"$imageHost/img/uploads/2016/02/04/gu.png?w=700&q=55&auto=format&usm=12&fit=max&s=")
}
it should "not convert the URL of a gif s3 upload (we do not support animated GIF)" in {
ImageServerSwitch.switchOn()
- Item700.bestFor(s3UploadGifImage) should be (Some(s"https://uploads.guim.co.uk/2016/02/04/meep.gif"))
+ Item700.bestSrcFor(s3UploadGifImage) should be (Some(s"https://uploads.guim.co.uk/2016/02/04/meep.gif"))
}
}
diff --git a/onward/app/views/fragments/mediaItem.scala.html b/onward/app/views/fragments/mediaItem.scala.html
index e72ee337dd7a..ef50a8e417fe 100644
--- a/onward/app/views/fragments/mediaItem.scala.html
+++ b/onward/app/views/fragments/mediaItem.scala.html
@@ -10,7 +10,7 @@
@content.trail.trailPicture.map{ imageMedia =>
- )
+ )
}
diff --git a/onward/app/views/fragments/richLinkBody.scala.html b/onward/app/views/fragments/richLinkBody.scala.html
index 2e80eb2f971f..6e1d9d50a55f 100644
--- a/onward/app/views/fragments/richLinkBody.scala.html
+++ b/onward/app/views/fragments/richLinkBody.scala.html
@@ -30,7 +30,7 @@
@if(content.content.cardStyle == Media || content.content.cardStyle == Feature || content.content.cardStyle == DefaultCardstyle) {
@content.trail.trailPicture.map { trailPictureContainer =>
- @Item460.bestFor(trailPictureContainer).map { imgSrc =>
+ @Item460.bestSrcFor(trailPictureContainer).map { imgSrc =>
-
+
@if(content.content.cardStyle == Media || content.content.cardStyle == Feature || content.content.cardStyle == DefaultCardstyle) {
@content.trail.trailPicture.map { trailPictureContainer =>
- @Item460.bestFor(trailPictureContainer).map { imgSrc =>
+ @Item460.bestSrcFor(trailPictureContainer).map { imgSrc =>