about summary refs log tree commit diff
path: root/app/javascript/mastodon/components
diff options
context:
space:
mode:
authorMatt Panaro <matt.panaro@gmail.com>2019-12-28 23:39:48 -0500
committerEugen Rochko <eugen@zeonfederated.com>2019-12-29 05:39:48 +0100
commit31f7c3fc5d9bf24fc79f3246a0de2cc907ecb34d (patch)
treee71a0104c416a7e336885dfab1bfb6492a474ba6 /app/javascript/mastodon/components
parent1155dc08352c3aa1cb763729122c34a3db6e64ec (diff)
Summary: fix slowness due to layout thrashing when reloading a large … (#12661)
* Summary: fix slowness due to layout thrashing when reloading a large set of status updates

in order to limit the maximum size of a status in a list view (e.g. the home timeline), so as to avoid having to scroll all the way through an abnormally large status update (see https://github.com/tootsuite/mastodon/pull/8205), the following steps are taken:
•the element containing the status is rendered in the browser
•its height is calculated, to determine if it exceeds the maximum height threshold.
Unfortunately for performance, these steps are carried out in the componentDidMount(/Update) method, which also performs style modifications on the element.  The combination of  height request and style modification during javascript evaluation in the browser leads to layout-thrashing, where the elements are repeatedly re-laid-out (see https://developers.google.com/web/fundamentals/performance/rendering/avoid-large-complex-layouts-and-layout-thrashing & https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Performance_best_practices_for_Firefox_fe_engineers).
The solution implemented here is to memoize the collapsed state in Redux the first time the status is seen (e.g. when fetched as part of a small batch, to populate the home timeline) , so that on subsequent re-renders, the value can be queried, rather than recalculated.  This strategy is derived from https://github.com/tootsuite/mastodon/pull/4439 & https://github.com/tootsuite/mastodon/pull/4909, and should resolve https://github.com/tootsuite/mastodon/issues/12455.

Andrew Lin (https://github.com/onethreeseven) is thanked for his assistance in root cause analysis and solution brainstorming

* remove getSnapshotBeforeUpdate from status

* remove componentWillUnmount from status

* persist last-intersected status update and restore when ScrollableList is restored

e.g. when navigating from home-timeline to a status conversational  thread and <Back again

* cache currently-viewing status id to avoid calling redux with identical value

* refactor collapse toggle to pass explicit boolean
Diffstat (limited to 'app/javascript/mastodon/components')
-rw-r--r--app/javascript/mastodon/components/intersection_observer_article.js6
-rw-r--r--app/javascript/mastodon/components/scrollable_list.js4
-rw-r--r--app/javascript/mastodon/components/status.js28
-rw-r--r--app/javascript/mastodon/components/status_content.js25
-rw-r--r--app/javascript/mastodon/components/status_list.js9
5 files changed, 40 insertions, 32 deletions
diff --git a/app/javascript/mastodon/components/intersection_observer_article.js b/app/javascript/mastodon/components/intersection_observer_article.js
index e453730ba..d475e5d1c 100644
--- a/app/javascript/mastodon/components/intersection_observer_article.js
+++ b/app/javascript/mastodon/components/intersection_observer_article.js
@@ -20,6 +20,8 @@ export default class IntersectionObserverArticle extends React.Component {
     cachedHeight: PropTypes.number,
     onHeightChange: PropTypes.func,
     children: PropTypes.node,
+    currentlyViewing: PropTypes.number,
+    updateCurrentlyViewing: PropTypes.func,
   };
 
   state = {
@@ -48,6 +50,8 @@ export default class IntersectionObserverArticle extends React.Component {
     );
 
     this.componentMounted = true;
+
+    if(id === this.props.currentlyViewing) this.node.scrollIntoView();
   }
 
   componentWillUnmount () {
@@ -60,6 +64,8 @@ export default class IntersectionObserverArticle extends React.Component {
   handleIntersection = (entry) => {
     this.entry = entry;
 
+    if(entry.intersectionRatio > 0.75 && this.props.updateCurrentlyViewing) this.props.updateCurrentlyViewing(this.id);
+
     scheduleIdleTask(this.calculateHeight);
     this.setState(this.updateStateAfterIntersection);
   }
diff --git a/app/javascript/mastodon/components/scrollable_list.js b/app/javascript/mastodon/components/scrollable_list.js
index 421756803..6338ccd5c 100644
--- a/app/javascript/mastodon/components/scrollable_list.js
+++ b/app/javascript/mastodon/components/scrollable_list.js
@@ -36,6 +36,8 @@ export default class ScrollableList extends PureComponent {
     emptyMessage: PropTypes.node,
     children: PropTypes.node,
     bindToDocument: PropTypes.bool,
+    currentlyViewing: PropTypes.number,
+    updateCurrentlyViewing: PropTypes.func,
   };
 
   static defaultProps = {
@@ -309,6 +311,8 @@ export default class ScrollableList extends PureComponent {
                 listLength={childrenCount}
                 intersectionObserverWrapper={this.intersectionObserverWrapper}
                 saveHeightKey={trackScroll ? `${this.context.router.route.location.key}:${scrollKey}` : null}
+                currentlyViewing={this.props.currentlyViewing}
+                updateCurrentlyViewing={this.props.updateCurrentlyViewing}
               >
                 {React.cloneElement(child, {
                   getScrollPosition: this.getScrollPosition,
diff --git a/app/javascript/mastodon/components/status.js b/app/javascript/mastodon/components/status.js
index e120278a0..12fc4a9a6 100644
--- a/app/javascript/mastodon/components/status.js
+++ b/app/javascript/mastodon/components/status.js
@@ -76,6 +76,7 @@ class Status extends ImmutablePureComponent {
     onEmbed: PropTypes.func,
     onHeightChange: PropTypes.func,
     onToggleHidden: PropTypes.func,
+    onToggleCollapsed: PropTypes.func,
     muted: PropTypes.bool,
     hidden: PropTypes.bool,
     unread: PropTypes.bool,
@@ -107,14 +108,6 @@ class Status extends ImmutablePureComponent {
     this.didShowCard = !this.props.muted && !this.props.hidden && this.props.status && this.props.status.get('card');
   }
 
-  getSnapshotBeforeUpdate () {
-    if (this.props.getScrollPosition) {
-      return this.props.getScrollPosition();
-    } else {
-      return null;
-    }
-  }
-
   static getDerivedStateFromProps(nextProps, prevState) {
     if (nextProps.status && nextProps.status.get('id') !== prevState.statusId) {
       return {
@@ -141,17 +134,6 @@ class Status extends ImmutablePureComponent {
     }
   }
 
-  componentWillUnmount() {
-    if (this.node && this.props.getScrollPosition) {
-      const position = this.props.getScrollPosition();
-      if (position !== null && this.node.offsetTop < position.top) {
-        requestAnimationFrame(() => {
-          this.props.updateScrollBottom(position.height - position.top);
-        });
-      }
-    }
-  }
-
   handleToggleMediaVisibility = () => {
     this.setState({ showMedia: !this.state.showMedia });
   }
@@ -196,7 +178,11 @@ class Status extends ImmutablePureComponent {
 
   handleExpandedToggle = () => {
     this.props.onToggleHidden(this._properStatus());
-  };
+  }
+
+  handleCollapsedToggle = isCollapsed => {
+    this.props.onToggleCollapsed(this._properStatus(), isCollapsed);
+  }
 
   renderLoadingMediaGallery () {
     return <div className='media-gallery' style={{ height: '110px' }} />;
@@ -466,7 +452,7 @@ class Status extends ImmutablePureComponent {
               </a>
             </div>
 
-            <StatusContent status={status} onClick={this.handleClick} expanded={!status.get('hidden')} onExpandedToggle={this.handleExpandedToggle} collapsable />
+            <StatusContent status={status} onClick={this.handleClick} expanded={!status.get('hidden')} onExpandedToggle={this.handleExpandedToggle} collapsable onCollapsedToggle={this.handleCollapsedToggle} />
 
             {media}
 
diff --git a/app/javascript/mastodon/components/status_content.js b/app/javascript/mastodon/components/status_content.js
index d13091325..5d921fd41 100644
--- a/app/javascript/mastodon/components/status_content.js
+++ b/app/javascript/mastodon/components/status_content.js
@@ -23,11 +23,11 @@ export default class StatusContent extends React.PureComponent {
     onExpandedToggle: PropTypes.func,
     onClick: PropTypes.func,
     collapsable: PropTypes.bool,
+    onCollapsedToggle: PropTypes.func,
   };
 
   state = {
     hidden: true,
-    collapsed: null, //  `collapsed: null` indicates that an element doesn't need collapsing, while `true` or `false` indicates that it does (and is/isn't).
   };
 
   _updateStatusLinks () {
@@ -62,14 +62,16 @@ export default class StatusContent extends React.PureComponent {
       link.setAttribute('rel', 'noopener noreferrer');
     }
 
-    if (
-      this.props.collapsable
-      && this.props.onClick
-      && this.state.collapsed === null
-      && node.clientHeight > MAX_HEIGHT
-      && this.props.status.get('spoiler_text').length === 0
-    ) {
-      this.setState({ collapsed: true });
+    if (this.props.status.get('collapsed', null) === null) {
+      let collapsed =
+          this.props.collapsable
+          && this.props.onClick
+          && node.clientHeight > MAX_HEIGHT
+          && this.props.status.get('spoiler_text').length === 0;
+
+      if(this.props.onCollapsedToggle) this.props.onCollapsedToggle(collapsed);
+
+      this.props.status.set('collapsed', collapsed);
     }
   }
 
@@ -178,6 +180,7 @@ export default class StatusContent extends React.PureComponent {
     }
 
     const hidden = this.props.onExpandedToggle ? !this.props.expanded : this.state.hidden;
+    const renderReadMore = this.props.onClick && status.get('collapsed');
 
     const content = { __html: status.get('contentHtml') };
     const spoilerContent = { __html: status.get('spoilerHtml') };
@@ -185,7 +188,7 @@ export default class StatusContent extends React.PureComponent {
     const classNames = classnames('status__content', {
       'status__content--with-action': this.props.onClick && this.context.router,
       'status__content--with-spoiler': status.get('spoiler_text').length > 0,
-      'status__content--collapsed': this.state.collapsed === true,
+      'status__content--collapsed': renderReadMore,
     });
 
     if (isRtl(status.get('search_index'))) {
@@ -237,7 +240,7 @@ export default class StatusContent extends React.PureComponent {
         </div>,
       ];
 
-      if (this.state.collapsed) {
+      if (renderReadMore) {
         output.push(readMoreButton);
       }
 
diff --git a/app/javascript/mastodon/components/status_list.js b/app/javascript/mastodon/components/status_list.js
index e1b370c91..82e069601 100644
--- a/app/javascript/mastodon/components/status_list.js
+++ b/app/javascript/mastodon/components/status_list.js
@@ -26,6 +26,8 @@ export default class StatusList extends ImmutablePureComponent {
     emptyMessage: PropTypes.node,
     alwaysPrepend: PropTypes.bool,
     timelineId: PropTypes.string,
+    currentlyViewing: PropTypes.number,
+    updateCurrentlyViewing: PropTypes.func,
   };
 
   static defaultProps = {
@@ -58,6 +60,12 @@ export default class StatusList extends ImmutablePureComponent {
     this.props.onLoadMore(this.props.statusIds.size > 0 ? this.props.statusIds.last() : undefined);
   }, 300, { leading: true })
 
+  updateCurrentlyViewingWithCache = (id) => {
+    if(this.cachedCurrentlyViewing === id) return;
+    this.cachedCurrentlyViewing = id;
+    this.props.updateCurrentlyViewing(id);
+  }
+
   _selectChild (index, align_top) {
     const container = this.node.node;
     const element = container.querySelector(`article:nth-of-type(${index + 1}) .focusable`);
@@ -79,6 +87,7 @@ export default class StatusList extends ImmutablePureComponent {
   render () {
     const { statusIds, featuredStatusIds, shouldUpdateScroll, onLoadMore, timelineId, ...other }  = this.props;
     const { isLoading, isPartial } = other;
+    other.updateCurrentlyViewing = this.updateCurrentlyViewingWithCache;
 
     if (isPartial) {
       return <RegenerationIndicator />;