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 next and previous buttons to image content in a series #20462

Merged
merged 11 commits into from
Nov 1, 2018
32 changes: 30 additions & 2 deletions applications/app/controllers/ImageContentController.scala
Original file line number Diff line number Diff line change
@@ -1,24 +1,30 @@
package controllers

import com.gu.contentapi.client.Parameter
import com.gu.contentapi.client.model.SearchQueryBase
import com.gu.contentapi.client.model.v1.{ItemResponse, Content => ApiContent}
import common._
import conf.switches.Switches
import contentapi.ContentApiClient
import model._
import pages.ContentHtmlPage
import play.api.libs.ws.WSClient
import play.api.mvc._
import services.ImageQuery
import views.support.RenderOtherStatus
import play.api.libs.json._
import conf.Configuration.contentApi

import scala.concurrent.Future

case class ImageContentPage(image: ImageContent, related: RelatedContent) extends ContentPage {
override lazy val item = image
override lazy val item: ImageContent = image
}

class ImageContentController(
val contentApiClient: ContentApiClient,
val controllerComponents: ControllerComponents
val controllerComponents: ControllerComponents,
wsClient: WSClient
)(implicit context: ApplicationContext)
extends BaseController with RendersItemResponse with ImageQuery with Logging with ImplicitControllerExecutionContext {

Expand All @@ -39,4 +45,26 @@ class ImageContentController(

private def isSupported(c: ApiContent) = c.isImageContent
override def canRender(i: ItemResponse): Boolean = i.content.exists(isSupported)

def getNextLightboxJson(path: String, tag: String, direction: String): Action[AnyContent] = Action.async { implicit request =>

val capiQuery: ContentApiNavQuery = ContentApiNavQuery(currentId = path, direction=direction)
.tag(tag).showTags("all").showElements("image").pageSize(contentApi.nextPreviousPageSize)

contentApiClient.thriftClient.getResponse(capiQuery).map {
response =>
val lightboxJson = response.results.flatMap(result => Content(result) match {
case content: ImageContent => Some(content.lightBox.javascriptConfig)
case _ => None
})
Cached(CacheTime.Default)(JsonComponent(JsArray(lightboxJson)))
}
}
}

case class ContentApiNavQuery(parameterHolder: Map[String, Parameter] = Map.empty, currentId: String, direction: String)
extends SearchQueryBase[ContentApiNavQuery] {
def withParameters(parameterMap: Map[String, Parameter]): ContentApiNavQuery = copy(parameterMap)

override def pathSegment: String = s"content/$currentId/$direction"
}
8 changes: 8 additions & 0 deletions applications/conf/routes
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ GET /opt/reset
GET /service-worker.js controllers.WebAppController.serviceWorker()
GET /2015-06-24-manifest.json controllers.WebAppController.manifest()

GET /getnext/$tag<[\w\d-]*(/[\w\d-]*)?(/[\w\d-]*)?>/*path controllers.ImageContentController.getNextLightboxJson(path, tag, direction = "next")
GET /getprev/$tag<[\w\d-]*(/[\w\d-]*)?(/[\w\d-]*)?>/*path controllers.ImageContentController.getNextLightboxJson(path, tag, direction = "prev")

# Newspaper pages
GET /theguardian controllers.NewspaperController.latestGuardianNewspaper()
GET /theobserver controllers.NewspaperController.latestObserverNewspaper()
Expand Down Expand Up @@ -89,6 +92,8 @@ GET /$path<[\w\d-]*(/[\w\d-]*)?/gallery/.*>
GET /$path<[\w\d-]*(/[\w\d-]*)?/(cartoon|picture|graphic)/.*>.json controllers.ImageContentController.renderJson(path)
GET /$path<[\w\d-]*(/[\w\d-]*)?/(cartoon|picture|graphic)/.*> controllers.ImageContentController.render(path)



# Audio and Video paths
GET /$path<[\w\d-]*(/[\w\d-]*)?/(video|audio)/.*>/info.json controllers.MediaController.renderInfoJson(path)
GET /$path<[\w\d-]*(/[\w\d-]*)?/(video|audio)/.*>.json controllers.MediaController.renderJson(path)
Expand All @@ -108,6 +113,9 @@ GET /$path<info/2017/jul/26/interactive-test>
GET /$shortCode<p/[\w]+> controllers.ShortUrlsController.redirectShortUrl(shortCode)
GET /$shortCode<p/[\w]+>/:campaignCode controllers.ShortUrlsController.fetchCampaignAndRedirectShortCode(shortCode, campaignCode)


#GET /getprevious/$path<[\w\d-]*(/[\w\d-]*)?(/[\w\d-]*)?> controllers.IndexController.render(path)

# Index pages for tags
GET /$path<[\w\d-]*(/[\w\d-]*)?(/[\w\d-]*)?>/trails.json controllers.IndexController.renderTrailsJson(path)
GET /$path<[\w\d-]*(/[\w\d-]*)?(/[\w\d-]*)?>/trails controllers.IndexController.renderTrails(path)
Expand Down
2 changes: 1 addition & 1 deletion applications/test/ImageContentControllerTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import org.scalatest.{BeforeAndAfterAll, DoNotDiscover, FlatSpec, Matchers}
val cartoonUrl = "commentisfree/cartoon/2013/jul/15/iain-duncan-smith-benefits-cap"
val pictureUrl = "artanddesign/picture/2013/oct/08/photography"

lazy val imageContentController = new ImageContentController(testContentApiClient, play.api.test.Helpers.stubControllerComponents())
lazy val imageContentController = new ImageContentController(testContentApiClient, play.api.test.Helpers.stubControllerComponents(), wsClient)

"Image Content Controller" should "200 when content type is picture" in {
val result = imageContentController.render(pictureUrl)(TestRequest(pictureUrl))
Expand Down
2 changes: 2 additions & 0 deletions common/app/common/configuration.scala
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ class GuardianConfiguration extends Logging {
).flatten: _*
)
}

lazy val nextPreviousPageSize: Int = configuration.getIntegerProperty("content.api.nextPreviousPageSize").getOrElse(50)
}

object ophanApi {
Expand Down
4 changes: 3 additions & 1 deletion common/app/model/content.scala
Original file line number Diff line number Diff line change
Expand Up @@ -843,7 +843,9 @@ case class GenericLightbox(
"srcsets" -> JsString(ImgSrc.srcset(container.images, GalleryMedia.lightbox)),
"sizes" -> JsString(GalleryMedia.lightbox.sizes),
"ratio" -> Try(JsNumber(img.width.toDouble / img.height.toDouble)).getOrElse(JsNumber(1)),
"role" -> JsString(img.role.toString)
"role" -> JsString(img.role.toString),
"parentContentId" -> JsString(properties.id),
"id" ->JsString(properties.id) //duplicated to simplify lightbox logic
))
}
JsObject(Seq(
Expand Down
17 changes: 8 additions & 9 deletions static/src/fonts/hinting-off/kerning-off/ascii/fonts.css
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
For more information please visit Commercial Type at http://commercialtype.com or email us at info[at]commercialtype.com

Copyright (C) 2013 Schwartzco Inc.

*/


Expand Down Expand Up @@ -280,7 +280,7 @@
font-style: normal;
font-stretch: normal;
}

@font-face {
font-family: 'GuardianAgateSans1Web';
src: url('GuardianAgateSans1Web/GuardianAgateSans1Web-RegularItalic.eot');
Expand Down Expand Up @@ -389,7 +389,7 @@
font-style: normal;
font-stretch: normal;
}

@font-face {
font-family: 'GuardianEgyptianWeb';
src: url('GuardianEgyptianWeb/GuardianEgyptianWeb-SemiboldItalic.eot');
Expand All @@ -416,8 +416,8 @@
font-style: normal;
font-stretch: normal;
}



@font-face {
font-family: 'GuardianEgyptianWeb';
Expand All @@ -433,7 +433,7 @@
}

/* GUARDIAN TEXT EGYPTIAN */

@font-face {
font-family: 'GuardianTextEgyptianWeb';
src: url('GuardianTextEgyptianWeb/GuardianTextEgyptianWeb-Regular.eot');
Expand Down Expand Up @@ -488,7 +488,7 @@
}



@font-face {
font-family: 'GuardianTextEgyptianWeb';
src: url('GuardianTextEgyptianWeb/GuardianTextEgyptianWeb-Bold.eot');
Expand Down Expand Up @@ -679,7 +679,7 @@
font-style: normal;
font-stretch: normal;
}


@font-face {
font-family: 'GuardianTextSansWeb';
Expand Down Expand Up @@ -734,4 +734,3 @@
font-style: normal;
font-stretch: normal;
}

133 changes: 96 additions & 37 deletions static/src/javascripts/projects/common/modules/gallery/lightbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import endslateTpl from 'raw-loader!common/views/content/endslate.html';
import loaderTpl from 'raw-loader!common/views/content/loader.html';
import shareButtonTpl from 'raw-loader!common/views/content/share-button.html';
import { loadCssPromise } from 'lib/load-css-promise';
import fetch from 'lib/fetch';

type ImageJson = {
caption: string,
Expand Down Expand Up @@ -81,6 +82,7 @@ class GalleryLightbox {
bodyScrollPosition: number;
endslateEl: bonzo;
endslate: Object;
startIndex: number;

constructor(): void {
// CONFIG
Expand Down Expand Up @@ -295,16 +297,87 @@ class GalleryLightbox {
this.fsm.trigger(event, data);
}

loadGalleryfromJson(galleryJson: GalleryJson, startIndex: number): void {
this.index = startIndex;
static loadNextOrPrevious(
currentImageId: string,
direction: string
): Promise<Object> {
const pathPrefix = direction === 'forwards' ? 'getnext' : 'getprev';
const seriesTag = config
.get('page.nonKeywordTagIds', '')
.split(',')
.filter(tag => tag.includes('series'))[0];
const fetchUrl = `/${pathPrefix}/${seriesTag}/${currentImageId}`;
return fetch(fetchUrl)
.then(response => response.json())
.then(json =>
// filter out non-lightbox items in the series
json.filter(image => image.images.length > 0)
);
}

if (this.galleryJson && galleryJson.id === this.galleryJson.id) {
loadOrOpen(newGalleryJson: GalleryJson, index: number): void {
this.index = index;
if (
this.galleryJson &&
newGalleryJson.id === this.galleryJson.id &&
newGalleryJson.images.length === this.galleryJson.images.length
) {
this.trigger('open');
} else {
this.trigger('loadJson', galleryJson);
this.trigger('loadJson', newGalleryJson);
}
}

fetchSurroundingJson(galleryJson: GalleryJson): Promise<Object> {
// if this is an image page, load series of images into the lightbox
if (
config.get('page.contentType') === 'ImageContent' &&
galleryJson.images.length < 2
) {
// store current path with leading slash removed
const currentId = window.location.pathname.substring(1);
// fetch next and previous images and load them
return Promise.all([
GalleryLightbox.loadNextOrPrevious(currentId, 'forwards'),
GalleryLightbox.loadNextOrPrevious(currentId, 'backwards'),
])
Copy link
Contributor

@SiAdcock SiAdcock Oct 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naive question time 😊

Will this happen every time the lightbox is opened? Is there a mechanism to prevent us from loading next and previous images that have already been loaded?

Perhaps it would be better to move some of this logic into the FSM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The galleryJson.images.length < 2 will prevent this from running if the images have already been loaded, but maybe more stuff could be done in the FSM hmm I'll take a look

.then(([nextResult, previousResult]) => ({
next: nextResult.map(e => e.images[0]),
previous: previousResult.map(e => e.images[0]),
}))
.then(({ next, previous }) => {
// combine this image, and the ones before and after it, into one big array
const allImages = previous
.reverse()
.concat(galleryJson.images, next);
if (allImages.length < 2) {
bonzo([this.nextBtn, this.prevBtn]).hide();
$(
'.gallery-lightbox__progress',
this.lightboxEl
).hide();
}

galleryJson.images = allImages;
this.startIndex = previous.length + 1;

return galleryJson;
});
}

return Promise.resolve(galleryJson);
}

loadHtml(json: GalleryJson): void {
this.images = json.images || [];
const imagesHtml = json.images
.map((img, i) => this.generateImgHTML(img, i + 1))
.join('');
this.$contentEl.html(imagesHtml);
this.$images = $('.js-gallery-lightbox-img', this.$contentEl[0]);
this.$countEl.text(this.images.length);
}

loadSurroundingImages(index: number, count: number): void {
let imageContent;
let $img;
Expand Down Expand Up @@ -435,19 +508,7 @@ class GalleryLightbox {
},
loadJson(json: GalleryJson): void {
this.galleryJson = json;
this.images = json.images || [];
this.$countEl.text(this.images.length);

const imagesHtml = this.images
.map((img, i) => this.generateImgHTML(img, i + 1))
.join('');

this.$contentEl.html(imagesHtml);

this.$images = $(
'.js-gallery-lightbox-img',
this.$contentEl[0]
);
this.loadHtml(json);

if (this.showEndslate) {
this.loadEndslate();
Expand All @@ -459,14 +520,6 @@ class GalleryLightbox {
this.initSwipe();
}

if (this.galleryJson.images.length < 2) {
bonzo([this.nextBtn, this.prevBtn]).hide();
$(
'.gallery-lightbox__progress',
this.lightboxEl
).hide();
}

this.state = 'image';
},
},
Expand Down Expand Up @@ -612,11 +665,9 @@ const init = (): void => {
const images = config.get('page.lightboxImages');

if (images && images.images.length > 0) {
let lightbox;
const lightbox = new GalleryLightbox();
const galleryHash = window.location.hash;

let res;

bean.on(document.body, 'click', '.js-gallerythumbs', (e: Event) => {
e.preventDefault();

Expand All @@ -628,25 +679,33 @@ const init = (): void => {
const galleryIndex = Number.isNaN(parsedGalleryIndex)
? 1
: parsedGalleryIndex; // 1-based index
lightbox = lightbox || new GalleryLightbox();

lightbox.loadGalleryfromJson(images, galleryIndex);
lightbox.fetchSurroundingJson(images).then(allImages => {
const startIndex = lightbox.startIndex
? lightbox.startIndex
: galleryIndex;
lightbox.loadOrOpen(allImages, startIndex);
});
});

lightbox = lightbox || new GalleryLightbox();
const galleryId = `/${config.get('page.pageId')}`;
const match = /\?index=(\d+)/.exec(document.location.href);
const res = /^#(?:img-)?(\d+)$/.exec(galleryHash);

let lightboxIndex = 0;

if (match) {
// index specified so launch lightbox at that index
pushUrl({}, document.title, galleryId, true); // lets back work properly
lightbox.loadGalleryfromJson(images, parseInt(match[1], 10));
} else {
res = /^#(?:img-)?(\d+)$/.exec(galleryHash);
lightboxIndex = parseInt(match[1], 10);
} else if (res) {
lightboxIndex = parseInt(res[1], 10);
}

if (res) {
lightbox.loadGalleryfromJson(images, parseInt(res[1], 10));
}
if (lightboxIndex > 0) {
lightbox.fetchSurroundingJson(images).then(allImages => {
lightbox.loadOrOpen(allImages, lightboxIndex);
});
}
}
});
Expand Down