From 72c397f2c17f4789d90c01e3bdfd47e4452be94a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Juh=C3=A9=20Lluveras?= Date: Tue, 11 Sep 2018 12:29:03 +0200 Subject: [PATCH 1/6] Use IntersectionObserver when it's available --- .eslintrc | 2 +- src/components/LazyLoadComponent.jsx | 3 +- src/components/PlaceholderWithoutTracking.jsx | 44 ++++++++++++++++++- src/hoc/trackWindowScroll.js | 16 +++++-- src/utils/intersection-observer.js | 6 +++ 5 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 src/utils/intersection-observer.js diff --git a/.eslintrc b/.eslintrc index 3c4fa33..ac49a2c 100644 --- a/.eslintrc +++ b/.eslintrc @@ -133,7 +133,7 @@ "padded-blocks": [1, "never"], "quote-props": [1, "as-needed"], "quotes": [1, 'single'], - "require-jsdoc": 1, + "require-jsdoc": 0, "semi-spacing": 1, "semi": 1, "sort-keys": 0, diff --git a/src/components/LazyLoadComponent.jsx b/src/components/LazyLoadComponent.jsx index f868e51..5bc064a 100644 --- a/src/components/LazyLoadComponent.jsx +++ b/src/components/LazyLoadComponent.jsx @@ -3,6 +3,7 @@ import { PropTypes } from 'prop-types'; import PlaceholderWithoutTracking from './PlaceholderWithoutTracking.jsx'; import PlaceholderWithTracking from './PlaceholderWithTracking.jsx'; +import isIntersectionObserverAvailable from '../utils/intersection-observer'; class LazyLoadComponent extends React.Component { constructor(props) { @@ -47,7 +48,7 @@ class LazyLoadComponent extends React.Component { const { className, height, placeholder, scrollPosition, style, threshold, width } = this.props; - if (this.isScrollTracked) { + if (this.isScrollTracked || isIntersectionObserverAvailable()) { return ( { + if (entry.isIntersecting) { + entry.target.onVisible(); + } + }); } componentDidMount() { - this.updateVisibility(); + if (this.placeholder && + window.LAZY_LOAD_OBSERVER && window.LAZY_LOAD_OBSERVER.observer) { + this.placeholder.onVisible = this.props.onVisible; + window.LAZY_LOAD_OBSERVER.observer.observe(this.placeholder); + } + + if (window.LAZY_LOAD_OBSERVER && + !window.LAZY_LOAD_OBSERVER.supportsObserver) { + this.updateVisibility(); + } + } + + componentWillUnMount() { + if (window.LAZY_LOAD_OBSERVER) { + window.LAZY_LOAD_OBSERVER.observer.unobserve(this.placeholder); + } } componentDidUpdate() { - this.updateVisibility(); + if (window.LAZY_LOAD_OBSERVER && + !window.LAZY_LOAD_OBSERVER.supportsObserver) { + this.updateVisibility(); + } } getPlaceholderBoundingBox(scrollPosition = this.props.scrollPosition) { diff --git a/src/hoc/trackWindowScroll.js b/src/hoc/trackWindowScroll.js index b4c20dc..832749b 100644 --- a/src/hoc/trackWindowScroll.js +++ b/src/hoc/trackWindowScroll.js @@ -2,12 +2,17 @@ import React from 'react'; import { PropTypes } from 'prop-types'; import debounce from 'lodash.debounce'; import throttle from 'lodash.throttle'; +import isIntersectionObserverAvailable from '../utils/intersection-observer'; const trackWindowScroll = (BaseComponent) => { class ScrollAwareComponent extends React.Component { constructor(props) { super(props); + if (isIntersectionObserverAvailable()) { + return; + } + const onChangeScroll = this.onChangeScroll.bind(this); if (props.delayMethod === 'debounce') { @@ -31,7 +36,7 @@ const trackWindowScroll = (BaseComponent) => { } componentDidMount() { - if (typeof window == 'undefined') { + if (typeof window == 'undefined' || isIntersectionObserverAvailable()) { return; } window.addEventListener('scroll', this.delayedScroll); @@ -39,7 +44,7 @@ const trackWindowScroll = (BaseComponent) => { } componentWillUnmount() { - if (typeof window === 'undefined') { + if (typeof window == 'undefined' || isIntersectionObserverAvailable()) { return; } window.removeEventListener('scroll', this.delayedScroll); @@ -47,6 +52,9 @@ const trackWindowScroll = (BaseComponent) => { } onChangeScroll() { + if (isIntersectionObserverAvailable()) { + return; + } this.setState({ scrollPosition: { x: (typeof window == 'undefined' ? @@ -63,10 +71,12 @@ const trackWindowScroll = (BaseComponent) => { render() { const { delayMethod, delayTime, ...props } = this.props; + const scrollPosition = isIntersectionObserverAvailable() ? + null : this.state.scrollPosition; return ( ); } diff --git a/src/utils/intersection-observer.js b/src/utils/intersection-observer.js new file mode 100644 index 0000000..97840e3 --- /dev/null +++ b/src/utils/intersection-observer.js @@ -0,0 +1,6 @@ +export default function() { + return ( + 'IntersectionObserver' in window && + 'isIntersecting' in window.IntersectionObserverEntry.prototype + ); +} From 364ce2142ef074e4b84c6a724ad029636246f79e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Juh=C3=A9=20Lluveras?= Date: Wed, 26 Dec 2018 12:44:36 +0100 Subject: [PATCH 2/6] Avoid using 'window' to store the IntersectionObserver --- src/components/PlaceholderWithoutTracking.jsx | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/components/PlaceholderWithoutTracking.jsx b/src/components/PlaceholderWithoutTracking.jsx index 9298c12..e161e63 100644 --- a/src/components/PlaceholderWithoutTracking.jsx +++ b/src/components/PlaceholderWithoutTracking.jsx @@ -7,15 +7,15 @@ class PlaceholderWithoutTracking extends React.Component { constructor(props) { super(props); - if (!window.LAZY_LOAD_OBSERVER) { + if (!this.LAZY_LOAD_OBSERVER) { const supportsObserver = isIntersectionObserverAvailable(); - window.LAZY_LOAD_OBSERVER = { supportsObserver }; + this.LAZY_LOAD_OBSERVER = { supportsObserver }; if (supportsObserver) { const { threshold } = props; - window.LAZY_LOAD_OBSERVER.observer = new IntersectionObserver( + this.LAZY_LOAD_OBSERVER.observer = new IntersectionObserver( this.checkIntersections, { rootMargin: threshold + 'px' }); } } @@ -31,26 +31,26 @@ class PlaceholderWithoutTracking extends React.Component { componentDidMount() { if (this.placeholder && - window.LAZY_LOAD_OBSERVER && window.LAZY_LOAD_OBSERVER.observer) { + this.LAZY_LOAD_OBSERVER && this.LAZY_LOAD_OBSERVER.observer) { this.placeholder.onVisible = this.props.onVisible; - window.LAZY_LOAD_OBSERVER.observer.observe(this.placeholder); + this.LAZY_LOAD_OBSERVER.observer.observe(this.placeholder); } - if (window.LAZY_LOAD_OBSERVER && - !window.LAZY_LOAD_OBSERVER.supportsObserver) { + if (this.LAZY_LOAD_OBSERVER && + !this.LAZY_LOAD_OBSERVER.supportsObserver) { this.updateVisibility(); } } componentWillUnMount() { - if (window.LAZY_LOAD_OBSERVER) { - window.LAZY_LOAD_OBSERVER.observer.unobserve(this.placeholder); + if (this.LAZY_LOAD_OBSERVER) { + this.LAZY_LOAD_OBSERVER.observer.unobserve(this.placeholder); } } componentDidUpdate() { - if (window.LAZY_LOAD_OBSERVER && - !window.LAZY_LOAD_OBSERVER.supportsObserver) { + if (this.LAZY_LOAD_OBSERVER && + !this.LAZY_LOAD_OBSERVER.supportsObserver) { this.updateVisibility(); } } From 46556059c1950523c8888fed0c02f3f2242a644e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Juh=C3=A9=20Lluveras?= Date: Sat, 29 Dec 2018 17:08:34 +0100 Subject: [PATCH 3/6] Add unit tests --- src/components/LazyLoadComponent.spec.js | 38 +++++++++++++++++++ src/components/PlaceholderWithoutTracking.jsx | 5 ++- .../PlaceholderWithoutTracking.spec.js | 26 +++++++++++++ src/utils/intersection-observer.spec.js | 21 ++++++++++ 4 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 src/utils/intersection-observer.spec.js diff --git a/src/components/LazyLoadComponent.spec.js b/src/components/LazyLoadComponent.spec.js index d23bc08..e236b63 100644 --- a/src/components/LazyLoadComponent.spec.js +++ b/src/components/LazyLoadComponent.spec.js @@ -7,6 +7,9 @@ import Adapter from 'enzyme-adapter-react-16'; import LazyLoadComponent from './LazyLoadComponent.jsx'; import PlaceholderWithTracking from './PlaceholderWithTracking.jsx'; import PlaceholderWithoutTracking from './PlaceholderWithoutTracking.jsx'; +import isIntersectionObserverAvailable from '../utils/intersection-observer'; + +jest.mock('../utils/intersection-observer'); configure({ adapter: new Adapter() }); @@ -16,6 +19,16 @@ const { } = ReactTestUtils; describe('LazyLoadComponent', function() { + const windowIntersectionObserver = window.IntersectionObserver; + + beforeEach(() => { + isIntersectionObserverAvailable.mockImplementation(() => false); + }); + + afterEach(() => { + window.IntersectionObserver = windowIntersectionObserver; + }); + it('renders a PlaceholderWithTracking when scrollPosition is undefined', function() { const lazyLoadComponent = mount( true); + window.IntersectionObserver = jest.fn(function() { + this.observe = jest.fn(); // eslint-disable-line babel/no-invalid-this + }); + + const lazyLoadComponent = mount( + +

Lorem Ipsum

+ + ); + + const placeholderWithTracking = scryRenderedComponentsWithType( + lazyLoadComponent.instance(), PlaceholderWithTracking); + const placeholderWithoutTracking = scryRenderedComponentsWithType( + lazyLoadComponent.instance(), PlaceholderWithoutTracking); + + expect(placeholderWithTracking.length).toEqual(0); + expect(placeholderWithoutTracking.length).toEqual(1); + }); + it('renders a PlaceholderWithoutTracking when scrollPosition is defined', function() { const lazyLoadComponent = mount( ); + const placeholderWithTracking = scryRenderedComponentsWithType( + lazyLoadComponent.instance(), PlaceholderWithTracking); const placeholderWithoutTracking = scryRenderedComponentsWithType( lazyLoadComponent.instance(), PlaceholderWithoutTracking); + expect(placeholderWithTracking.length).toEqual(0); expect(placeholderWithoutTracking.length).toEqual(1); }); diff --git a/src/components/PlaceholderWithoutTracking.jsx b/src/components/PlaceholderWithoutTracking.jsx index e161e63..224c995 100644 --- a/src/components/PlaceholderWithoutTracking.jsx +++ b/src/components/PlaceholderWithoutTracking.jsx @@ -16,7 +16,8 @@ class PlaceholderWithoutTracking extends React.Component { const { threshold } = props; this.LAZY_LOAD_OBSERVER.observer = new IntersectionObserver( - this.checkIntersections, { rootMargin: threshold + 'px' }); + this.checkIntersections, { rootMargin: threshold + 'px' } + ); } } } @@ -120,7 +121,7 @@ PlaceholderWithoutTracking.propTypes = { scrollPosition: PropTypes.shape({ x: PropTypes.number.isRequired, y: PropTypes.number.isRequired, - }).isRequired, + }), className: PropTypes.string, height: PropTypes.number, placeholder: PropTypes.element, diff --git a/src/components/PlaceholderWithoutTracking.spec.js b/src/components/PlaceholderWithoutTracking.spec.js index 39e6948..ad6dc9b 100644 --- a/src/components/PlaceholderWithoutTracking.spec.js +++ b/src/components/PlaceholderWithoutTracking.spec.js @@ -8,6 +8,9 @@ import { configure, mount } from 'enzyme'; import Adapter from 'enzyme-adapter-react-16'; import PlaceholderWithoutTracking from './PlaceholderWithoutTracking.jsx'; +import isIntersectionObserverAvailable from '../utils/intersection-observer'; + +jest.mock('../utils/intersection-observer'); configure({ adapter: new Adapter() }); @@ -73,6 +76,16 @@ describe('PlaceholderWithoutTracking', function() { expect(placeholderWrapper.length).toEqual(numberOfPlaceholderWrappers); } + const windowIntersectionObserver = window.IntersectionObserver; + + beforeEach(() => { + isIntersectionObserverAvailable.mockImplementation(() => false); + }); + + afterEach(() => { + window.IntersectionObserver = windowIntersectionObserver; + }); + it('renders the default placeholder when it\'s not in the viewport', function() { const className = 'placeholder-wrapper'; const component = renderPlaceholderWithoutTracking({ @@ -168,4 +181,17 @@ describe('PlaceholderWithoutTracking', function() { expect(onVisible).toHaveBeenCalledTimes(1); }); + + it('doesn\'t track placeholder visibility if IntersectionObserver is available', function() { + isIntersectionObserverAvailable.mockImplementation(() => true); + window.IntersectionObserver = jest.fn(function() { + this.observe = jest.fn(); // eslint-disable-line babel/no-invalid-this + }); + const onVisible = jest.fn(); + const component = renderPlaceholderWithoutTracking({ + onVisible, + }); + + expect(onVisible).toHaveBeenCalledTimes(0); + }); }); diff --git a/src/utils/intersection-observer.spec.js b/src/utils/intersection-observer.spec.js new file mode 100644 index 0000000..4094908 --- /dev/null +++ b/src/utils/intersection-observer.spec.js @@ -0,0 +1,21 @@ +import isIntersectionObserverAvailable from './intersection-observer'; + +describe('isIntersectionObserverAvailable', function() { + it('returns true if IntersectionObserver is available', function() { + window.IntersectionObserver = {}; + window.IntersectionObserverEntry = { + prototype: { + isIntersecting: () => null, + }, + }; + + expect(isIntersectionObserverAvailable()).toBe(true); + }); + + it('returns false if IntersectionObserver is not available', function() { + delete window.IntersectionObserver; + delete window.IntersectionObserverEntry; + + expect(isIntersectionObserverAvailable()).toBe(false); + }); +}); From c73bdaa3bc4262eddd75bac252147080d66f6ec4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Juh=C3=A9=20Lluveras?= Date: Sun, 30 Dec 2018 17:51:32 +0100 Subject: [PATCH 4/6] Remove unnecessary conditional --- src/components/PlaceholderWithoutTracking.jsx | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/components/PlaceholderWithoutTracking.jsx b/src/components/PlaceholderWithoutTracking.jsx index 224c995..e30274f 100644 --- a/src/components/PlaceholderWithoutTracking.jsx +++ b/src/components/PlaceholderWithoutTracking.jsx @@ -7,18 +7,16 @@ class PlaceholderWithoutTracking extends React.Component { constructor(props) { super(props); - if (!this.LAZY_LOAD_OBSERVER) { - const supportsObserver = isIntersectionObserverAvailable(); + const supportsObserver = isIntersectionObserverAvailable(); - this.LAZY_LOAD_OBSERVER = { supportsObserver }; + this.LAZY_LOAD_OBSERVER = { supportsObserver }; - if (supportsObserver) { - const { threshold } = props; + if (supportsObserver) { + const { threshold } = props; - this.LAZY_LOAD_OBSERVER.observer = new IntersectionObserver( - this.checkIntersections, { rootMargin: threshold + 'px' } - ); - } + this.LAZY_LOAD_OBSERVER.observer = new IntersectionObserver( + this.checkIntersections, { rootMargin: threshold + 'px' } + ); } } From 6e21866768608b4d1d866877dd8c6c861cb3b70e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Juh=C3=A9=20Lluveras?= Date: Sun, 30 Dec 2018 18:11:40 +0100 Subject: [PATCH 5/6] Fix tests in Travis --- src/utils/intersection-observer.spec.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/utils/intersection-observer.spec.js b/src/utils/intersection-observer.spec.js index 4094908..fc5fd16 100644 --- a/src/utils/intersection-observer.spec.js +++ b/src/utils/intersection-observer.spec.js @@ -14,6 +14,9 @@ describe('isIntersectionObserverAvailable', function() { it('returns false if IntersectionObserver is not available', function() { delete window.IntersectionObserver; + window.IntersectionObserverEntry = { + prototype: {}, + }; delete window.IntersectionObserverEntry; expect(isIntersectionObserverAvailable()).toBe(false); From 8d1805c8321b0fa644016eedd7d83775b4c27ed7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Juh=C3=A9=20Lluveras?= Date: Sun, 30 Dec 2018 18:15:19 +0100 Subject: [PATCH 6/6] Fix wrong props order --- src/components/PlaceholderWithoutTracking.jsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/components/PlaceholderWithoutTracking.jsx b/src/components/PlaceholderWithoutTracking.jsx index e30274f..924b26a 100644 --- a/src/components/PlaceholderWithoutTracking.jsx +++ b/src/components/PlaceholderWithoutTracking.jsx @@ -116,14 +116,14 @@ class PlaceholderWithoutTracking extends React.Component { PlaceholderWithoutTracking.propTypes = { onVisible: PropTypes.func.isRequired, - scrollPosition: PropTypes.shape({ - x: PropTypes.number.isRequired, - y: PropTypes.number.isRequired, - }), className: PropTypes.string, height: PropTypes.number, placeholder: PropTypes.element, threshold: PropTypes.number, + scrollPosition: PropTypes.shape({ + x: PropTypes.number.isRequired, + y: PropTypes.number.isRequired, + }), width: PropTypes.number, };