Skip to content
This repository has been archived by the owner on Feb 29, 2020. It is now read-only.

Commit

Permalink
Bug 1531933 - Optimize image sizes & lazy load (#4885)
Browse files Browse the repository at this point in the history
  • Loading branch information
gvn committed Apr 11, 2019
1 parent 2f5f05d commit 400eacc
Show file tree
Hide file tree
Showing 14 changed files with 236 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export class CardGrid extends React.PureComponent {
pos={rec.pos}
campaignId={rec.campaign_id}
image_src={rec.image_src}
raw_image_src={rec.raw_image_src}
title={rec.title}
excerpt={rec.excerpt}
url={rec.url}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {actionCreators as ac} from "common/Actions.jsm";
import {DSImage} from "../DSImage/DSImage.jsx";
import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu";
import {ImpressionStats} from "../../DiscoveryStreamImpressionStats/ImpressionStats";
import React from "react";
Expand Down Expand Up @@ -36,7 +37,7 @@ export class DSCard extends React.PureComponent {
onLinkClick={!this.props.placeholder ? this.onLinkClick : undefined}
url={this.props.url}>
<div className="img-wrapper">
<div className="img" style={{backgroundImage: `url(${this.props.image_src}`}} />
<DSImage extraClassNames="img" source={this.props.image_src} rawSource={this.props.raw_image_src} />
</div>
<div className="meta">
<div className="info-wrap">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,13 @@ $excerpt-line-height: 20;
}

.img {
@include image-as-background;
height: 0;
padding-top: 50%; // 2:1 aspect ratio

img {
border-radius: 4px;
box-shadow: inset 0 0 0 0.5px $black-15;
}
}

.ds-card-link {
Expand Down
103 changes: 103 additions & 0 deletions content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import {cache} from "./cache";
import React from "react";
import ReactDOM from "react-dom";

export class DSImage extends React.PureComponent {
constructor(props) {
super(props);

this.onOptimizedImageError = this.onOptimizedImageError.bind(this);

this.state = {
isSeen: false,
optimizedImageFailed: false,
};
}

onSeen(entries) {
if (this.state) {
if (entries.some(entry => entry.isIntersecting)) {
if (this.props.optimize) {
this.setState({
containerWidth: ReactDOM.findDOMNode(this).clientWidth,
});
}

this.setState({
isSeen: true,
});

// Stop observing since element has been seen
this.observer.unobserve(ReactDOM.findDOMNode(this));
}
}
}

reformatImageURL(url, width) {
// Change the image URL to request a size tailored for the parent container width
// Also: force JPEG, quality 60, no upscaling, no EXIF data
// Uses Thumbor: https://thumbor.readthedocs.io/en/latest/usage.html
return `https://img-getpocket.cdn.mozilla.net/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${encodeURIComponent(url)}`;
}

componentDidMount() {
this.observer = new IntersectionObserver(this.onSeen.bind(this));
this.observer.observe(ReactDOM.findDOMNode(this));
}

componentWillUnmount() {
// Remove observer on unmount
if (this.observer) {
this.observer.unobserve(ReactDOM.findDOMNode(this));
}
}

render() {
const classNames = `ds-image${this.props.extraClassNames ? ` ${this.props.extraClassNames}` : ``}`;

let img;

if (this.state && this.state.isSeen) {
if (this.props.optimize && this.props.rawSource && !this.state.optimizedImageFailed) {
let source;
let source2x;

if (this.state && this.state.containerWidth) {
let baseSource = this.props.rawSource;

source = this.reformatImageURL(
baseSource,
cache.query(baseSource, this.state.containerWidth, `1x`)
);

source2x = this.reformatImageURL(
baseSource,
cache.query(baseSource, this.state.containerWidth * 2, `2x`)
);

img = (<img onError={this.onOptimizedImageError} src={source} srcSet={`${source2x} 2x`} />);
}
} else {
img = (<img src={this.props.source} />);
}
}

return (
<picture className={classNames}>{img}</picture>
);
}

onOptimizedImageError() {
// This will trigger a re-render and the unoptimized 450px image will be used as a fallback
this.setState({
optimizedImageFailed: true,
});
}
}

DSImage.defaultProps = {
source: null, // The current source style from Pocket API (always 450px)
rawSource: null, // Unadulterated image URL to filter through Thumbor
extraClassNames: null, // Additional classnames to append to component
optimize: true, // Measure parent container to request exact sizes
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
.ds-image {
display: block;
position: relative;

img {
background-color: var(--newtab-card-placeholder-color);
position: absolute;
top: 0;
width: 100%;
height: 100%;
object-fit: cover;
}
}
28 changes: 28 additions & 0 deletions content-src/components/DiscoveryStreamComponents/DSImage/cache.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// This cache is for tracking queued image source requests from DSImage instances and leveraging
// larger sizes of images to scale down in the browser instead of making additional
// requests for smaller sizes from the server.

let cache = {
query(url, size, set) {
// Create an empty set if none exists
// Need multiple cache sets because the browser decides what set to use based on pixel density
if (this.queuedImages[set] === undefined) {
this.queuedImages[set] = {};
}

let sizeToRequest; // The px width to request from Thumbor via query value

if (!this.queuedImages[set][url] || this.queuedImages[set][url] < size) {
this.queuedImages[set][url] = size;
sizeToRequest = size;
} else {
// Use the larger size already queued for download (and allow browser to scale down)
sizeToRequest = this.queuedImages[set][url];
}

return sizeToRequest;
},
queuedImages: {},
};

export {cache};
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {DSCard, PlaceholderDSCard} from "../DSCard/DSCard.jsx";
import {actionCreators as ac} from "common/Actions.jsm";
import {DSEmptyState} from "../DSEmptyState/DSEmptyState.jsx";
import {DSImage} from "../DSImage/DSImage.jsx";
import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu";
import {ImpressionStats} from "../../DiscoveryStreamImpressionStats/ImpressionStats";
import {List} from "../List/List.jsx";
Expand Down Expand Up @@ -75,7 +76,7 @@ export class Hero extends React.PureComponent {
onLinkClick={this.onLinkClick}
url={heroRec.url}>
<div className="img-wrapper">
<div className="img" style={{backgroundImage: `url(${heroRec.image_src})`}} />
<DSImage extraClassNames="img" source={heroRec.image_src} rawSource={heroRec.raw_image_src} />
</div>
<div className="meta">
<div className="header-and-excerpt">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ $card-header-in-hero-line-height: 20;
.ds-hero {
position: relative;

.img {
@include image-as-background;
}

header {
font-weight: 600;
}
Expand Down Expand Up @@ -100,6 +96,11 @@ $card-header-in-hero-line-height: 20;
.img {
height: 0;
padding-top: 50%; // 2:1 aspect ratio

img {
border-radius: 4px;
box-shadow: inset 0 0 0 0.5px $black-15;
}
}

.meta {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {actionCreators as ac} from "common/Actions.jsm";
import {connect} from "react-redux";
import {DSEmptyState} from "../DSEmptyState/DSEmptyState.jsx";
import {DSImage} from "../DSImage/DSImage.jsx";
import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu";
import {ImpressionStats} from "../../DiscoveryStreamImpressionStats/ImpressionStats";
import React from "react";
Expand Down Expand Up @@ -56,7 +57,7 @@ export class ListItem extends React.PureComponent {
<span className="ds-list-item-info">{this.props.domain}</span>
</p>
</div>
<div className="ds-list-image" style={{backgroundImage: `url(${this.props.image_src})`}} />
<DSImage extraClassNames="ds-list-image" source={this.props.image_src} rawSource={this.props.raw_image_src} />
<ImpressionStats
campaignId={this.props.campaignId}
rows={[{id: this.props.id, pos: this.props.pos}]}
Expand Down Expand Up @@ -99,6 +100,7 @@ export function _List(props) {
excerpt={rec.excerpt}
id={rec.id}
image_src={rec.image_src}
raw_image_src={rec.raw_image_src}
pos={rec.pos}
title={rec.title}
context={rec.context}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,14 @@ $item-line-height: 20;
}

.ds-list-image {
@include image-as-background;
height: $item-image-size;
margin-inline-start: $item-font-size * 1px;
min-height: $item-image-size;

img {
border-radius: 4px;
box-shadow: inset 0 0 0 0.5px $black-15;
}
}

&:hover {
Expand Down
1 change: 1 addition & 0 deletions content-src/styles/_activity-stream.scss
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ input {
@import '../components/DiscoveryStreamComponents/TopSites/TopSites';
@import '../components/DiscoveryStreamComponents/DSLinkMenu/DSLinkMenu';
@import '../components/DiscoveryStreamComponents/DSCard/DSCard';
@import '../components/DiscoveryStreamComponents/DSImage/DSImage';
@import '../components/DiscoveryStreamComponents/DSMessage/DSMessage';
@import '../components/DiscoveryStreamImpressionStats/ImpressionStats';
@import '../components/DiscoveryStreamComponents/DSEmptyState/DSEmptyState';
Expand Down
33 changes: 33 additions & 0 deletions test/unit/common/ImgCache.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import {cache} from "content-src/components/DiscoveryStreamComponents/DSImage/cache.js";

describe("ImgCache", () => {
beforeEach(() => {
// Wipe cache
cache.queuedImages = {};
});

describe("group", () => {
it("should create multiple cache sets", () => {
cache.query(`http://example.org/a.jpg`, 500, `1x`);
cache.query(`http://example.org/a.jpg`, 1000, `2x`);

assert.equal(typeof cache.queuedImages[`1x`], `object`);
assert.equal(typeof cache.queuedImages[`2x`], `object`);
});

it("should cache larger values", () => {
cache.query(`http://example.org/a.jpg`, 200, `1x`);
cache.query(`http://example.org/a.jpg`, 500, `1x`);

assert.equal(cache.queuedImages[`1x`][`http://example.org/a.jpg`], 500);
});

it("should return a larger resolution if already cached", () => {
cache.query(`http://example.org/a.jpg`, 500, `1x`);

let result = cache.query(`http://example.org/a.jpg`, 200, `1x`);

assert.equal(result, 500);
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import {DSImage} from "content-src/components/DiscoveryStreamComponents/DSImage/DSImage";
import {mount} from "enzyme";
import React from "react";

describe("Discovery Stream <DSImage>", () => {
it("should have a child with class ds-image", () => {
const img = mount(<DSImage />);
const child = img.find(".ds-image");

assert.lengthOf(child, 1);
});

it("should set proper sources if only `source` is available", () => {
const img = mount(<DSImage source="https://placekitten.com/g/640/480" />);

img.setState({
isSeen: true,
containerWidth: 640,
});

assert.equal(img.find("img").prop("src"), "https://placekitten.com/g/640/480");
});

it("should set proper sources if `rawSource` is available", () => {
const img = mount(<DSImage rawSource="https://placekitten.com/g/640/480" />);

img.setState({
isSeen: true,
containerWidth: 640,
});

assert.equal(img.find("img").prop("src"), "https://img-getpocket.cdn.mozilla.net/640x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/https%3A%2F%2Fplacekitten.com%2Fg%2F640%2F480");
assert.equal(img.find("img").prop("srcSet"), "https://img-getpocket.cdn.mozilla.net/1280x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/https%3A%2F%2Fplacekitten.com%2Fg%2F640%2F480 2x");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,6 @@ describe("<ListItem> presentation component", () => {
assert.lengthOf(anchors, 1);
});

it("should include an background image of props.image_src", () => {
const wrapper = shallow(<ListItem {...ValidListItemProps} />);

const imageStyle = wrapper.find(".ds-list-image").prop("style");
assert.propertyVal(imageStyle, "backgroundImage", `url(${ValidListItemProps.image_src})`);
});

it("should not contain 'span.ds-list-item-context' without props.context", () => {
const wrapper = shallow(<ListItem {...ValidListItemProps} />);

Expand Down

0 comments on commit 400eacc

Please sign in to comment.