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

Bug 1531933 – Use proper image sizes #4885

Merged
merged 42 commits into from Apr 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
2cbb357
basic implementation of ds image component
gvn Mar 21, 2019
bd34725
progress
gvn Mar 26, 2019
fddb16c
Merge branch 'master' into bz-1531933
gvn Mar 26, 2019
b1d9dcf
responsive image workingish
gvn Mar 26, 2019
e61c035
adding ds image to all ds components
gvn Mar 27, 2019
56b59e3
remove comment
gvn Mar 28, 2019
2f65b36
adding memoization
gvn Mar 28, 2019
e2c96ed
wip
gvn Apr 1, 2019
bf31d4b
Merge branch 'master' into bz-1531933
gvn Apr 1, 2019
2cdf1f0
further utilize thumbor
gvn Apr 2, 2019
1377985
fallback to image_src if raw_image_src is missing
gvn Apr 3, 2019
68d3a03
lint
gvn Apr 3, 2019
044ea7b
use mozilla cdn
gvn Apr 3, 2019
10b7ab0
allow turning optimization on and off
gvn Apr 5, 2019
0775057
lint bustin
gvn Apr 5, 2019
0792807
all thumbs loading but not on moz cdn
gvn Apr 5, 2019
a752773
using moz cdn
gvn Apr 5, 2019
9a872e9
linttttt
gvn Apr 5, 2019
fb93af8
deprecate test
gvn Apr 5, 2019
9391ab0
Merge branch 'master' into bz-1531933
gvn Apr 5, 2019
856ec66
adding tests for cache
gvn Apr 5, 2019
7208b86
lazy load rough implementation
gvn Apr 9, 2019
72a702d
allow optimization
gvn Apr 9, 2019
8b9576e
measuring widths on intersection
gvn Apr 9, 2019
2c65419
lint
gvn Apr 9, 2019
90c4c24
lower threshold
gvn Apr 9, 2019
bc9c7a1
turning optimization back on
gvn Apr 9, 2019
17976fe
let images load when opening new tab
gvn Apr 9, 2019
12c382d
fallback to 450px images
gvn Apr 9, 2019
7f25ec1
lint
gvn Apr 9, 2019
ca36bf3
simplify URL construction
gvn Apr 10, 2019
9033239
threshold 0
gvn Apr 10, 2019
2cb1101
turn off impression observer after element is seen
gvn Apr 10, 2019
48d674d
remove listener on unmount
gvn Apr 10, 2019
a125ca4
adding dsimage jsx tests
gvn Apr 10, 2019
c58e40b
linttt
gvn Apr 10, 2019
13d189b
Merge branch 'master' into bz-1531933
gvn Apr 10, 2019
050abbc
using bg color
gvn Apr 10, 2019
d523a9e
removing compiled css
gvn Apr 10, 2019
dedda10
Merge branch 'master' into bz-1531933
gvn Apr 11, 2019
943b620
simplifying options since using defaults
gvn Apr 11, 2019
6b7ccb1
fix multiple intersections
gvn Apr 11, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
@@ -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
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
@@ -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
};
@@ -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
@@ -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};
@@ -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
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
@@ -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
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
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
@@ -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);
});
});
});
@@ -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");
});
});
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