From 81e2265e7203ffb6ff16d4f22d0721aa47999e84 Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Mon, 14 Nov 2022 09:42:06 +0000 Subject: [PATCH] Merge pull request #10397 from overleaf/td-memoize-file-outline Memoize file outline GitOrigin-RevId: cb086bab2b6ead251362180d776e7eaff18fc639 --- .../outline/components/outline-item.js | 37 +++++++++--------- .../outline/components/outline-list.js | 38 +++++++++++++++++-- .../outline/components/outline-root.js | 1 + .../outline/components/outline-item.test.js | 2 + 4 files changed, 57 insertions(+), 21 deletions(-) diff --git a/services/web/frontend/js/features/outline/components/outline-item.js b/services/web/frontend/js/features/outline/components/outline-item.js index 928934288e..03fe4acfa0 100644 --- a/services/web/frontend/js/features/outline/components/outline-item.js +++ b/services/web/frontend/js/features/outline/components/outline-item.js @@ -1,4 +1,4 @@ -import { useState, useEffect, useRef } from 'react' +import { useState, useEffect, useRef, memo } from 'react' import PropTypes from 'prop-types' import scrollIntoViewIfNeeded from 'scroll-into-view-if-needed' import classNames from 'classnames' @@ -6,15 +6,13 @@ import { useTranslation } from 'react-i18next' import OutlineList from './outline-list' import Icon from '../../../shared/components/icon' -function getChildrenLines(children) { - return (children || []) - .map(child => { - return getChildrenLines(child.children).concat(child.line) - }) - .flat() -} - -function OutlineItem({ outlineItem, jumpToLine, highlightedLine }) { +const OutlineItem = memo(function OutlineItem({ + outlineItem, + jumpToLine, + highlightedLine, + matchesHighlightedLine, + containsHighlightedLine, +}) { const { t } = useTranslation() const [expanded, setExpanded] = useState(true) @@ -25,12 +23,8 @@ function OutlineItem({ outlineItem, jumpToLine, highlightedLine }) { 'outline-item-no-children': !outlineItem.children, }) - const hasHighlightedChild = - !expanded && - getChildrenLines(outlineItem.children).includes(highlightedLine) - - const isHighlighted = - highlightedLine === outlineItem.line || hasHighlightedChild + const hasHighlightedChild = !expanded && containsHighlightedLine + const isHighlighted = matchesHighlightedLine || hasHighlightedChild const itemLinkClasses = classNames('outline-item-link', { 'outline-item-link-highlight': isHighlighted, @@ -90,16 +84,21 @@ function OutlineItem({ outlineItem, jumpToLine, highlightedLine }) { {expanded && outlineItem.children ? ( + // highlightedLine is only provided to this list if the list contains + // the highlighted line. This means that whenever the list does not + // contain the highlighted line, the props provided to it are the same + // and the component can be memoized. ) : null} ) -} +}) OutlineItem.propTypes = { outlineItem: PropTypes.exact({ @@ -113,6 +112,8 @@ OutlineItem.propTypes = { }).isRequired, jumpToLine: PropTypes.func.isRequired, highlightedLine: PropTypes.number, + matchesHighlightedLine: PropTypes.bool, + containsHighlightedLine: PropTypes.bool, } export default OutlineItem diff --git a/services/web/frontend/js/features/outline/components/outline-list.js b/services/web/frontend/js/features/outline/components/outline-list.js index c7992235be..d7930a01a1 100644 --- a/services/web/frontend/js/features/outline/components/outline-list.js +++ b/services/web/frontend/js/features/outline/components/outline-list.js @@ -1,32 +1,64 @@ import PropTypes from 'prop-types' import classNames from 'classnames' import OutlineItem from './outline-item' +import { memo } from 'react' -function OutlineList({ outline, jumpToLine, isRoot, highlightedLine }) { +function getChildrenLines(children) { + return (children || []) + .map(child => { + return getChildrenLines(child.children).concat(child.line) + }) + .flat() +} + +const OutlineList = memo(function OutlineList({ + outline, + jumpToLine, + isRoot, + highlightedLine, + containsHighlightedLine, +}) { const listClasses = classNames('outline-item-list', { 'outline-item-list-root': isRoot, }) return ( ) -} +}) OutlineList.propTypes = { outline: PropTypes.array.isRequired, jumpToLine: PropTypes.func.isRequired, isRoot: PropTypes.bool, highlightedLine: PropTypes.number, + containsHighlightedLine: PropTypes.bool, } export default OutlineList diff --git a/services/web/frontend/js/features/outline/components/outline-root.js b/services/web/frontend/js/features/outline/components/outline-root.js index 9d403ace8f..665a38e06a 100644 --- a/services/web/frontend/js/features/outline/components/outline-root.js +++ b/services/web/frontend/js/features/outline/components/outline-root.js @@ -14,6 +14,7 @@ function OutlineRoot({ outline, jumpToLine, highlightedLine }) { jumpToLine={jumpToLine} isRoot highlightedLine={highlightedLine} + containsHighlightedLine /> ) : (
diff --git a/services/web/test/frontend/features/outline/components/outline-item.test.js b/services/web/test/frontend/features/outline/components/outline-item.test.js index 0fdd9dd6dc..d557d395c8 100644 --- a/services/web/test/frontend/features/outline/components/outline-item.test.js +++ b/services/web/test/frontend/features/outline/components/outline-item.test.js @@ -54,6 +54,7 @@ describe('', function () { outlineItem={outlineItem} jumpToLine={jumpToLine} highlightedLine={1} + matchesHighlightedLine /> ) @@ -71,6 +72,7 @@ describe('', function () { outlineItem={outlineItem} jumpToLine={jumpToLine} highlightedLine={2} + containsHighlightedLine /> )