From 2cbb35759e7f111ca2d92f429b91904857efb915 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Thu, 21 Mar 2019 11:38:39 -0700 Subject: [PATCH 01/37] basic implementation of ds image component --- .../DSImage/DSImage.jsx | 64 +++++++++++++++++++ .../DiscoveryStreamComponents/Hero/Hero.jsx | 3 +- 2 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx new file mode 100644 index 0000000000..07f9db3eb1 --- /dev/null +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -0,0 +1,64 @@ +import React from "react"; + +export class DSImage extends React.PureComponent { + constructor(props) { + super(props); + } + + // Change the image URL to request a size tailored for the parent container width + reformatImageURL(url, width) { + // Determine if URL matches Thumbor spec + // https://thumbor.readthedocs.io/en/latest/usage.html + if (url && width && url.match(/&resize=[^&]*/)) { + url = url.replace(/&resize=[^&]*/, `&resize=w${width}`); + } + + return url; + } + + componentDidMount() { + this.setState({ + parentContainerWidth: ReactDOM.findDOMNode(this).parentNode.clientWidth + }); + } + + render() { + const classNames = `ds-image${this.props.extraClassNames ? ` ${this.props.extraClassNames}` : ``}`; + + let element; + let source; + + if (this.state && this.state.parentContainerWidth) { + console.log(`parentContainerWidth is defined: ${this.state.parentContainerWidth}`); + source = this.reformatImageURL(this.props.source, this.state.parentContainerWidth); + } + + if (this.props.asBackground) { + element = ( +
+ ) + } else { + element = ( + + ) + } + + return element; + } +} + +DSImage.defaultProps = { + extraClassNames: null, + asBackground: false +} + +/* + +TODO: + +- switch over to Picture element +- detect retina and use 1x if not present +- force jpeg (?) +- enable lazy loading + +*/ diff --git a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx index 871ba81a42..b3bf9444bd 100644 --- a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx +++ b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx @@ -1,5 +1,6 @@ import {actionCreators as ac} from "common/Actions.jsm"; import {DSCard} from "../DSCard/DSCard.jsx"; +import {DSImage} from "../DSImage/DSImage.jsx"; import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu"; import {ImpressionStats} from "../../DiscoveryStreamImpressionStats/ImpressionStats"; import {List} from "../List/List.jsx"; @@ -77,7 +78,7 @@ export class Hero extends React.PureComponent { onLinkClick={this.onLinkClick} url={heroRec.url}>
-
+
From bd34725c94a30743ef3150859345f61014dff8ba Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 26 Mar 2019 13:04:41 -0700 Subject: [PATCH 02/37] progress --- .../components/DiscoveryStreamBase/DiscoveryStreamBase.jsx | 2 ++ .../components/DiscoveryStreamComponents/DSImage/DSImage.jsx | 1 + 2 files changed, 3 insertions(+) diff --git a/content-src/components/DiscoveryStreamBase/DiscoveryStreamBase.jsx b/content-src/components/DiscoveryStreamBase/DiscoveryStreamBase.jsx index 34ab8cc641..d819c7af53 100644 --- a/content-src/components/DiscoveryStreamBase/DiscoveryStreamBase.jsx +++ b/content-src/components/DiscoveryStreamBase/DiscoveryStreamBase.jsx @@ -121,6 +121,8 @@ export class _DiscoveryStreamBase extends React.PureComponent { items={component.properties.items} /> ); case "Hero": + console.log(component); + return ( = 9 ? `cards` : `list`} diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 07f9db3eb1..eb9859c75d 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -60,5 +60,6 @@ TODO: - detect retina and use 1x if not present - force jpeg (?) - enable lazy loading +- memoize images so that smaller versions aren't fetched unnecissarily */ From b1d9dcf310dec694dc698d654a7acc2a40bd02ab Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 26 Mar 2019 15:23:35 -0700 Subject: [PATCH 03/37] responsive image workingish --- .../DSImage/DSImage.jsx | 28 +++++++------------ .../DSImage/DSImage.scss | 14 ++++++++++ .../DiscoveryStreamComponents/Hero/Hero.jsx | 2 +- content-src/styles/_activity-stream.scss | 1 + 4 files changed, 26 insertions(+), 19 deletions(-) create mode 100644 content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index eb9859c75d..d850f50c8e 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -25,39 +25,31 @@ export class DSImage extends React.PureComponent { render() { const classNames = `ds-image${this.props.extraClassNames ? ` ${this.props.extraClassNames}` : ``}`; - let element; - let source; + let source, source2x; if (this.state && this.state.parentContainerWidth) { - console.log(`parentContainerWidth is defined: ${this.state.parentContainerWidth}`); source = this.reformatImageURL(this.props.source, this.state.parentContainerWidth); + source2x = this.reformatImageURL(this.props.source, this.state.parentContainerWidth * 2); } - if (this.props.asBackground) { - element = ( -
- ) - } else { - element = ( - - ) - } - - return element; + return ( + + + + ) } } DSImage.defaultProps = { - extraClassNames: null, - asBackground: false + extraClassNames: null } /* TODO: -- switch over to Picture element -- detect retina and use 1x if not present ++ switch over to Picture element ++ detect retina and use 1x if not present - force jpeg (?) - enable lazy loading - memoize images so that smaller versions aren't fetched unnecissarily diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss new file mode 100644 index 0000000000..3396aa1371 --- /dev/null +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss @@ -0,0 +1,14 @@ +.ds-image { + // border: 1px solid pink; + + display: block; + position: relative; + + img { + position: absolute; + top: 0; + width: 100%; + height: 100%; + object-fit: cover; + } +} diff --git a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx index b3bf9444bd..d9ca5c1b4c 100644 --- a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx +++ b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx @@ -78,7 +78,7 @@ export class Hero extends React.PureComponent { onLinkClick={this.onLinkClick} url={heroRec.url}>
- +
diff --git a/content-src/styles/_activity-stream.scss b/content-src/styles/_activity-stream.scss index dff0615f2e..4f073a513b 100644 --- a/content-src/styles/_activity-stream.scss +++ b/content-src/styles/_activity-stream.scss @@ -157,6 +157,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'; From e61c035f39d75bacc51eefeea62b917bcf36e499 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Wed, 27 Mar 2019 14:27:33 -0700 Subject: [PATCH 04/37] adding ds image to all ds components --- .../DiscoveryStreamBase.jsx | 2 -- .../DSCard/DSCard.jsx | 3 ++- .../DSCard/_DSCard.scss | 6 +++++- .../DSImage/DSImage.jsx | 18 +++++++++++++----- .../DiscoveryStreamComponents/Hero/_Hero.scss | 9 +++++---- .../DiscoveryStreamComponents/List/List.jsx | 3 ++- .../DiscoveryStreamComponents/List/_List.scss | 6 +++++- 7 files changed, 32 insertions(+), 15 deletions(-) diff --git a/content-src/components/DiscoveryStreamBase/DiscoveryStreamBase.jsx b/content-src/components/DiscoveryStreamBase/DiscoveryStreamBase.jsx index bd5349e191..a1545c0f49 100644 --- a/content-src/components/DiscoveryStreamBase/DiscoveryStreamBase.jsx +++ b/content-src/components/DiscoveryStreamBase/DiscoveryStreamBase.jsx @@ -122,8 +122,6 @@ export class _DiscoveryStreamBase extends React.PureComponent { items={component.properties.items} /> ); case "Hero": - console.log(component); - return ( = 9 ? `cards` : `list`} diff --git a/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx b/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx index bddb75a388..7f9c821c19 100644 --- a/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx @@ -1,5 +1,6 @@ import {actionCreators as ac} from "common/Actions.jsm"; import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu"; +import {DSImage} from "../DSImage/DSImage.jsx"; import {ImpressionStats} from "../../DiscoveryStreamImpressionStats/ImpressionStats"; import React from "react"; import {SafeAnchor} from "../SafeAnchor/SafeAnchor"; @@ -36,7 +37,7 @@ export class DSCard extends React.PureComponent { onLinkClick={this.onLinkClick} url={this.props.url}>
-
+
diff --git a/content-src/components/DiscoveryStreamComponents/DSCard/_DSCard.scss b/content-src/components/DiscoveryStreamComponents/DSCard/_DSCard.scss index 41c4752acd..4c0b6cf2f2 100644 --- a/content-src/components/DiscoveryStreamComponents/DSCard/_DSCard.scss +++ b/content-src/components/DiscoveryStreamComponents/DSCard/_DSCard.scss @@ -34,9 +34,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 { diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index d850f50c8e..cb600d4f93 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -32,11 +32,19 @@ export class DSImage extends React.PureComponent { source2x = this.reformatImageURL(this.props.source, this.state.parentContainerWidth * 2); } - return ( - - - - ) + let element = ( + + ); + + if (source && source2x) { + element = ( + + + + ); + } + + return element; } } diff --git a/content-src/components/DiscoveryStreamComponents/Hero/_Hero.scss b/content-src/components/DiscoveryStreamComponents/Hero/_Hero.scss index 99d7a5e236..ee057a0872 100644 --- a/content-src/components/DiscoveryStreamComponents/Hero/_Hero.scss +++ b/content-src/components/DiscoveryStreamComponents/Hero/_Hero.scss @@ -4,10 +4,6 @@ $card-header-in-hero-line-height: 20; .ds-hero { position: relative; - .img { - @include image-as-background; - } - header { font-weight: 600; } @@ -95,6 +91,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 { diff --git a/content-src/components/DiscoveryStreamComponents/List/List.jsx b/content-src/components/DiscoveryStreamComponents/List/List.jsx index b97ed0c50a..eb7897364f 100644 --- a/content-src/components/DiscoveryStreamComponents/List/List.jsx +++ b/content-src/components/DiscoveryStreamComponents/List/List.jsx @@ -1,6 +1,7 @@ import {actionCreators as ac} from "common/Actions.jsm"; import {connect} from "react-redux"; import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu"; +import {DSImage} from "../DSImage/DSImage.jsx"; import {ImpressionStats} from "../../DiscoveryStreamImpressionStats/ImpressionStats"; import React from "react"; import {SafeAnchor} from "../SafeAnchor/SafeAnchor"; @@ -55,7 +56,7 @@ export class ListItem extends React.PureComponent { {this.props.domain}

-
+ Date: Thu, 28 Mar 2019 09:47:15 -0700 Subject: [PATCH 05/37] remove comment --- .../components/DiscoveryStreamComponents/DSImage/DSImage.scss | 2 -- 1 file changed, 2 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss index 3396aa1371..a195196233 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss @@ -1,6 +1,4 @@ .ds-image { - // border: 1px solid pink; - display: block; position: relative; From 2f65b36dde39ddab199d99c105e801966617ff0c Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Thu, 28 Mar 2019 14:07:24 -0700 Subject: [PATCH 06/37] adding memoization --- .../DSImage/DSImage.jsx | 39 ++++++++++++------- .../DSImage/cache.js | 29 ++++++++++++++ 2 files changed, 54 insertions(+), 14 deletions(-) create mode 100644 content-src/components/DiscoveryStreamComponents/DSImage/cache.js diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index cb600d4f93..86770d06fb 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -1,4 +1,5 @@ import React from "react"; +import {cache} from "./cache"; export class DSImage extends React.PureComponent { constructor(props) { @@ -17,8 +18,14 @@ export class DSImage extends React.PureComponent { } componentDidMount() { + let parentMeasurement = ReactDOM.findDOMNode(this).parentNode.clientWidth; + + // Quirk: Whenever the `Network` tab is open in devtools this is sometimes too large + // This will keep it from ever being larger than the overall container width in edge cases + parentMeasurement = parentMeasurement > 936 ? 936 : parentMeasurement; + this.setState({ - parentContainerWidth: ReactDOM.findDOMNode(this).parentNode.clientWidth + parentContainerWidth: parentMeasurement }); } @@ -28,23 +35,26 @@ export class DSImage extends React.PureComponent { let source, source2x; if (this.state && this.state.parentContainerWidth) { - source = this.reformatImageURL(this.props.source, this.state.parentContainerWidth); - source2x = this.reformatImageURL(this.props.source, this.state.parentContainerWidth * 2); + source = this.reformatImageURL( + this.props.source, + cache.query(this.props.source, this.state.parentContainerWidth, `1x`) + ); + + source2x = this.reformatImageURL( + this.props.source, + cache.query(this.props.source, this.state.parentContainerWidth * 2, `2x`) + ); } - let element = ( - - ); + let img = null; if (source && source2x) { - element = ( - - - - ); + img = (); } - return element; + return ( + {img} + ); } } @@ -58,8 +68,9 @@ TODO: + switch over to Picture element + detect retina and use 1x if not present -- force jpeg (?) ++ memoize images so that smaller versions aren't fetched unnecissarily ++ 2 caches for 1x and 2x (because one set will not get loaded) +- force jpeg (need cleaner img urls - in progress) - enable lazy loading -- memoize images so that smaller versions aren't fetched unnecissarily */ diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/cache.js b/content-src/components/DiscoveryStreamComponents/DSImage/cache.js new file mode 100644 index 0000000000..8963de9283 --- /dev/null +++ b/content-src/components/DiscoveryStreamComponents/DSImage/cache.js @@ -0,0 +1,29 @@ +let cache = { + query(url, size, set) { + // console.log(`query: ${url} – ${size}`); + + // 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]; + } + + console.log(sizeToRequest); + return sizeToRequest; + }, + queuedImages: {} +}; + +window.imgcache = cache; // TODO: FOR DEBUG ONLY! + +export { cache }; From e2c96ed05b77429c673e1bf7093276f96d8b6f9d Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Mon, 1 Apr 2019 11:06:39 -0700 Subject: [PATCH 07/37] wip --- .../DiscoveryStreamComponents/DSImage/DSImage.jsx | 4 ++-- .../components/DiscoveryStreamComponents/DSImage/cache.js | 7 +++++-- .../components/DiscoveryStreamComponents/Hero/Hero.jsx | 3 +++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 86770d06fb..11801e9c61 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -11,7 +11,7 @@ export class DSImage extends React.PureComponent { // Determine if URL matches Thumbor spec // https://thumbor.readthedocs.io/en/latest/usage.html if (url && width && url.match(/&resize=[^&]*/)) { - url = url.replace(/&resize=[^&]*/, `&resize=w${width}`); + url = url.replace(/&resize=[^&]*$/, `&resize=w${width}`); } return url; @@ -71,6 +71,6 @@ TODO: + memoize images so that smaller versions aren't fetched unnecissarily + 2 caches for 1x and 2x (because one set will not get loaded) - force jpeg (need cleaner img urls - in progress) -- enable lazy loading +- enable lazy loading (probably follow-on ticket) */ diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/cache.js b/content-src/components/DiscoveryStreamComponents/DSImage/cache.js index 8963de9283..9f302f6a4a 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/cache.js +++ b/content-src/components/DiscoveryStreamComponents/DSImage/cache.js @@ -1,3 +1,7 @@ +// 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) { // console.log(`query: ${url} – ${size}`); @@ -18,12 +22,11 @@ let cache = { sizeToRequest = this.queuedImages[set][url]; } - console.log(sizeToRequest); return sizeToRequest; }, queuedImages: {} }; -window.imgcache = cache; // TODO: FOR DEBUG ONLY! +// window.imgcache = cache; // TODO: FOR DEBUG ONLY! export { cache }; diff --git a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx index d9ca5c1b4c..84694a567b 100644 --- a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx +++ b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx @@ -67,6 +67,9 @@ export class Hero extends React.PureComponent { type={`Hero`} /> ); + console.log(heroRec); + + return (
{this.props.title}
From 2cdf1f0fbe8711f487b4142750a6ed78ddbdea8f Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 2 Apr 2019 16:40:51 -0700 Subject: [PATCH 08/37] further utilize thumbor --- .../DSImage/DSImage.jsx | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 11801e9c61..ad7935ffff 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -7,21 +7,17 @@ export class DSImage extends React.PureComponent { } // 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 reformatImageURL(url, width) { - // Determine if URL matches Thumbor spec - // https://thumbor.readthedocs.io/en/latest/usage.html - if (url && width && url.match(/&resize=[^&]*/)) { - url = url.replace(/&resize=[^&]*$/, `&resize=w${width}`); - } - - return url; + return `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${encodeURIComponent(url)}`; } componentDidMount() { let parentMeasurement = ReactDOM.findDOMNode(this).parentNode.clientWidth; - // Quirk: Whenever the `Network` tab is open in devtools this is sometimes too large - // This will keep it from ever being larger than the overall container width in edge cases + // Quirk: Whenever the `Network` tab is open in devtools this is sometimes too large (eg: resolves to window width) + // This will keep it from ever being larger than the overall *container width* in these edge cases parentMeasurement = parentMeasurement > 936 ? 936 : parentMeasurement; this.setState({ @@ -46,7 +42,7 @@ export class DSImage extends React.PureComponent { ); } - let img = null; + let img; if (source && source2x) { img = (); @@ -70,7 +66,8 @@ TODO: + detect retina and use 1x if not present + memoize images so that smaller versions aren't fetched unnecissarily + 2 caches for 1x and 2x (because one set will not get loaded) -- force jpeg (need cleaner img urls - in progress) ++ force jpeg (need cleaner img urls - in progress) +- force aspect ratio? - enable lazy loading (probably follow-on ticket) */ From 1377985d14064c48ff5cbd316b7a7923aa550433 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Wed, 3 Apr 2019 10:59:26 -0700 Subject: [PATCH 09/37] fallback to image_src if raw_image_src is missing --- .../components/DiscoveryStreamComponents/CardGrid/CardGrid.jsx | 2 +- content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx | 2 +- content-src/components/DiscoveryStreamComponents/List/List.jsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/CardGrid/CardGrid.jsx b/content-src/components/DiscoveryStreamComponents/CardGrid/CardGrid.jsx index 43814e89c4..16d51a608a 100644 --- a/content-src/components/DiscoveryStreamComponents/CardGrid/CardGrid.jsx +++ b/content-src/components/DiscoveryStreamComponents/CardGrid/CardGrid.jsx @@ -17,7 +17,7 @@ export class CardGrid extends React.PureComponent { key={`dscard-${index}`} pos={rec.pos} campaignId={rec.campaign_id} - image_src={rec.image_src} + image_src={rec.raw_image_src || rec.image_src} title={rec.title} excerpt={rec.excerpt} url={rec.url} diff --git a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx index 84694a567b..1ab9040a2d 100644 --- a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx +++ b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx @@ -81,7 +81,7 @@ export class Hero extends React.PureComponent { onLinkClick={this.onLinkClick} url={heroRec.url}>
- +
diff --git a/content-src/components/DiscoveryStreamComponents/List/List.jsx b/content-src/components/DiscoveryStreamComponents/List/List.jsx index eb7897364f..87730b1a80 100644 --- a/content-src/components/DiscoveryStreamComponents/List/List.jsx +++ b/content-src/components/DiscoveryStreamComponents/List/List.jsx @@ -94,7 +94,7 @@ export function _List(props) { domain={rec.domain} excerpt={rec.excerpt} id={rec.id} - image_src={rec.image_src} + image_src={rec.raw_image_src || rec.image_src} pos={rec.pos} title={rec.title} context={rec.context} From 68d3a032393727d6c108feca72e861cdb2cf0ae6 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Wed, 3 Apr 2019 11:26:56 -0700 Subject: [PATCH 10/37] lint --- .../DSCard/DSCard.jsx | 4 +- .../DSImage/DSImage.jsx | 43 ++++++++----------- .../DSImage/cache.js | 8 +--- .../DiscoveryStreamComponents/Hero/Hero.jsx | 5 +-- .../DiscoveryStreamComponents/List/List.jsx | 4 +- 5 files changed, 25 insertions(+), 39 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx b/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx index 7f9c821c19..744d672e44 100644 --- a/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx @@ -1,6 +1,6 @@ import {actionCreators as ac} from "common/Actions.jsm"; -import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu"; import {DSImage} from "../DSImage/DSImage.jsx"; +import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu"; import {ImpressionStats} from "../../DiscoveryStreamImpressionStats/ImpressionStats"; import React from "react"; import {SafeAnchor} from "../SafeAnchor/SafeAnchor"; @@ -37,7 +37,7 @@ export class DSCard extends React.PureComponent { onLinkClick={this.onLinkClick} url={this.props.url}>
- +
diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index ad7935ffff..0884d299b0 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -1,34 +1,41 @@ -import React from "react"; import {cache} from "./cache"; +import React from "react"; +import ReactDOM from "react-dom"; export class DSImage extends React.PureComponent { - constructor(props) { - super(props); - } - // 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 reformatImageURL(url, width) { + // TODO: append: https://img-getpocket.cdn.mozilla.net/direct?url= return `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${encodeURIComponent(url)}`; } - componentDidMount() { + measureParentElementWidth() { let parentMeasurement = ReactDOM.findDOMNode(this).parentNode.clientWidth; // Quirk: Whenever the `Network` tab is open in devtools this is sometimes too large (eg: resolves to window width) // This will keep it from ever being larger than the overall *container width* in these edge cases parentMeasurement = parentMeasurement > 936 ? 936 : parentMeasurement; + return parentMeasurement; + } + + setParentContainerWidth(width) { this.setState({ - parentContainerWidth: parentMeasurement + parentContainerWidth: width, }); } + componentDidMount() { + this.setParentContainerWidth(this.measureParentElementWidth()); + } + render() { const classNames = `ds-image${this.props.extraClassNames ? ` ${this.props.extraClassNames}` : ``}`; - let source, source2x; + let source; + let source2x; if (this.state && this.state.parentContainerWidth) { source = this.reformatImageURL( @@ -45,7 +52,7 @@ export class DSImage extends React.PureComponent { let img; if (source && source2x) { - img = (); + img = (); } return ( @@ -55,19 +62,5 @@ export class DSImage extends React.PureComponent { } DSImage.defaultProps = { - extraClassNames: null -} - -/* - -TODO: - -+ switch over to Picture element -+ detect retina and use 1x if not present -+ memoize images so that smaller versions aren't fetched unnecissarily -+ 2 caches for 1x and 2x (because one set will not get loaded) -+ force jpeg (need cleaner img urls - in progress) -- force aspect ratio? -- enable lazy loading (probably follow-on ticket) - -*/ + extraClassNames: null, +}; diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/cache.js b/content-src/components/DiscoveryStreamComponents/DSImage/cache.js index 9f302f6a4a..25e84765f7 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/cache.js +++ b/content-src/components/DiscoveryStreamComponents/DSImage/cache.js @@ -4,8 +4,6 @@ let cache = { query(url, size, set) { - // console.log(`query: ${url} – ${size}`); - // 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) { @@ -24,9 +22,7 @@ let cache = { return sizeToRequest; }, - queuedImages: {} + queuedImages: {}, }; -// window.imgcache = cache; // TODO: FOR DEBUG ONLY! - -export { cache }; +export {cache}; diff --git a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx index 1ab9040a2d..1695075168 100644 --- a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx +++ b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx @@ -67,9 +67,6 @@ export class Hero extends React.PureComponent { type={`Hero`} /> ); - console.log(heroRec); - - return (
{this.props.title}
@@ -81,7 +78,7 @@ export class Hero extends React.PureComponent { onLinkClick={this.onLinkClick} url={heroRec.url}>
- +
diff --git a/content-src/components/DiscoveryStreamComponents/List/List.jsx b/content-src/components/DiscoveryStreamComponents/List/List.jsx index 87730b1a80..bf5a23bde8 100644 --- a/content-src/components/DiscoveryStreamComponents/List/List.jsx +++ b/content-src/components/DiscoveryStreamComponents/List/List.jsx @@ -1,7 +1,7 @@ import {actionCreators as ac} from "common/Actions.jsm"; import {connect} from "react-redux"; -import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu"; import {DSImage} from "../DSImage/DSImage.jsx"; +import {DSLinkMenu} from "../DSLinkMenu/DSLinkMenu"; import {ImpressionStats} from "../../DiscoveryStreamImpressionStats/ImpressionStats"; import React from "react"; import {SafeAnchor} from "../SafeAnchor/SafeAnchor"; @@ -56,7 +56,7 @@ export class ListItem extends React.PureComponent { {this.props.domain}

- + Date: Wed, 3 Apr 2019 11:37:00 -0700 Subject: [PATCH 11/37] use mozilla cdn --- .../DiscoveryStreamComponents/DSImage/DSImage.jsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 0884d299b0..4fc503ed86 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -7,8 +7,10 @@ export class DSImage extends React.PureComponent { // Also: force JPEG, quality 60, no upscaling, no EXIF data // Uses Thumbor: https://thumbor.readthedocs.io/en/latest/usage.html reformatImageURL(url, width) { - // TODO: append: https://img-getpocket.cdn.mozilla.net/direct?url= - return `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${encodeURIComponent(url)}`; + let image = `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${url}`; + + // Use Mozilla CDN: + return `https://img-getpocket.cdn.mozilla.net/direct?url=${encodeURIComponent(image)}`; } measureParentElementWidth() { From 10b7ab06b79a112df1414d4728a8959a538a0974 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Fri, 5 Apr 2019 10:25:03 -0700 Subject: [PATCH 12/37] allow turning optimization on and off --- .../CardGrid/CardGrid.jsx | 3 +- .../DSCard/DSCard.jsx | 2 +- .../DSImage/DSImage.jsx | 69 +++++++++++-------- .../DiscoveryStreamComponents/Hero/Hero.jsx | 2 +- .../DiscoveryStreamComponents/List/List.jsx | 5 +- 5 files changed, 48 insertions(+), 33 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/CardGrid/CardGrid.jsx b/content-src/components/DiscoveryStreamComponents/CardGrid/CardGrid.jsx index 16d51a608a..11e51b745f 100644 --- a/content-src/components/DiscoveryStreamComponents/CardGrid/CardGrid.jsx +++ b/content-src/components/DiscoveryStreamComponents/CardGrid/CardGrid.jsx @@ -17,7 +17,8 @@ export class CardGrid extends React.PureComponent { key={`dscard-${index}`} pos={rec.pos} campaignId={rec.campaign_id} - image_src={rec.raw_image_src || rec.image_src} + image_src={rec.image_src} + raw_image_src={rec.raw_image_src} title={rec.title} excerpt={rec.excerpt} url={rec.url} diff --git a/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx b/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx index 744d672e44..5794d2da40 100644 --- a/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSCard/DSCard.jsx @@ -37,7 +37,7 @@ export class DSCard extends React.PureComponent { onLinkClick={this.onLinkClick} url={this.props.url}>
- +
diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 4fc503ed86..d7ca9e116b 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -3,58 +3,66 @@ import React from "react"; import ReactDOM from "react-dom"; export class DSImage extends React.PureComponent { - // 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 reformatImageURL(url, width) { - let image = `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${url}`; + // 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 + let image = `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${encodeURIComponent(url)}`; // Use Mozilla CDN: - return `https://img-getpocket.cdn.mozilla.net/direct?url=${encodeURIComponent(image)}`; + return encodeURI(`https://img-getpocket.cdn.mozilla.net/direct?url=${image}`); } - measureParentElementWidth() { - let parentMeasurement = ReactDOM.findDOMNode(this).parentNode.clientWidth; + measureElementWidth() { + let width = ReactDOM.findDOMNode(this).clientWidth; // Quirk: Whenever the `Network` tab is open in devtools this is sometimes too large (eg: resolves to window width) // This will keep it from ever being larger than the overall *container width* in these edge cases - parentMeasurement = parentMeasurement > 936 ? 936 : parentMeasurement; + width = width > 936 ? 936 : width; - return parentMeasurement; + return width; } - setParentContainerWidth(width) { + setContainerWidth(width) { this.setState({ - parentContainerWidth: width, + containerWidth: width, }); } componentDidMount() { - this.setParentContainerWidth(this.measureParentElementWidth()); + if (this.props.optimize) { + this.setContainerWidth(this.measureElementWidth()); + } } render() { const classNames = `ds-image${this.props.extraClassNames ? ` ${this.props.extraClassNames}` : ``}`; - let source; - let source2x; + let img; - if (this.state && this.state.parentContainerWidth) { - source = this.reformatImageURL( - this.props.source, - cache.query(this.props.source, this.state.parentContainerWidth, `1x`) - ); + if (this.props.optimize && this.props.rawSource) { + let source; + let source2x; - source2x = this.reformatImageURL( - this.props.source, - cache.query(this.props.source, this.state.parentContainerWidth * 2, `2x`) - ); - } + if (this.state && this.state.containerWidth) { + let baseSource = this.props.rawSource; - let img; + source = this.reformatImageURL( + baseSource, + cache.query(baseSource, this.state.containerWidth, `1x`) + ); - if (source && source2x) { - img = (); + source2x = this.reformatImageURL( + baseSource, + cache.query(baseSource, this.state.containerWidth * 2, `2x`) + ); + + img = (); + } + } else { + console.log(`no raw source`); + + img = (); } return ( @@ -64,5 +72,10 @@ export class DSImage extends React.PureComponent { } DSImage.defaultProps = { - extraClassNames: null, + 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 + + // TODO: Turn this on once raw source is available and sanitized + optimize: true // Measure parent container to request exact sizes }; diff --git a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx index 1695075168..b2aa76c166 100644 --- a/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx +++ b/content-src/components/DiscoveryStreamComponents/Hero/Hero.jsx @@ -78,7 +78,7 @@ export class Hero extends React.PureComponent { onLinkClick={this.onLinkClick} url={heroRec.url}>
- +
diff --git a/content-src/components/DiscoveryStreamComponents/List/List.jsx b/content-src/components/DiscoveryStreamComponents/List/List.jsx index bf5a23bde8..7237962517 100644 --- a/content-src/components/DiscoveryStreamComponents/List/List.jsx +++ b/content-src/components/DiscoveryStreamComponents/List/List.jsx @@ -56,7 +56,7 @@ export class ListItem extends React.PureComponent { {this.props.domain}

- + Date: Fri, 5 Apr 2019 10:28:15 -0700 Subject: [PATCH 13/37] lint bustin --- .../DiscoveryStreamComponents/DSImage/DSImage.jsx | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index d7ca9e116b..62008b0746 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -60,8 +60,6 @@ export class DSImage extends React.PureComponent { img = (); } } else { - console.log(`no raw source`); - img = (); } @@ -72,10 +70,8 @@ export class DSImage extends React.PureComponent { } DSImage.defaultProps = { - source: null, // The current source style from Pocket API – always 450px + 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 - - // TODO: Turn this on once raw source is available and sanitized - optimize: true // Measure parent container to request exact sizes + optimize: true, // Measure parent container to request exact sizes }; From 07928070ef19e48c28112808724cbe4783e701c5 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Fri, 5 Apr 2019 11:53:17 -0700 Subject: [PATCH 14/37] all thumbs loading but not on moz cdn --- .../DiscoveryStreamComponents/DSImage/DSImage.jsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 62008b0746..6a9644344d 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -4,13 +4,19 @@ import ReactDOM from "react-dom"; export class DSImage extends React.PureComponent { reformatImageURL(url, width) { + let urlIsEncoded = (url !== decodeURIComponent(url) && url !== decodeURI(url)); + + url = urlIsEncoded ? url : encodeURIComponent(url); + // 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 - let image = `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${encodeURIComponent(url)}`; + url = `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${(url)}`; // Use Mozilla CDN: - return encodeURI(`https://img-getpocket.cdn.mozilla.net/direct?url=${image}`); + // return (`https://img-getpocket.cdn.mozilla.net/direct?url=${(url)}`); + + return url; } measureElementWidth() { From a7527736e1f98d3f600f194f694f426e12f30348 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Fri, 5 Apr 2019 12:08:19 -0700 Subject: [PATCH 15/37] using moz cdn --- .../DiscoveryStreamComponents/DSImage/DSImage.jsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 6a9644344d..41c230c0c4 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -4,8 +4,9 @@ import ReactDOM from "react-dom"; export class DSImage extends React.PureComponent { reformatImageURL(url, width) { - let urlIsEncoded = (url !== decodeURIComponent(url) && url !== decodeURI(url)); + const urlIsEncoded = url !== decodeURI(url); + // Encode the URL if it needs it url = urlIsEncoded ? url : encodeURIComponent(url); // Change the image URL to request a size tailored for the parent container width @@ -14,9 +15,7 @@ export class DSImage extends React.PureComponent { url = `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${(url)}`; // Use Mozilla CDN: - // return (`https://img-getpocket.cdn.mozilla.net/direct?url=${(url)}`); - - return url; + return (`https://img-getpocket.cdn.mozilla.net/direct?url=${encodeURIComponent(url)}`); } measureElementWidth() { From 9a872e9838bb84172e812b26785b3a6f42d22302 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Fri, 5 Apr 2019 12:20:21 -0700 Subject: [PATCH 16/37] linttttt --- .../DiscoveryStreamComponents/DSImage/DSImage.jsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 41c230c0c4..9192dcb393 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -7,15 +7,15 @@ export class DSImage extends React.PureComponent { const urlIsEncoded = url !== decodeURI(url); // Encode the URL if it needs it - url = urlIsEncoded ? url : encodeURIComponent(url); + let constructedURL = urlIsEncoded ? url : encodeURIComponent(url); // 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 - url = `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${(url)}`; + constructedURL = `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${(constructedURL)}`; // Use Mozilla CDN: - return (`https://img-getpocket.cdn.mozilla.net/direct?url=${encodeURIComponent(url)}`); + return `https://img-getpocket.cdn.mozilla.net/direct?url=${encodeURIComponent(constructedURL)}`; } measureElementWidth() { From fb93af8a510b0f43795f676a446ae2fa99a42b81 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Fri, 5 Apr 2019 13:55:21 -0700 Subject: [PATCH 17/37] deprecate test --- .../components/DiscoveryStreamComponents/List.test.jsx | 7 ------- 1 file changed, 7 deletions(-) diff --git a/test/unit/content-src/components/DiscoveryStreamComponents/List.test.jsx b/test/unit/content-src/components/DiscoveryStreamComponents/List.test.jsx index 60ad7d2c6c..37e38bb62e 100644 --- a/test/unit/content-src/components/DiscoveryStreamComponents/List.test.jsx +++ b/test/unit/content-src/components/DiscoveryStreamComponents/List.test.jsx @@ -102,13 +102,6 @@ describe(" presentation component", () => { assert.lengthOf(anchors, 1); }); - it("should include an background image of props.image_src", () => { - const wrapper = shallow(); - - 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(); From 856ec66a650501ad78e4592cc7d51e94ae37caf3 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Fri, 5 Apr 2019 15:01:49 -0700 Subject: [PATCH 18/37] adding tests for cache --- test/unit/common/ImgCache.test.js | 33 +++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 test/unit/common/ImgCache.test.js diff --git a/test/unit/common/ImgCache.test.js b/test/unit/common/ImgCache.test.js new file mode 100644 index 0000000000..b42785e53b --- /dev/null +++ b/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); + }); + }); +}); From 7208b86fc3b75257284ecd9880b82a44fe38a820 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 9 Apr 2019 11:22:45 -0700 Subject: [PATCH 19/37] lazy load rough implementation --- .../DSImage/DSImage.jsx | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 9192dcb393..b6e31eae59 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -3,6 +3,19 @@ import React from "react"; import ReactDOM from "react-dom"; export class DSImage extends React.PureComponent { + onSeen(element) { + // console.log(element); + + if (this.state && document.visibilityState === `visible` && ReactDOM.findDOMNode(this).clientHeight && element[0].isIntersecting) { + this.setState({ + isSeen: true, + timesSeen: this.state.timesSeen + 1 + }); + + console.log(this.state.timesSeen); + } + } + reformatImageURL(url, width) { const urlIsEncoded = url !== decodeURI(url); @@ -35,6 +48,21 @@ export class DSImage extends React.PureComponent { } componentDidMount() { + this.setState({ + isSeen: false, + timesSeen: 0 + }); + + let options = { + // rootMargin: `10px`, + root: document.querySelector(`document`), + threshold: 1, + } + + this.observer = new IntersectionObserver(this.onSeen.bind(this), options); + + this.observer.observe(ReactDOM.findDOMNode(this)); + if (this.props.optimize) { this.setContainerWidth(this.measureElementWidth()); } @@ -65,7 +93,10 @@ export class DSImage extends React.PureComponent { img = (); } } else { - img = (); + if (this.state && this.state.timesSeen > 0) + img = (); + else + img = null; } return ( @@ -78,5 +109,5 @@ 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 + optimize: false, // Measure parent container to request exact sizes }; From 72a702dd66ab4d2fc29d36db94544832cb638435 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 9 Apr 2019 11:34:42 -0700 Subject: [PATCH 20/37] allow optimization --- .../DSImage/DSImage.jsx | 47 ++++++++----------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index b6e31eae59..c5c083b35a 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -4,15 +4,10 @@ import ReactDOM from "react-dom"; export class DSImage extends React.PureComponent { onSeen(element) { - // console.log(element); - if (this.state && document.visibilityState === `visible` && ReactDOM.findDOMNode(this).clientHeight && element[0].isIntersecting) { this.setState({ - isSeen: true, - timesSeen: this.state.timesSeen + 1 + isSeen: true }); - - console.log(this.state.timesSeen); } } @@ -50,7 +45,6 @@ export class DSImage extends React.PureComponent { componentDidMount() { this.setState({ isSeen: false, - timesSeen: 0 }); let options = { @@ -73,30 +67,29 @@ export class DSImage extends React.PureComponent { let img; - if (this.props.optimize && this.props.rawSource) { - let source; - let source2x; + if (this.state && this.state.isSeen) { + if (this.props.optimize && this.props.rawSource) { + let source; + let source2x; - if (this.state && this.state.containerWidth) { - let baseSource = this.props.rawSource; + if (this.state && this.state.containerWidth) { + let baseSource = this.props.rawSource; - source = this.reformatImageURL( - baseSource, - cache.query(baseSource, this.state.containerWidth, `1x`) - ); + source = this.reformatImageURL( + baseSource, + cache.query(baseSource, this.state.containerWidth, `1x`) + ); - source2x = this.reformatImageURL( - baseSource, - cache.query(baseSource, this.state.containerWidth * 2, `2x`) - ); + source2x = this.reformatImageURL( + baseSource, + cache.query(baseSource, this.state.containerWidth * 2, `2x`) + ); - img = (); - } - } else { - if (this.state && this.state.timesSeen > 0) + img = (); + } + } else { img = (); - else - img = null; + } } return ( @@ -109,5 +102,5 @@ 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: false, // Measure parent container to request exact sizes + optimize: true, // Measure parent container to request exact sizes }; From 8b9576edfeb3d7bfd5776b7e4b9d71dde4245bde Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 9 Apr 2019 11:43:44 -0700 Subject: [PATCH 21/37] measuring widths on intersection --- .../DSImage/DSImage.jsx | 28 +++++-------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index c5c083b35a..2b6af426d0 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -4,7 +4,13 @@ import ReactDOM from "react-dom"; export class DSImage extends React.PureComponent { onSeen(element) { - if (this.state && document.visibilityState === `visible` && ReactDOM.findDOMNode(this).clientHeight && element[0].isIntersecting) { + if (this.state && document.visibilityState === `visible` && element[0].isIntersecting) { + if (this.props.optimize) { + this.setState({ + containerWidth: ReactDOM.findDOMNode(this).clientWidth, + }); + } + this.setState({ isSeen: true }); @@ -26,22 +32,6 @@ export class DSImage extends React.PureComponent { return `https://img-getpocket.cdn.mozilla.net/direct?url=${encodeURIComponent(constructedURL)}`; } - measureElementWidth() { - let width = ReactDOM.findDOMNode(this).clientWidth; - - // Quirk: Whenever the `Network` tab is open in devtools this is sometimes too large (eg: resolves to window width) - // This will keep it from ever being larger than the overall *container width* in these edge cases - width = width > 936 ? 936 : width; - - return width; - } - - setContainerWidth(width) { - this.setState({ - containerWidth: width, - }); - } - componentDidMount() { this.setState({ isSeen: false, @@ -56,10 +46,6 @@ export class DSImage extends React.PureComponent { this.observer = new IntersectionObserver(this.onSeen.bind(this), options); this.observer.observe(ReactDOM.findDOMNode(this)); - - if (this.props.optimize) { - this.setContainerWidth(this.measureElementWidth()); - } } render() { From 2c65419033229fdfa94b58a25892fa3959f05e9f Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 9 Apr 2019 11:52:39 -0700 Subject: [PATCH 22/37] lint --- .../DSImage/DSImage.jsx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 2b6af426d0..628abbf136 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -3,6 +3,14 @@ import React from "react"; import ReactDOM from "react-dom"; export class DSImage extends React.PureComponent { + constructor(props) { + super(props); + + this.state = { + isSeen: false, + }; + } + onSeen(element) { if (this.state && document.visibilityState === `visible` && element[0].isIntersecting) { if (this.props.optimize) { @@ -12,7 +20,7 @@ export class DSImage extends React.PureComponent { } this.setState({ - isSeen: true + isSeen: true, }); } } @@ -33,15 +41,10 @@ export class DSImage extends React.PureComponent { } componentDidMount() { - this.setState({ - isSeen: false, - }); - let options = { - // rootMargin: `10px`, root: document.querySelector(`document`), threshold: 1, - } + }; this.observer = new IntersectionObserver(this.onSeen.bind(this), options); From 90c4c245eae3275952d97c13a11ee8f46b31201b Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 9 Apr 2019 12:26:05 -0700 Subject: [PATCH 23/37] lower threshold --- .../components/DiscoveryStreamComponents/DSImage/DSImage.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 628abbf136..8c4f990fb8 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -43,7 +43,7 @@ export class DSImage extends React.PureComponent { componentDidMount() { let options = { root: document.querySelector(`document`), - threshold: 1, + threshold: 0.5, }; this.observer = new IntersectionObserver(this.onSeen.bind(this), options); @@ -91,5 +91,5 @@ 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 + optimize: false, // Measure parent container to request exact sizes }; From bc9c7a12808327548561663b2e6b4a7afe9daa6c Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 9 Apr 2019 13:26:33 -0700 Subject: [PATCH 24/37] turning optimization back on --- .../components/DiscoveryStreamComponents/DSImage/DSImage.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 8c4f990fb8..963e5a7762 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -91,5 +91,5 @@ 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: false, // Measure parent container to request exact sizes + optimize: true, // Measure parent container to request exact sizes }; From 17976feae787fac5e8475c5a5348cf9bc7455be3 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 9 Apr 2019 13:38:29 -0700 Subject: [PATCH 25/37] let images load when opening new tab --- .../components/DiscoveryStreamComponents/DSImage/DSImage.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 963e5a7762..a3ecf7f0f9 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -12,7 +12,7 @@ export class DSImage extends React.PureComponent { } onSeen(element) { - if (this.state && document.visibilityState === `visible` && element[0].isIntersecting) { + if (this.state && element[0].isIntersecting) { if (this.props.optimize) { this.setState({ containerWidth: ReactDOM.findDOMNode(this).clientWidth, From 12c382d1a6d469cfe6a02510154e344b64cb9f55 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 9 Apr 2019 15:08:11 -0700 Subject: [PATCH 26/37] fallback to 450px images --- .../DSImage/DSImage.jsx | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index a3ecf7f0f9..a69c292c45 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -8,6 +8,7 @@ export class DSImage extends React.PureComponent { this.state = { isSeen: false, + optimizedImageFailed: false, }; } @@ -28,8 +29,10 @@ export class DSImage extends React.PureComponent { reformatImageURL(url, width) { const urlIsEncoded = url !== decodeURI(url); + let constructedURL = url; + // Encode the URL if it needs it - let constructedURL = urlIsEncoded ? url : encodeURIComponent(url); + constructedURL = urlIsEncoded ? url : encodeURIComponent(url); // Change the image URL to request a size tailored for the parent container width // Also: force JPEG, quality 60, no upscaling, no EXIF data @@ -37,7 +40,9 @@ export class DSImage extends React.PureComponent { constructedURL = `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${(constructedURL)}`; // Use Mozilla CDN: - return `https://img-getpocket.cdn.mozilla.net/direct?url=${encodeURIComponent(constructedURL)}`; + constructedURL = `https://img-getpocket.cdn.mozilla.net/direct?url=${encodeURIComponent(constructedURL)}`; + + return constructedURL; } componentDidMount() { @@ -57,7 +62,7 @@ export class DSImage extends React.PureComponent { let img; if (this.state && this.state.isSeen) { - if (this.props.optimize && this.props.rawSource) { + if (this.props.optimize && this.props.rawSource && !this.state.optimizedImageFailed) { let source; let source2x; @@ -74,7 +79,7 @@ export class DSImage extends React.PureComponent { cache.query(baseSource, this.state.containerWidth * 2, `2x`) ); - img = (); + img = (); } } else { img = (); @@ -85,6 +90,15 @@ export class DSImage extends React.PureComponent { {img} ); } + + onOptimizedImageError() { + console.error(`Optimized image failed to load. URL: ${this.props.rawSource}`); + + // This will trigger a re-render and the normal 450px image will be used as a fallback + this.setState({ + optimizedImageFailed: true, + }); + } } DSImage.defaultProps = { From 7f25ec1dfb6d42726b86831ecb65a708efb4907e Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Tue, 9 Apr 2019 15:14:59 -0700 Subject: [PATCH 27/37] lint --- .../DiscoveryStreamComponents/DSImage/DSImage.jsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index a69c292c45..d4bf878a83 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -6,6 +6,8 @@ export class DSImage extends React.PureComponent { constructor(props) { super(props); + this.onOptimizedImageError = this.onOptimizedImageError.bind(this); + this.state = { isSeen: false, optimizedImageFailed: false, @@ -79,7 +81,7 @@ export class DSImage extends React.PureComponent { cache.query(baseSource, this.state.containerWidth * 2, `2x`) ); - img = (); + img = (); } } else { img = (); @@ -92,8 +94,6 @@ export class DSImage extends React.PureComponent { } onOptimizedImageError() { - console.error(`Optimized image failed to load. URL: ${this.props.rawSource}`); - // This will trigger a re-render and the normal 450px image will be used as a fallback this.setState({ optimizedImageFailed: true, From ca36bf3ad676edbc5d937a54d6d2bae8614962f0 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Wed, 10 Apr 2019 10:42:07 -0700 Subject: [PATCH 28/37] simplify URL construction --- .../DiscoveryStreamComponents/DSImage/DSImage.jsx | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index d4bf878a83..1418b4ff01 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -29,22 +29,10 @@ export class DSImage extends React.PureComponent { } reformatImageURL(url, width) { - const urlIsEncoded = url !== decodeURI(url); - - let constructedURL = url; - - // Encode the URL if it needs it - constructedURL = urlIsEncoded ? url : encodeURIComponent(url); - // 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 - constructedURL = `https://pocket-image-cache.com/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${(constructedURL)}`; - - // Use Mozilla CDN: - constructedURL = `https://img-getpocket.cdn.mozilla.net/direct?url=${encodeURIComponent(constructedURL)}`; - - return constructedURL; + return `https://img-getpocket.cdn.mozilla.net/${width}x0/filters:format(jpeg):quality(60):no_upscale():strip_exif()/${encodeURIComponent(url)}`; } componentDidMount() { From 9033239f6ba5abb07ece84728c08921368e92503 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Wed, 10 Apr 2019 10:44:18 -0700 Subject: [PATCH 29/37] threshold 0 --- .../components/DiscoveryStreamComponents/DSImage/DSImage.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 1418b4ff01..a7c7fce425 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -38,7 +38,7 @@ export class DSImage extends React.PureComponent { componentDidMount() { let options = { root: document.querySelector(`document`), - threshold: 0.5, + threshold: 0, // Load as soon as the first pixel crosses into view }; this.observer = new IntersectionObserver(this.onSeen.bind(this), options); From 2cb11012f3e8bf2788d8147aa8844945c08f7f97 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Wed, 10 Apr 2019 10:50:03 -0700 Subject: [PATCH 30/37] turn off impression observer after element is seen --- .../components/DiscoveryStreamComponents/DSImage/DSImage.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index a7c7fce425..f124120cde 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -25,6 +25,9 @@ export class DSImage extends React.PureComponent { this.setState({ isSeen: true, }); + + // Stop observing since element has been seen + this.observer.unobserve(ReactDOM.findDOMNode(this)); } } From 48d674dc8212d9e27b293aea93190636bc1c8046 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Wed, 10 Apr 2019 12:17:01 -0700 Subject: [PATCH 31/37] remove listener on unmount --- .../DiscoveryStreamComponents/DSImage/DSImage.jsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index f124120cde..77080541ac 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -49,6 +49,13 @@ export class DSImage extends React.PureComponent { 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}` : ``}`; From a125ca4e1b5175b8ecfeb417529cda9418f1e16e Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Wed, 10 Apr 2019 13:26:38 -0700 Subject: [PATCH 32/37] adding dsimage jsx tests --- .../DSImage.test.jsx | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 test/unit/content-src/components/DiscoveryStreamComponents/DSImage.test.jsx diff --git a/test/unit/content-src/components/DiscoveryStreamComponents/DSImage.test.jsx b/test/unit/content-src/components/DiscoveryStreamComponents/DSImage.test.jsx new file mode 100644 index 0000000000..f3053def60 --- /dev/null +++ b/test/unit/content-src/components/DiscoveryStreamComponents/DSImage.test.jsx @@ -0,0 +1,36 @@ +import {DSImage} from "content-src/components/DiscoveryStreamComponents/DSImage/DSImage"; +import React from "react"; +import {mount} from "enzyme"; + +describe("Discovery Stream ", () => { + it("should have a child with class ds-image", () => { + const img = mount(); + const child = img.find(".ds-image"); + + assert.lengthOf(child, 1); + }); + + it("should set proper sources if only `source` is available", () => { + const img = mount(); + + 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(); + + 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"); + }); + +}); From c58e40ba124d13190eba6658fc904abe1b967d1a Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Wed, 10 Apr 2019 13:27:52 -0700 Subject: [PATCH 33/37] linttt --- .../components/DiscoveryStreamComponents/DSImage.test.jsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/unit/content-src/components/DiscoveryStreamComponents/DSImage.test.jsx b/test/unit/content-src/components/DiscoveryStreamComponents/DSImage.test.jsx index f3053def60..03fafbf40f 100644 --- a/test/unit/content-src/components/DiscoveryStreamComponents/DSImage.test.jsx +++ b/test/unit/content-src/components/DiscoveryStreamComponents/DSImage.test.jsx @@ -1,6 +1,6 @@ import {DSImage} from "content-src/components/DiscoveryStreamComponents/DSImage/DSImage"; -import React from "react"; import {mount} from "enzyme"; +import React from "react"; describe("Discovery Stream ", () => { it("should have a child with class ds-image", () => { @@ -15,7 +15,7 @@ describe("Discovery Stream ", () => { img.setState({ isSeen: true, - containerWidth: 640 + containerWidth: 640, }); assert.equal(img.find("img").prop("src"), "https://placekitten.com/g/640/480"); @@ -26,11 +26,10 @@ describe("Discovery Stream ", () => { img.setState({ isSeen: true, - containerWidth: 640 + 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"); }); - }); From 050abbc84bc2ffe502be0ad0812d325c1459dedf Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Wed, 10 Apr 2019 14:17:00 -0700 Subject: [PATCH 34/37] using bg color --- components/DiscoveryStreamComponents/DSImage/DSImage.css | 3 ++- .../components/DiscoveryStreamComponents/DSImage/DSImage.scss | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/components/DiscoveryStreamComponents/DSImage/DSImage.css b/components/DiscoveryStreamComponents/DSImage/DSImage.css index 8f02a131cb..877f577eb4 100644 --- a/components/DiscoveryStreamComponents/DSImage/DSImage.css +++ b/components/DiscoveryStreamComponents/DSImage/DSImage.css @@ -2,10 +2,11 @@ display: block; position: relative; } .ds-image img { + background-color: var(--newtab-card-placeholder-color); position: absolute; top: 0; width: 100%; height: 100%; object-fit: cover; } -/*# sourceMappingURL=data:application/json;base64,ewoJInZlcnNpb24iOiAzLAoJImZpbGUiOiAiY29tcG9uZW50cy9EaXNjb3ZlcnlTdHJlYW1Db21wb25lbnRzL0RTSW1hZ2UvRFNJbWFnZS5jc3MiLAoJInNvdXJjZXMiOiBbCgkJImNvbnRlbnQtc3JjL2NvbXBvbmVudHMvRGlzY292ZXJ5U3RyZWFtQ29tcG9uZW50cy9EU0ltYWdlL0RTSW1hZ2Uuc2NzcyIKCV0sCgkic291cmNlc0NvbnRlbnQiOiBbCgkJIi5kcy1pbWFnZSB7XG4gIGRpc3BsYXk6IGJsb2NrO1xuICBwb3NpdGlvbjogcmVsYXRpdmU7XG5cbiAgaW1nIHtcbiAgICBwb3NpdGlvbjogYWJzb2x1dGU7XG4gICAgdG9wOiAwO1xuICAgIHdpZHRoOiAxMDAlO1xuICAgIGhlaWdodDogMTAwJTtcbiAgICBvYmplY3QtZml0OiBjb3ZlcjtcbiAgfVxufVxuIgoJXSwKCSJuYW1lcyI6IFtdLAoJIm1hcHBpbmdzIjogIkFBQUEsQUFBQSxTQUFTLENBQUM7RUFDUixPQUFPLEVBQUUsS0FBSztFQUNkLFFBQVEsRUFBRSxRQUFRLEdBU25CO0VBWEQsQUFJRSxTQUpPLENBSVAsR0FBRyxDQUFDO0lBQ0YsUUFBUSxFQUFFLFFBQVE7SUFDbEIsR0FBRyxFQUFFLENBQUM7SUFDTixLQUFLLEVBQUUsSUFBSTtJQUNYLE1BQU0sRUFBRSxJQUFJO0lBQ1osVUFBVSxFQUFFLEtBQUssR0FDbEIiCn0= */ \ No newline at end of file +/*# sourceMappingURL=data:application/json;base64,ewoJInZlcnNpb24iOiAzLAoJImZpbGUiOiAiY29tcG9uZW50cy9EaXNjb3ZlcnlTdHJlYW1Db21wb25lbnRzL0RTSW1hZ2UvRFNJbWFnZS5jc3MiLAoJInNvdXJjZXMiOiBbCgkJImNvbnRlbnQtc3JjL2NvbXBvbmVudHMvRGlzY292ZXJ5U3RyZWFtQ29tcG9uZW50cy9EU0ltYWdlL0RTSW1hZ2Uuc2NzcyIKCV0sCgkic291cmNlc0NvbnRlbnQiOiBbCgkJIi5kcy1pbWFnZSB7XG4gIGRpc3BsYXk6IGJsb2NrO1xuICBwb3NpdGlvbjogcmVsYXRpdmU7XG5cbiAgaW1nIHtcbiAgICBiYWNrZ3JvdW5kLWNvbG9yOiB2YXIoLS1uZXd0YWItY2FyZC1wbGFjZWhvbGRlci1jb2xvcik7XG4gICAgcG9zaXRpb246IGFic29sdXRlO1xuICAgIHRvcDogMDtcbiAgICB3aWR0aDogMTAwJTtcbiAgICBoZWlnaHQ6IDEwMCU7XG4gICAgb2JqZWN0LWZpdDogY292ZXI7XG4gIH1cbn1cbiIKCV0sCgkibmFtZXMiOiBbXSwKCSJtYXBwaW5ncyI6ICJBQUFBLEFBQUEsU0FBUyxDQUFDO0VBQ1IsT0FBTyxFQUFFLEtBQUs7RUFDZCxRQUFRLEVBQUUsUUFBUSxHQVVuQjtFQVpELEFBSUUsU0FKTyxDQUlQLEdBQUcsQ0FBQztJQUNGLGdCQUFnQixFQUFFLG9DQUFvQztJQUN0RCxRQUFRLEVBQUUsUUFBUTtJQUNsQixHQUFHLEVBQUUsQ0FBQztJQUNOLEtBQUssRUFBRSxJQUFJO0lBQ1gsTUFBTSxFQUFFLElBQUk7SUFDWixVQUFVLEVBQUUsS0FBSyxHQUNsQiIKfQ== */ \ No newline at end of file diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss index a195196233..45b6a49811 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.scss @@ -3,6 +3,7 @@ position: relative; img { + background-color: var(--newtab-card-placeholder-color); position: absolute; top: 0; width: 100%; From d523a9ee14f2df465a3be18d626ecffc1e4b2cb4 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Wed, 10 Apr 2019 14:18:52 -0700 Subject: [PATCH 35/37] removing compiled css --- .../DiscoveryStreamComponents/DSImage/DSImage.css | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 components/DiscoveryStreamComponents/DSImage/DSImage.css diff --git a/components/DiscoveryStreamComponents/DSImage/DSImage.css b/components/DiscoveryStreamComponents/DSImage/DSImage.css deleted file mode 100644 index 877f577eb4..0000000000 --- a/components/DiscoveryStreamComponents/DSImage/DSImage.css +++ /dev/null @@ -1,12 +0,0 @@ -.ds-image { - display: block; - position: relative; } - .ds-image img { - background-color: var(--newtab-card-placeholder-color); - position: absolute; - top: 0; - width: 100%; - height: 100%; - object-fit: cover; } - -/*# sourceMappingURL=data:application/json;base64,ewoJInZlcnNpb24iOiAzLAoJImZpbGUiOiAiY29tcG9uZW50cy9EaXNjb3ZlcnlTdHJlYW1Db21wb25lbnRzL0RTSW1hZ2UvRFNJbWFnZS5jc3MiLAoJInNvdXJjZXMiOiBbCgkJImNvbnRlbnQtc3JjL2NvbXBvbmVudHMvRGlzY292ZXJ5U3RyZWFtQ29tcG9uZW50cy9EU0ltYWdlL0RTSW1hZ2Uuc2NzcyIKCV0sCgkic291cmNlc0NvbnRlbnQiOiBbCgkJIi5kcy1pbWFnZSB7XG4gIGRpc3BsYXk6IGJsb2NrO1xuICBwb3NpdGlvbjogcmVsYXRpdmU7XG5cbiAgaW1nIHtcbiAgICBiYWNrZ3JvdW5kLWNvbG9yOiB2YXIoLS1uZXd0YWItY2FyZC1wbGFjZWhvbGRlci1jb2xvcik7XG4gICAgcG9zaXRpb246IGFic29sdXRlO1xuICAgIHRvcDogMDtcbiAgICB3aWR0aDogMTAwJTtcbiAgICBoZWlnaHQ6IDEwMCU7XG4gICAgb2JqZWN0LWZpdDogY292ZXI7XG4gIH1cbn1cbiIKCV0sCgkibmFtZXMiOiBbXSwKCSJtYXBwaW5ncyI6ICJBQUFBLEFBQUEsU0FBUyxDQUFDO0VBQ1IsT0FBTyxFQUFFLEtBQUs7RUFDZCxRQUFRLEVBQUUsUUFBUSxHQVVuQjtFQVpELEFBSUUsU0FKTyxDQUlQLEdBQUcsQ0FBQztJQUNGLGdCQUFnQixFQUFFLG9DQUFvQztJQUN0RCxRQUFRLEVBQUUsUUFBUTtJQUNsQixHQUFHLEVBQUUsQ0FBQztJQUNOLEtBQUssRUFBRSxJQUFJO0lBQ1gsTUFBTSxFQUFFLElBQUk7SUFDWixVQUFVLEVBQUUsS0FBSyxHQUNsQiIKfQ== */ \ No newline at end of file From 943b620fcc86486f9d914f16f82d1d7cff6ae6b6 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Thu, 11 Apr 2019 10:33:19 -0700 Subject: [PATCH 36/37] simplifying options since using defaults --- .../DiscoveryStreamComponents/DSImage/DSImage.jsx | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 77080541ac..6303efb7fd 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -39,13 +39,7 @@ export class DSImage extends React.PureComponent { } componentDidMount() { - let options = { - root: document.querySelector(`document`), - threshold: 0, // Load as soon as the first pixel crosses into view - }; - - this.observer = new IntersectionObserver(this.onSeen.bind(this), options); - + this.observer = new IntersectionObserver(this.onSeen.bind(this)); this.observer.observe(ReactDOM.findDOMNode(this)); } @@ -92,7 +86,7 @@ export class DSImage extends React.PureComponent { } onOptimizedImageError() { - // This will trigger a re-render and the normal 450px image will be used as a fallback + // This will trigger a re-render and the unoptimized 450px image will be used as a fallback this.setState({ optimizedImageFailed: true, }); From 6b7ccb11285064c3936cc968df5c99db32aa32f1 Mon Sep 17 00:00:00 2001 From: Gavin Lazar Suntop Date: Thu, 11 Apr 2019 10:38:49 -0700 Subject: [PATCH 37/37] fix multiple intersections --- .../DSImage/DSImage.jsx | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx index 6303efb7fd..1281873e81 100644 --- a/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx +++ b/content-src/components/DiscoveryStreamComponents/DSImage/DSImage.jsx @@ -14,20 +14,22 @@ export class DSImage extends React.PureComponent { }; } - onSeen(element) { - if (this.state && element[0].isIntersecting) { - if (this.props.optimize) { + onSeen(entries) { + if (this.state) { + if (entries.some(entry => entry.isIntersecting)) { + if (this.props.optimize) { + this.setState({ + containerWidth: ReactDOM.findDOMNode(this).clientWidth, + }); + } + this.setState({ - containerWidth: ReactDOM.findDOMNode(this).clientWidth, + isSeen: true, }); - } - this.setState({ - isSeen: true, - }); - - // Stop observing since element has been seen - this.observer.unobserve(ReactDOM.findDOMNode(this)); + // Stop observing since element has been seen + this.observer.unobserve(ReactDOM.findDOMNode(this)); + } } }