From da0a5777688e7c950417c6e48d382b7d70f6fdd3 Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Sat, 10 Feb 2018 20:56:48 +0100 Subject: [PATCH] Revert 4a48d03b310a5eb71ff3cf8f89d49b543e0431be (fixes #348) Since 4a48d03b310a5eb71ff3cf8f89d49b543e0431be, IntersectionObserverArticle assumes that its children do not change unless the number of children changes. This is not the case with the notification overlay, which resulted in the checkmark of notification cleaning mode not updating unless scrolling to make notifications appear/disappear. This change may negatively impact performances. --- .../intersection_observer_article.js | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/app/javascript/flavours/glitch/components/intersection_observer_article.js b/app/javascript/flavours/glitch/components/intersection_observer_article.js index 8b06f9a8c8..f7f6b0a532 100644 --- a/app/javascript/flavours/glitch/components/intersection_observer_article.js +++ b/app/javascript/flavours/glitch/components/intersection_observer_article.js @@ -1,15 +1,10 @@ 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 "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 { +export default class IntersectionObserverArticle extends ImmutablePureComponent { static propTypes = { intersectionObserverWrapper: PropTypes.object.isRequired, @@ -27,15 +22,18 @@ export default class IntersectionObserverArticle extends React.Component { } shouldComponentUpdate (nextProps, nextState) { - 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 + 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 return true; } - // Otherwise, diff based on props - const propsToDiff = isUnrendered ? updateOnPropsForUnrendered : updateOnPropsForRendered; - return !propsToDiff.every(prop => is(nextProps[prop], this.props[prop])); + // Otherwise, diff based on "updateOnProps" and "updateOnStates" + return super.shouldComponentUpdate(nextProps, nextState); } componentDidMount () {