From 82cd138c89730279205a0dccab4ee5c39f92b342 Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Sat, 29 Jun 2019 10:43:45 +0200 Subject: Fix some React warnings --- .../flavours/glitch/components/intersection_observer_article.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/javascript/flavours/glitch/components/intersection_observer_article.js') diff --git a/app/javascript/flavours/glitch/components/intersection_observer_article.js b/app/javascript/flavours/glitch/components/intersection_observer_article.js index 900c98638..295347c29 100644 --- a/app/javascript/flavours/glitch/components/intersection_observer_article.js +++ b/app/javascript/flavours/glitch/components/intersection_observer_article.js @@ -119,7 +119,7 @@ export default class IntersectionObserverArticle extends ImmutablePureComponent data-id={id} tabIndex='0' style={style}> - {children && React.cloneElement(children, { hidden: !isIntersecting && (isHidden || cachedHeight) })} + {children && React.cloneElement(children, { hidden: !isIntersecting && (isHidden || !!cachedHeight) })} ); } -- cgit From c0b5ee315dcf6ae5d716eaf5756348ef096397ef Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Sun, 30 Jun 2019 10:24:56 +0200 Subject: Revert to using upstream's optimisations This *does* break things, as `shouldComponentUpdate` assume the children to never change! --- .../components/intersection_observer_article.js | 26 +++++++++++++--------- 1 file changed, 15 insertions(+), 11 deletions(-) (limited to 'app/javascript/flavours/glitch/components/intersection_observer_article.js') diff --git a/app/javascript/flavours/glitch/components/intersection_observer_article.js b/app/javascript/flavours/glitch/components/intersection_observer_article.js index 295347c29..2a99a2620 100644 --- a/app/javascript/flavours/glitch/components/intersection_observer_article.js +++ b/app/javascript/flavours/glitch/components/intersection_observer_article.js @@ -3,8 +3,14 @@ import PropTypes from 'prop-types'; import ImmutablePureComponent from 'react-immutable-pure-component'; import scheduleIdleTask from 'flavours/glitch/util/schedule_idle_task'; import getRectFromEntry from 'flavours/glitch/util/get_rect_from_entry'; +import { is } from 'immutable'; -export default class IntersectionObserverArticle extends ImmutablePureComponent { +// Diff these props in the "rendered" state +const updateOnPropsForRendered = ['id', 'index', 'listLength']; +// Diff these props in the "unrendered" state +const updateOnPropsForUnrendered = ['id', 'index', 'listLength', 'cachedHeight']; + +export default class IntersectionObserverArticle extends React.Component { static propTypes = { intersectionObserverWrapper: PropTypes.object.isRequired, @@ -22,20 +28,18 @@ export default class IntersectionObserverArticle extends ImmutablePureComponent } shouldComponentUpdate (nextProps, nextState) { - if (!nextState.isIntersecting && nextState.isHidden) { - // It's only if we're not intersecting (i.e. offscreen) and isHidden is true - // that either "isIntersecting" or "isHidden" matter, and then they're - // the only things that matter (and updated ARIA attributes). - return this.state.isIntersecting || !this.state.isHidden || nextProps.listLength !== this.props.listLength; - } else if (nextState.isIntersecting && !this.state.isIntersecting) { - // If we're going from a non-intersecting state to an intersecting state, - // (i.e. offscreen to onscreen), then we definitely need to re-render + const isUnrendered = !this.state.isIntersecting && (this.state.isHidden || this.props.cachedHeight); + const willBeUnrendered = !nextState.isIntersecting && (nextState.isHidden || nextProps.cachedHeight); + if (!!isUnrendered !== !!willBeUnrendered) { + // If we're going from rendered to unrendered (or vice versa) then update return true; } - // Otherwise, diff based on "updateOnProps" and "updateOnStates" - return super.shouldComponentUpdate(nextProps, nextState); + // Otherwise, diff based on props + const propsToDiff = isUnrendered ? updateOnPropsForUnrendered : updateOnPropsForRendered; + return !propsToDiff.every(prop => is(nextProps[prop], this.props[prop])); } + componentDidMount () { const { intersectionObserverWrapper, id } = this.props; -- cgit From 82a76f03a4e503d76c29570cd97f6e135ef3d7a7 Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Sun, 30 Jun 2019 18:05:37 +0200 Subject: Assume children of visible IntersectionObserverArticle always change This fixes multiple issues, while adding few computations --- .../glitch/components/intersection_observer_article.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'app/javascript/flavours/glitch/components/intersection_observer_article.js') diff --git a/app/javascript/flavours/glitch/components/intersection_observer_article.js b/app/javascript/flavours/glitch/components/intersection_observer_article.js index 2a99a2620..c8427f0bf 100644 --- a/app/javascript/flavours/glitch/components/intersection_observer_article.js +++ b/app/javascript/flavours/glitch/components/intersection_observer_article.js @@ -5,8 +5,6 @@ import scheduleIdleTask from 'flavours/glitch/util/schedule_idle_task'; import getRectFromEntry from 'flavours/glitch/util/get_rect_from_entry'; import { is } from 'immutable'; -// Diff these props in the "rendered" state -const updateOnPropsForRendered = ['id', 'index', 'listLength']; // Diff these props in the "unrendered" state const updateOnPropsForUnrendered = ['id', 'index', 'listLength', 'cachedHeight']; @@ -34,9 +32,12 @@ export default class IntersectionObserverArticle extends React.Component { // If we're going from rendered to unrendered (or vice versa) then update return true; } - // Otherwise, diff based on props - const propsToDiff = isUnrendered ? updateOnPropsForUnrendered : updateOnPropsForRendered; - return !propsToDiff.every(prop => is(nextProps[prop], this.props[prop])); + // If we are and remain hidden, diff based on props + if (isUnrendered) { + return !updateOnPropsForUnrendered.every(prop => is(nextProps[prop], this.props[prop])); + } + // Else, assume the children have changed + return true; } -- cgit From c49f7d5d164d59340a3106967b3e9e5deaf99345 Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Mon, 1 Jul 2019 12:45:18 +0200 Subject: Use strict equality rather than Immutable.is as the compared props are values --- .../flavours/glitch/components/intersection_observer_article.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'app/javascript/flavours/glitch/components/intersection_observer_article.js') diff --git a/app/javascript/flavours/glitch/components/intersection_observer_article.js b/app/javascript/flavours/glitch/components/intersection_observer_article.js index c8427f0bf..03b3700df 100644 --- a/app/javascript/flavours/glitch/components/intersection_observer_article.js +++ b/app/javascript/flavours/glitch/components/intersection_observer_article.js @@ -1,9 +1,7 @@ import React from 'react'; import PropTypes from 'prop-types'; -import ImmutablePureComponent from 'react-immutable-pure-component'; import scheduleIdleTask from 'flavours/glitch/util/schedule_idle_task'; import getRectFromEntry from 'flavours/glitch/util/get_rect_from_entry'; -import { is } from 'immutable'; // Diff these props in the "unrendered" state const updateOnPropsForUnrendered = ['id', 'index', 'listLength', 'cachedHeight']; @@ -34,7 +32,7 @@ export default class IntersectionObserverArticle extends React.Component { } // If we are and remain hidden, diff based on props if (isUnrendered) { - return !updateOnPropsForUnrendered.every(prop => is(nextProps[prop], this.props[prop])); + return !updateOnPropsForUnrendered.every(prop => nextProps[prop] === this.props[prop]); } // Else, assume the children have changed return true; -- cgit