Explorar o código

Merge pull request #13911 from overleaf/td-review-panel-performance

Review panel: memoize entry views for performance

GitOrigin-RevId: 3c305845ad0914a7ffeb595e7235d7dceb4c780a
Tim Down %!s(int64=3) %!d(string=hai) anos
pai
achega
0d3af56efa
Modificáronse 24 ficheiros con 494 adicións e 309 borrados
  1. 17 7
      services/web/frontend/js/features/source-editor/components/review-panel/current-file-container.tsx
  2. 2 2
      services/web/frontend/js/features/source-editor/components/review-panel/editor-widgets/toggle-widget.tsx
  3. 2 2
      services/web/frontend/js/features/source-editor/components/review-panel/entries/add-comment-entry.tsx
  4. 24 35
      services/web/frontend/js/features/source-editor/components/review-panel/entries/aggregate-change-entry.tsx
  5. 3 17
      services/web/frontend/js/features/source-editor/components/review-panel/entries/bulk-actions-entry/bulk-actions-entry.tsx
  6. 3 2
      services/web/frontend/js/features/source-editor/components/review-panel/entries/bulk-actions-entry/modal.tsx
  7. 41 51
      services/web/frontend/js/features/source-editor/components/review-panel/entries/change-entry.tsx
  8. 20 25
      services/web/frontend/js/features/source-editor/components/review-panel/entries/comment-entry.tsx
  9. 5 8
      services/web/frontend/js/features/source-editor/components/review-panel/entries/comment.tsx
  10. 7 5
      services/web/frontend/js/features/source-editor/components/review-panel/entries/resolved-comment-entry.tsx
  11. 18 5
      services/web/frontend/js/features/source-editor/components/review-panel/overview-file.tsx
  12. 86 41
      services/web/frontend/js/features/source-editor/components/review-panel/positioned-entries.tsx
  13. 2 2
      services/web/frontend/js/features/source-editor/components/review-panel/toggler.tsx
  14. 11 9
      services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-dropdown.tsx
  15. 8 1
      services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-scroller.tsx
  16. 6 4
      services/web/frontend/js/features/source-editor/components/review-panel/toolbar/toggle-menu.tsx
  17. 21 0
      services/web/frontend/js/features/source-editor/components/review-panel/types/base-change-entry-props.ts
  18. 2 1
      services/web/frontend/js/features/source-editor/components/review-panel/upgrade-track-changes-modal.tsx
  19. 31 0
      services/web/frontend/js/features/source-editor/components/review-panel/utils/compare-props-with-shallow-array-compare.ts
  20. 72 55
      services/web/frontend/js/features/source-editor/context/review-panel/hooks/use-angular-review-panel-state.ts
  21. 23 19
      services/web/frontend/js/features/source-editor/context/review-panel/types/review-panel-state.ts
  22. 5 18
      services/web/frontend/stylesheets/app/editor/review-panel.less
  23. 81 0
      services/web/test/frontend/features/review-panel/unit/compare-props-with-shallow-array-compare.tests.ts
  24. 4 0
      services/web/types/review-panel/entry.ts

+ 17 - 7
services/web/frontend/js/features/source-editor/components/review-panel/current-file-container.tsx

@@ -34,9 +34,8 @@ function CurrentFileContainer() {
     users,
     entryHover,
     nVisibleSelectedChanges: nChanges,
-    toggleReviewPanel,
   } = useReviewPanelValueContext()
-  const { setEntryHover } = useReviewPanelUpdaterFnsContext()
+  const { setEntryHover, toggleReviewPanel } = useReviewPanelUpdaterFnsContext()
   const contentHeight = useCodeMirrorContentHeight()
 
   const currentDocEntries =
@@ -87,10 +86,15 @@ function CurrentFileContainer() {
                   <ChangeEntry
                     key={id}
                     docId={openDocId}
-                    entry={entry}
                     entryId={id}
                     permissions={permissions}
                     user={users[entry.metadata.user_id]}
+                    content={entry.content}
+                    offset={entry.offset}
+                    type={entry.type}
+                    focused={entry.focused}
+                    entryIds={entry.entry_ids}
+                    timestamp={entry.metadata.ts}
                     onMouseEnter={onMouseEnter}
                     onMouseLeave={onMouseLeave}
                     onIndicatorClick={toggleReviewPanel}
@@ -103,10 +107,15 @@ function CurrentFileContainer() {
                   <AggregateChangeEntry
                     key={id}
                     docId={openDocId}
-                    entry={entry}
                     entryId={id}
                     permissions={permissions}
                     user={users[entry.metadata.user_id]}
+                    content={entry.content}
+                    replacedContent={entry.metadata.replaced_content}
+                    offset={entry.offset}
+                    focused={entry.focused}
+                    entryIds={entry.entry_ids}
+                    timestamp={entry.metadata.ts}
                     onMouseEnter={onMouseEnter}
                     onMouseLeave={onMouseLeave}
                     onIndicatorClick={toggleReviewPanel}
@@ -123,10 +132,12 @@ function CurrentFileContainer() {
                   <CommentEntry
                     key={id}
                     docId={openDocId}
-                    entry={entry}
+                    threadId={entry.thread_id}
+                    thread={commentThreads[entry.thread_id]}
                     entryId={id}
+                    offset={entry.offset}
+                    focused={entry.focused}
                     permissions={permissions}
-                    threads={commentThreads}
                     onMouseEnter={onMouseEnter}
                     onMouseLeave={onMouseLeave}
                     onIndicatorClick={toggleReviewPanel}
@@ -142,7 +153,6 @@ function CurrentFileContainer() {
                 return (
                   <BulkActionsEntry
                     key={id}
-                    entry={entry}
                     entryId={entry.type}
                     nChanges={nChanges}
                   />

+ 2 - 2
services/web/frontend/js/features/source-editor/components/review-panel/editor-widgets/toggle-widget.tsx

@@ -1,11 +1,11 @@
 import { Trans } from 'react-i18next'
-import { useReviewPanelValueContext } from '../../../context/review-panel/review-panel-context'
+import { useReviewPanelUpdaterFnsContext } from '../../../context/review-panel/review-panel-context'
 import { useCodeMirrorStateContext } from '../../codemirror-editor'
 import { EditorView } from '@codemirror/view'
 import classnames from 'classnames'
 
 function ToggleWidget() {
-  const { toggleReviewPanel } = useReviewPanelValueContext()
+  const { toggleReviewPanel } = useReviewPanelUpdaterFnsContext()
   const state = useCodeMirrorStateContext()
   const darkTheme = state.facet(EditorView.darkTheme)
 

+ 2 - 2
services/web/frontend/js/features/source-editor/components/review-panel/entries/add-comment-entry.tsx

@@ -19,8 +19,8 @@ type AddCommentEntryProps = {
 
 function AddCommentEntry({ entryId }: AddCommentEntryProps) {
   const { t } = useTranslation()
-  const { isAddingComment, submitNewComment } = useReviewPanelValueContext()
-  const { setIsAddingComment, handleLayoutChange } =
+  const { isAddingComment } = useReviewPanelValueContext()
+  const { setIsAddingComment, submitNewComment, handleLayoutChange } =
     useReviewPanelUpdaterFnsContext()
 
   const [content, setContent] = useState('')

+ 24 - 35
services/web/frontend/js/features/source-editor/components/review-panel/entries/aggregate-change-entry.tsx

@@ -1,55 +1,41 @@
 import { useTranslation } from 'react-i18next'
-import { useState } from 'react'
+import { memo, useState } from 'react'
 import EntryContainer from './entry-container'
 import EntryCallout from './entry-callout'
 import EntryActions from './entry-actions'
 import Icon from '../../../../../shared/components/icon'
-import {
-  useReviewPanelUpdaterFnsContext,
-  useReviewPanelValueContext,
-} from '../../../context/review-panel/review-panel-context'
+import { useReviewPanelUpdaterFnsContext } from '../../../context/review-panel/review-panel-context'
 import { formatTime } from '../../../../utils/format-date'
 import classnames from 'classnames'
-import { ReviewPanelAggregateChangeEntry } from '../../../../../../../types/review-panel/entry'
-import {
-  ReviewPanelPermissions,
-  ReviewPanelUser,
-  ThreadId,
-} from '../../../../../../../types/review-panel/review-panel'
-import { DocId } from '../../../../../../../types/project-settings'
+import comparePropsWithShallowArrayCompare from '../utils/compare-props-with-shallow-array-compare'
+import { BaseChangeEntryProps } from '../types/base-change-entry-props'
 
-type AggregateChangeEntryProps = {
-  docId: DocId
-  entry: ReviewPanelAggregateChangeEntry
-  entryId: ThreadId
-  permissions: ReviewPanelPermissions
-  user: ReviewPanelUser | undefined
-  contentLimit?: number
-  onMouseEnter?: () => void
-  onMouseLeave?: () => void
-  onIndicatorClick?: () => void
+interface AggregateChangeEntryProps extends BaseChangeEntryProps {
+  replacedContent: string
 }
 
 function AggregateChangeEntry({
   docId,
-  entry,
   entryId,
   permissions,
   user,
+  content,
+  replacedContent,
+  offset,
+  focused,
+  entryIds,
+  timestamp,
   contentLimit = 17,
   onMouseEnter,
   onMouseLeave,
   onIndicatorClick,
 }: AggregateChangeEntryProps) {
   const { t } = useTranslation()
-  const { acceptChanges, rejectChanges, gotoEntry } =
-    useReviewPanelValueContext()
-  const { handleLayoutChange } = useReviewPanelUpdaterFnsContext()
+  const { acceptChanges, rejectChanges, gotoEntry, handleLayoutChange } =
+    useReviewPanelUpdaterFnsContext()
   const [isDeletionCollapsed, setIsDeletionCollapsed] = useState(true)
   const [isInsertionCollapsed, setIsInsertionCollapsed] = useState(true)
 
-  const replacedContent = entry.metadata.replaced_content
-  const content = entry.content
   const deletionNeedsCollapsing = replacedContent.length > contentLimit
   const insertionNeedsCollapsing = content.length > contentLimit
 
@@ -71,7 +57,7 @@ function AggregateChangeEntry({
       '.rp-entry-action-icon i',
     ]) {
       if (target.matches(selector)) {
-        gotoEntry(docId, entry.offset)
+        gotoEntry(docId, offset)
         break
       }
     }
@@ -98,7 +84,7 @@ function AggregateChangeEntry({
       {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events,jsx-a11y/no-static-element-interactions */}
       <div
         className={classnames('rp-entry-indicator', {
-          'rp-entry-indicator-focused': entry.focused,
+          'rp-entry-indicator-focused': focused,
         })}
         onClick={onIndicatorClick}
       >
@@ -106,7 +92,7 @@ function AggregateChangeEntry({
       </div>
       <div
         className={classnames('rp-entry', 'rp-entry-aggregate', {
-          'rp-entry-focused': entry.focused,
+          'rp-entry-focused': focused,
         })}
       >
         <div className="rp-entry-body">
@@ -141,7 +127,7 @@ function AggregateChangeEntry({
               )}
             </div>
             <div className="rp-entry-metadata">
-              {formatTime(entry.metadata.ts, 'MMM D, Y h:mm A')}
+              {formatTime(timestamp, 'MMM D, Y h:mm A')}
               &nbsp;&bull;&nbsp;
               {user && (
                 <span
@@ -156,10 +142,10 @@ function AggregateChangeEntry({
         </div>
         {permissions.write && (
           <EntryActions>
-            <EntryActions.Button onClick={() => rejectChanges(entry.entry_ids)}>
+            <EntryActions.Button onClick={() => rejectChanges(entryIds)}>
               <Icon type="times" /> {t('reject')}
             </EntryActions.Button>
-            <EntryActions.Button onClick={() => acceptChanges(entry.entry_ids)}>
+            <EntryActions.Button onClick={() => acceptChanges(entryIds)}>
               <Icon type="check" /> {t('accept')}
             </EntryActions.Button>
           </EntryActions>
@@ -169,4 +155,7 @@ function AggregateChangeEntry({
   )
 }
 
-export default AggregateChangeEntry
+export default memo(
+  AggregateChangeEntry,
+  comparePropsWithShallowArrayCompare('entryIds')
+)

+ 3 - 17
services/web/frontend/js/features/source-editor/components/review-panel/entries/bulk-actions-entry/bulk-actions-entry.tsx

@@ -7,12 +7,11 @@ import Modal, { useBulkActionsModal } from './modal'
 import { ReviewPanelBulkActionsEntry } from '../../../../../../../../types/review-panel/entry'
 
 type BulkActionsEntryProps = {
-  entry: ReviewPanelBulkActionsEntry
   entryId: ReviewPanelBulkActionsEntry['type']
   nChanges: number
 }
 
-function BulkActionsEntry({ entry, entryId, nChanges }: BulkActionsEntryProps) {
+function BulkActionsEntry({ entryId, nChanges }: BulkActionsEntryProps) {
   const { t } = useTranslation()
   const {
     show,
@@ -28,21 +27,8 @@ function BulkActionsEntry({ entry, entryId, nChanges }: BulkActionsEntryProps) {
       <EntryContainer id={entryId}>
         {nChanges > 1 && (
           <>
-            <EntryCallout
-              className="rp-entry-callout-bulk-actions"
-              style={{
-                top: entry.screenPos
-                  ? entry.screenPos.y + entry.screenPos.height - 1 + 'px'
-                  : undefined,
-              }}
-            />
-            <BulkActions
-              className="rp-entry"
-              style={{
-                top: entry.screenPos.y + 'px',
-                visibility: entry.visible ? 'visible' : 'hidden',
-              }}
-            >
+            <EntryCallout className="rp-entry-callout-bulk-actions" />
+            <BulkActions className="rp-entry">
               <BulkActions.Button onClick={handleShowBulkRejectDialog}>
                 <Icon type="times" /> {t('reject_all')} ({nChanges})
               </BulkActions.Button>

+ 3 - 2
services/web/frontend/js/features/source-editor/components/review-panel/entries/bulk-actions-entry/modal.tsx

@@ -2,7 +2,7 @@ import { useState, useCallback } from 'react'
 import { useTranslation } from 'react-i18next'
 import { Button, Modal as BootstrapModal } from 'react-bootstrap'
 import AccessibleModal from '../../../../../../shared/components/accessible-modal'
-import { useReviewPanelValueContext } from '../../../../context/review-panel/review-panel-context'
+import { useReviewPanelUpdaterFnsContext } from '../../../../context/review-panel/review-panel-context'
 
 type BulkActionsModalProps = {
   show: boolean
@@ -52,7 +52,8 @@ function Modal({
 export function useBulkActionsModal() {
   const [show, setShow] = useState(false)
   const [isAccept, setIsAccept] = useState(false)
-  const { bulkAcceptActions, bulkRejectActions } = useReviewPanelValueContext()
+  const { bulkAcceptActions, bulkRejectActions } =
+    useReviewPanelUpdaterFnsContext()
 
   const handleShowBulkAcceptDialog = useCallback(() => {
     setIsAccept(true)

+ 41 - 51
services/web/frontend/js/features/source-editor/components/review-panel/entries/change-entry.tsx

@@ -1,60 +1,47 @@
 import { useTranslation } from 'react-i18next'
-import { useState } from 'react'
+import { memo, useState } from 'react'
 import EntryContainer from './entry-container'
 import EntryCallout from './entry-callout'
 import EntryActions from './entry-actions'
 import Icon from '../../../../../shared/components/icon'
-import {
-  useReviewPanelUpdaterFnsContext,
-  useReviewPanelValueContext,
-} from '../../../context/review-panel/review-panel-context'
+import { useReviewPanelUpdaterFnsContext } from '../../../context/review-panel/review-panel-context'
 import { formatTime } from '../../../../utils/format-date'
 import classnames from 'classnames'
-import {
-  ReviewPanelDeleteEntry,
-  ReviewPanelInsertEntry,
-} from '../../../../../../../types/review-panel/entry'
-import {
-  ReviewPanelPermissions,
-  ReviewPanelUser,
-  ThreadId,
-} from '../../../../../../../types/review-panel/review-panel'
-import { DocId } from '../../../../../../../types/project-settings'
+import { ReviewPanelChangeEntry } from '../../../../../../../types/review-panel/entry'
+import { BaseChangeEntryProps } from '../types/base-change-entry-props'
+import comparePropsWithShallowArrayCompare from '../utils/compare-props-with-shallow-array-compare'
 
-type ChangeEntryProps = {
-  docId: DocId
-  entry: ReviewPanelInsertEntry | ReviewPanelDeleteEntry
-  entryId: ThreadId
-  permissions: ReviewPanelPermissions
-  user: ReviewPanelUser | undefined
-  contentLimit?: number
-  onMouseEnter?: () => void
-  onMouseLeave?: () => void
-  onIndicatorClick?: () => void
+interface ChangeEntryProps extends BaseChangeEntryProps {
+  type: ReviewPanelChangeEntry['type']
 }
 
 function ChangeEntry({
   docId,
-  entry,
   entryId,
   permissions,
   user,
+  content,
+  offset,
+  type,
+  focused,
+  entryIds,
+  timestamp,
   contentLimit = 40,
   onMouseEnter,
   onMouseLeave,
   onIndicatorClick,
 }: ChangeEntryProps) {
   const { t } = useTranslation()
-  const { acceptChanges, rejectChanges, gotoEntry } =
-    useReviewPanelValueContext()
-  const { handleLayoutChange } = useReviewPanelUpdaterFnsContext()
+  const { handleLayoutChange, acceptChanges, rejectChanges, gotoEntry } =
+    useReviewPanelUpdaterFnsContext()
   const [isCollapsed, setIsCollapsed] = useState(true)
 
-  const content = isCollapsed
-    ? entry.content.substring(0, contentLimit)
-    : entry.content
+  const contentToDisplay = isCollapsed
+    ? content.substring(0, contentLimit)
+    : content
 
-  const needsCollapsing = entry.content.length > contentLimit
+  const needsCollapsing = content.length > contentLimit
+  const isInsert = type === 'insert'
 
   const handleEntryClick = (e: React.MouseEvent<HTMLDivElement>) => {
     const target = e.target as Element
@@ -66,7 +53,7 @@ function ChangeEntry({
       '.rp-entry-action-icon i',
     ]) {
       if (target.matches(selector)) {
-        gotoEntry(docId, entry.offset)
+        gotoEntry(docId, offset)
         break
       }
     }
@@ -84,28 +71,24 @@ function ChangeEntry({
       onMouseLeave={onMouseLeave}
       id={entryId}
     >
-      <EntryCallout className={`rp-entry-callout-${entry.type}`} />
+      <EntryCallout className={`rp-entry-callout-${type}`} />
       {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
       <div
         className={classnames('rp-entry-indicator', {
-          'rp-entry-indicator-focused': entry.focused,
+          'rp-entry-indicator-focused': focused,
         })}
         onClick={onIndicatorClick}
       >
-        {entry.type === 'insert' ? (
-          <Icon type="pencil" />
-        ) : (
-          <i className="rp-icon-delete" />
-        )}
+        {isInsert ? <Icon type="pencil" /> : <i className="rp-icon-delete" />}
       </div>
       <div
-        className={classnames('rp-entry', `rp-entry-${entry.type}`, {
-          'rp-entry-focused': entry.focused,
+        className={classnames('rp-entry', `rp-entry-${type}`, {
+          'rp-entry-focused': focused,
         })}
       >
         <div className="rp-entry-body">
           <div className="rp-entry-action-icon">
-            {entry.type === 'insert' ? (
+            {isInsert ? (
               <Icon type="pencil" />
             ) : (
               <i className="rp-icon-delete" />
@@ -114,15 +97,19 @@ function ChangeEntry({
           <div className="rp-entry-details">
             <div className="rp-entry-description">
               <span>
-                {entry.type === 'insert' ? (
+                {isInsert ? (
                   <>
                     {t('tracked_change_added')}&nbsp;
-                    <ins className="rp-content-highlight">{content}</ins>
+                    <ins className="rp-content-highlight">
+                      {contentToDisplay}
+                    </ins>
                   </>
                 ) : (
                   <>
                     {t('tracked_change_deleted')}&nbsp;
-                    <del className="rp-content-highlight">{content}</del>
+                    <del className="rp-content-highlight">
+                      {contentToDisplay}
+                    </del>
                   </>
                 )}
                 {needsCollapsing && (
@@ -138,7 +125,7 @@ function ChangeEntry({
               </span>
             </div>
             <div className="rp-entry-metadata">
-              {formatTime(entry.metadata.ts, 'MMM D, Y h:mm A')}
+              {formatTime(timestamp, 'MMM D, Y h:mm A')}
               &nbsp;&bull;&nbsp;
               {user && (
                 <span
@@ -153,10 +140,10 @@ function ChangeEntry({
         </div>
         {permissions.write && (
           <EntryActions>
-            <EntryActions.Button onClick={() => rejectChanges(entry.entry_ids)}>
+            <EntryActions.Button onClick={() => rejectChanges(entryIds)}>
               <Icon type="times" /> {t('reject')}
             </EntryActions.Button>
-            <EntryActions.Button onClick={() => acceptChanges(entry.entry_ids)}>
+            <EntryActions.Button onClick={() => acceptChanges(entryIds)}>
               <Icon type="check" /> {t('accept')}
             </EntryActions.Button>
           </EntryActions>
@@ -166,4 +153,7 @@ function ChangeEntry({
   )
 }
 
-export default ChangeEntry
+export default memo(
+  ChangeEntry,
+  comparePropsWithShallowArrayCompare('entryIds')
+)

+ 20 - 25
services/web/frontend/js/features/source-editor/components/review-panel/entries/comment-entry.tsx

@@ -1,4 +1,4 @@
-import { useState, useRef, useEffect } from 'react'
+import { useState, useRef, useEffect, memo } from 'react'
 import { useTranslation } from 'react-i18next'
 import EntryContainer from './entry-container'
 import EntryCallout from './entry-callout'
@@ -8,52 +8,47 @@ import AutoExpandingTextArea, {
   resetHeight,
 } from '../../../../../shared/components/auto-expanding-text-area'
 import Icon from '../../../../../shared/components/icon'
-import {
-  useReviewPanelUpdaterFnsContext,
-  useReviewPanelValueContext,
-} from '../../../context/review-panel/review-panel-context'
+import { useReviewPanelUpdaterFnsContext } from '../../../context/review-panel/review-panel-context'
 import classnames from 'classnames'
-import { ReviewPanelCommentEntry } from '../../../../../../../types/review-panel/entry'
 import {
-  ReviewPanelCommentThreads,
   ReviewPanelPermissions,
   ThreadId,
 } from '../../../../../../../types/review-panel/review-panel'
 import { DocId } from '../../../../../../../types/project-settings'
+import { ReviewPanelCommentThread } from '../../../../../../../types/review-panel/comment-thread'
+import { ReviewPanelCommentEntry } from '../../../../../../../types/review-panel/entry'
 
 type CommentEntryProps = {
   docId: DocId
-  entry: ReviewPanelCommentEntry
   entryId: ThreadId
+  thread: ReviewPanelCommentThread | undefined
+  threadId: ReviewPanelCommentEntry['thread_id']
   permissions: ReviewPanelPermissions
-  threads: ReviewPanelCommentThreads
   onMouseEnter?: () => void
   onMouseLeave?: () => void
   onIndicatorClick?: () => void
-}
+} & Pick<ReviewPanelCommentEntry, 'offset' | 'focused'>
 
 function CommentEntry({
   docId,
-  entry,
   entryId,
+  thread,
+  threadId,
+  offset,
+  focused,
   permissions,
-  threads,
   onMouseEnter,
   onMouseLeave,
   onIndicatorClick,
 }: CommentEntryProps) {
   const { t } = useTranslation()
-  const { gotoEntry, resolveComment, submitReply } =
-    useReviewPanelValueContext()
-  const { handleLayoutChange } = useReviewPanelUpdaterFnsContext()
+  const { gotoEntry, resolveComment, submitReply, handleLayoutChange } =
+    useReviewPanelUpdaterFnsContext()
   const [replyContent, setReplyContent] = useState('')
   const [animating, setAnimating] = useState(false)
   const [resolved, setResolved] = useState(false)
   const entryDivRef = useRef<HTMLDivElement | null>(null)
 
-  const thread =
-    entry.thread_id in threads ? threads[entry.thread_id] : undefined
-
   const handleEntryClick = (e: React.MouseEvent<HTMLDivElement>) => {
     const target = e.target as Element
 
@@ -65,7 +60,7 @@ function CommentEntry({
       '.rp-entry-metadata',
     ]) {
       if (target.matches(selector)) {
-        gotoEntry(docId, entry.offset)
+        gotoEntry(docId, offset)
         break
       }
     }
@@ -93,7 +88,7 @@ function CommentEntry({
 
       if (replyContent.length) {
         ;(e.target as HTMLTextAreaElement).blur()
-        submitReply(entry, replyContent)
+        submitReply(threadId, replyContent)
         setReplyContent('')
         resetHeight(e)
       }
@@ -102,7 +97,7 @@ function CommentEntry({
 
   const handleOnReply = () => {
     if (replyContent.length) {
-      submitReply(entry, replyContent)
+      submitReply(threadId, replyContent)
       setReplyContent('')
     }
   }
@@ -139,7 +134,7 @@ function CommentEntry({
         {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
         <div
           className={classnames('rp-entry-indicator', {
-            'rp-entry-indicator-focused': entry.focused,
+            'rp-entry-indicator-focused': focused,
           })}
           onClick={onIndicatorClick}
         >
@@ -147,7 +142,7 @@ function CommentEntry({
         </div>
         <div
           className={classnames('rp-entry', 'rp-entry-comment', {
-            'rp-entry-focused': entry.focused,
+            'rp-entry-focused': focused,
             'rp-entry-comment-resolving': animating,
           })}
           ref={entryDivRef}
@@ -160,7 +155,7 @@ function CommentEntry({
               <Comment
                 key={comment.id}
                 thread={thread}
-                threadId={entry.thread_id}
+                threadId={threadId}
                 comment={comment}
               />
             ))}
@@ -204,4 +199,4 @@ function CommentEntry({
   )
 }
 
-export default CommentEntry
+export default memo(CommentEntry)

+ 5 - 8
services/web/frontend/js/features/source-editor/components/review-panel/entries/comment.tsx

@@ -1,11 +1,8 @@
 import { useTranslation } from 'react-i18next'
-import { useState } from 'react'
+import { memo, useState } from 'react'
 import AutoExpandingTextArea from '../../../../../shared/components/auto-expanding-text-area'
 import { formatTime } from '../../../../utils/format-date'
-import {
-  useReviewPanelUpdaterFnsContext,
-  useReviewPanelValueContext,
-} from '../../../context/review-panel/review-panel-context'
+import { useReviewPanelUpdaterFnsContext } from '../../../context/review-panel/review-panel-context'
 import { ReviewPanelCommentThread } from '../../../../../../../types/review-panel/comment-thread'
 import {
   ReviewPanelCommentThreadMessage,
@@ -20,8 +17,8 @@ type CommentProps = {
 
 function Comment({ thread, threadId, comment }: CommentProps) {
   const { t } = useTranslation()
-  const { deleteComment, saveEdit } = useReviewPanelValueContext()
-  const { handleLayoutChange } = useReviewPanelUpdaterFnsContext()
+  const { handleLayoutChange, deleteComment, saveEdit } =
+    useReviewPanelUpdaterFnsContext()
   const [deleting, setDeleting] = useState(false)
   const [editing, setEditing] = useState(false)
 
@@ -120,4 +117,4 @@ function Comment({ thread, threadId, comment }: CommentProps) {
   )
 }
 
-export default Comment
+export default memo(Comment)

+ 7 - 5
services/web/frontend/js/features/source-editor/components/review-panel/entries/resolved-comment-entry.tsx

@@ -1,9 +1,10 @@
 import { useTranslation } from 'react-i18next'
-import { useState } from 'react'
+import { memo, useState } from 'react'
 import Linkify from 'react-linkify'
 import { formatTime } from '../../../../utils/format-date'
-import { useReviewPanelValueContext } from '../../../context/review-panel/review-panel-context'
+import { useReviewPanelUpdaterFnsContext } from '../../../context/review-panel/review-panel-context'
 import { FilteredResolvedComments } from '../toolbar/resolved-comments-dropdown'
+import { ReviewPanelPermissions } from '../../../../../../../types/review-panel/review-panel'
 
 function LinkDecorator(
   decoratedHref: string,
@@ -19,16 +20,17 @@ function LinkDecorator(
 
 type ResolvedCommentEntryProps = {
   thread: FilteredResolvedComments
+  permissions: ReviewPanelPermissions
   contentLimit?: number
 }
 
 function ResolvedCommentEntry({
   thread,
+  permissions,
   contentLimit = 40,
 }: ResolvedCommentEntryProps) {
   const { t } = useTranslation()
-  const { permissions, unresolveComment, deleteThread } =
-    useReviewPanelValueContext()
+  const { unresolveComment, deleteThread } = useReviewPanelUpdaterFnsContext()
   const [isCollapsed, setIsCollapsed] = useState(false)
   const needsCollapsing = thread.content.length > contentLimit
   const content = isCollapsed
@@ -122,4 +124,4 @@ function ResolvedCommentEntry({
   )
 }
 
-export default ResolvedCommentEntry
+export default memo(ResolvedCommentEntry)

+ 18 - 5
services/web/frontend/js/features/source-editor/components/review-panel/overview-file.tsx

@@ -81,10 +81,15 @@ function OverviewFile({ docId, docPath }: OverviewFileProps) {
               <ChangeEntry
                 key={id}
                 docId={docId}
-                entry={entry}
                 entryId={id}
                 permissions={permissions}
                 user={users[entry.metadata.user_id]}
+                content={entry.content}
+                offset={entry.offset}
+                type={entry.type}
+                focused={entry.focused}
+                entryIds={entry.entry_ids}
+                timestamp={entry.metadata.ts}
               />
             )
           }
@@ -94,24 +99,32 @@ function OverviewFile({ docId, docPath }: OverviewFileProps) {
               <AggregateChangeEntry
                 key={id}
                 docId={docId}
-                entry={entry}
                 entryId={id}
                 permissions={permissions}
                 user={users[entry.metadata.user_id]}
+                content={entry.content}
+                replacedContent={entry.metadata.replaced_content}
+                offset={entry.offset}
+                focused={entry.focused}
+                entryIds={entry.entry_ids}
+                timestamp={entry.metadata.ts}
               />
             )
           }
 
           if (entry.type === 'comment') {
-            if (!commentThreads[entry.thread_id]?.resolved) {
+            const thread = commentThreads[entry.thread_id]
+            if (!thread?.resolved) {
               return (
                 <CommentEntry
                   key={id}
                   docId={docId}
-                  entry={entry}
+                  threadId={entry.thread_id}
+                  thread={thread}
                   entryId={id}
+                  offset={entry.offset}
+                  focused={entry.focused}
                   permissions={permissions}
-                  threads={commentThreads}
                 />
               )
             }

+ 86 - 41
services/web/frontend/js/features/source-editor/components/review-panel/positioned-entries.tsx

@@ -4,14 +4,20 @@ import {
   ReviewPanelEntry,
   ReviewPanelEntryScreenPos,
 } from '../../../../../../types/review-panel/entry'
-import useScopeValue from '../../../../shared/hooks/use-scope-value'
 import { debugConsole } from '../../../../utils/debugging'
 import { useReviewPanelValueContext } from '../../context/review-panel/review-panel-context'
 import useEventListener from '../../../../shared/hooks/use-event-listener'
 import { ReviewPanelDocEntries } from '../../../../../../types/review-panel/review-panel'
 import { dispatchReviewPanelLayout } from '../../extensions/changes/change-manager'
+import { isEqual } from 'lodash'
+
+type Positions = {
+  entryTop: number
+  callout: { top: number; height: number; inverted: boolean }
+}
 
 type EntryView = {
+  entryId: keyof ReviewPanelDocEntries
   wrapper: HTMLElement
   indicator: HTMLElement | null
   box: HTMLElement
@@ -22,10 +28,16 @@ type EntryView = {
   previousCalloutTop: number | null
   entry: ReviewPanelEntry
   visible: boolean
-  positions?: {
-    entryTop: number
-    callout: { top: number; height: number; inverted: boolean }
-  }
+  positions?: Positions
+}
+
+type EntryPositions = Pick<EntryView, 'entryId' | 'positions'>
+
+type LayoutInfo = {
+  focusedEntryIndex: number
+  overflowTop: number
+  height: number
+  positions: EntryPositions[]
 }
 
 function css(el: HTMLElement, props: React.CSSProperties) {
@@ -53,6 +65,13 @@ function calculateCalloutPosition(
   }
 }
 
+function positionsEqual(
+  entryPos1: EntryPositions[],
+  entryPos2: EntryPositions[]
+) {
+  return isEqual(entryPos1, entryPos2)
+}
+
 const calculateEntryViewPositions = (
   entryViews: EntryView[],
   lineHeight: number,
@@ -90,14 +109,16 @@ function PositionedEntries({
   contentHeight,
   children,
 }: PositionedEntriesProps) {
-  const { navHeight, toolbarHeight } = useReviewPanelValueContext()
+  const { navHeight, toolbarHeight, lineHeight } = useReviewPanelValueContext()
   const containerRef = useRef<HTMLDivElement | null>(null)
   const { reviewPanelOpen } = useLayoutContext()
-  const [lineHeight] = useScopeValue<number>(
-    'reviewPanel.rendererData.lineHeight'
-  )
   const animationTimerRef = useRef<number | null>(null)
-  const previousFocusedEntryIndexRef = useRef(0)
+  const previousLayoutInfoRef = useRef<LayoutInfo>({
+    focusedEntryIndex: 0,
+    overflowTop: 0,
+    height: 0,
+    positions: [],
+  })
 
   const layout = () => {
     const container = containerRef.current
@@ -117,7 +138,9 @@ function PositionedEntries({
     for (const wrapper of container.querySelectorAll<HTMLElement>(
       '.rp-entry-wrapper'
     )) {
-      const entryId = wrapper.dataset.entryId
+      const entryId = wrapper.dataset.entryId as
+        | EntryView['entryId']
+        | undefined
       if (!entryId) {
         throw new Error('Could not find an entry ID')
       }
@@ -138,6 +161,7 @@ function PositionedEntries({
         const previousEntryTopData = box.dataset.previousEntryTop
         const previousCalloutTopData = callout.dataset.previousEntryTop
         entryViews.push({
+          entryId,
           wrapper,
           indicator,
           box,
@@ -200,11 +224,10 @@ function PositionedEntries({
     let focusedEntryIndex = entryViews.findIndex(view => view.entry.focused)
     if (focusedEntryIndex === -1) {
       focusedEntryIndex = Math.min(
-        previousFocusedEntryIndexRef.current,
+        previousLayoutInfoRef.current.focusedEntryIndex,
         entryViews.length - 1
       )
     }
-    previousFocusedEntryIndexRef.current = focusedEntryIndex
 
     const focusedEntryView = entryViews[focusedEntryIndex]
     if (!focusedEntryView.entry.screenPos) {
@@ -352,43 +375,65 @@ function PositionedEntries({
       animationTimerRef.current = null
     }
 
-    moveEntriesToInitialPosition()
-
-    // Inform the editor of the new top overflow
-    window.dispatchEvent(
-      new CustomEvent('review-panel:event', {
-        detail: {
-          type: 'sizes',
-          payload: {
-            overflowTop,
-            height: lastEntryBottom + navPaddedHeight,
-          },
-        },
+    // Check whether the positions of any entry have changed since the last
+    // layout
+    const positions = entryViews.map(
+      (entryView): EntryPositions => ({
+        entryId: entryView.entryId,
+        positions: entryView.positions,
       })
     )
 
-    // Schedule the final, animated move
-    animationTimerRef.current = window.setTimeout(moveToFinalPositions, 60)
+    const positionsChanged = !positionsEqual(
+      previousLayoutInfoRef.current.positions,
+      positions
+    )
+
+    // Check whether the top overflow or review panel height have changed
+    const overflowTopChanged =
+      overflowTop !== previousLayoutInfoRef.current.overflowTop
+
+    const height = lastEntryBottom + navPaddedHeight
+    const heightChanged = height !== previousLayoutInfoRef.current.height
+
+    // Move entries into their initial positions
+    if (positionsChanged || overflowTopChanged) {
+      moveEntriesToInitialPosition()
+    }
+
+    // Inform the editor of the new top overflow and/or height if either has
+    // changed
+    if (overflowTopChanged || heightChanged) {
+      window.dispatchEvent(
+        new CustomEvent('review-panel:event', {
+          detail: {
+            type: 'sizes',
+            payload: {
+              overflowTop,
+              height,
+            },
+          },
+        })
+      )
+    }
+
+    // Do the final move
+    if (positionsChanged || overflowTopChanged) {
+      moveToFinalPositions()
+    }
+
+    previousLayoutInfoRef.current = {
+      positions,
+      focusedEntryIndex,
+      height,
+      overflowTop,
+    }
   }
 
   useEventListener('review-panel:layout', () => {
-    if (animationTimerRef.current) {
-      window.clearTimeout(animationTimerRef.current)
-      animationTimerRef.current = null
-    }
     layout()
   })
 
-  // Cancel scheduled move on unmount
-  useEffect(() => {
-    return () => {
-      if (animationTimerRef.current) {
-        window.clearTimeout(animationTimerRef.current)
-        animationTimerRef.current = null
-      }
-    }
-  }, [])
-
   // Layout on first render. This is necessary to ensure layout happens when
   // switching from overview to current file view
   useEffect(() => {

+ 2 - 2
services/web/frontend/js/features/source-editor/components/review-panel/toggler.tsx

@@ -1,9 +1,9 @@
 import { useTranslation } from 'react-i18next'
-import { useReviewPanelValueContext } from '../../context/review-panel/review-panel-context'
+import { useReviewPanelUpdaterFnsContext } from '../../context/review-panel/review-panel-context'
 
 function Toggler() {
   const { t } = useTranslation()
-  const { toggleReviewPanel } = useReviewPanelValueContext()
+  const { toggleReviewPanel } = useReviewPanelUpdaterFnsContext()
 
   const handleTogglerClick = (event: React.MouseEvent<HTMLButtonElement>) => {
     const target = event.target as HTMLButtonElement

+ 11 - 9
services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-dropdown.tsx

@@ -4,7 +4,10 @@ import Icon from '../../../../../shared/components/icon'
 import Tooltip from '../../../../../shared/components/tooltip'
 import ResolvedCommentsScroller from './resolved-comments-scroller'
 import classnames from 'classnames'
-import { useReviewPanelValueContext } from '../../../context/review-panel/review-panel-context'
+import {
+  useReviewPanelUpdaterFnsContext,
+  useReviewPanelValueContext,
+} from '../../../context/review-panel/review-panel-context'
 import {
   ReviewPanelDocEntries,
   ThreadId,
@@ -26,12 +29,10 @@ function ResolvedCommentsDropdown() {
   const { t } = useTranslation()
   const [isOpen, setIsOpen] = useState(false)
   const [isLoading, setIsLoading] = useState(false)
-  const {
-    docs,
-    commentThreads,
-    resolvedComments,
-    refreshResolvedCommentsDropdown,
-  } = useReviewPanelValueContext()
+  const { docs, commentThreads, resolvedComments, permissions } =
+    useReviewPanelValueContext()
+
+  const { refreshResolvedCommentsDropdown } = useReviewPanelUpdaterFnsContext()
 
   const handleResolvedCommentsClick = () => {
     setIsOpen(isOpen => {
@@ -117,11 +118,12 @@ function ResolvedCommentsDropdown() {
           <div className="rp-loading">
             <Icon type="spinner" spin />
           </div>
-        ) : (
+        ) : isOpen ? (
           <ResolvedCommentsScroller
             resolvedComments={filteredResolvedComments}
+            permissions={permissions}
           />
-        )}
+        ) : null}
       </div>
     </div>
   )

+ 8 - 1
services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-scroller.tsx

@@ -2,13 +2,16 @@ import { useTranslation } from 'react-i18next'
 import { useMemo } from 'react'
 import ResolvedCommentEntry from '../entries/resolved-comment-entry'
 import { FilteredResolvedComments } from './resolved-comments-dropdown'
+import { ReviewPanelPermissions } from '../../../../../../../types/review-panel/review-panel'
 
 type ResolvedCommentsScrollerProps = {
   resolvedComments: FilteredResolvedComments[]
+  permissions: ReviewPanelPermissions
 }
 
 function ResolvedCommentsScroller({
   resolvedComments,
+  permissions,
 }: ResolvedCommentsScrollerProps) {
   const { t } = useTranslation()
 
@@ -21,7 +24,11 @@ function ResolvedCommentsScroller({
   return (
     <div className="resolved-comments-scroller">
       {sortedResolvedComments.map(comment => (
-        <ResolvedCommentEntry key={comment.entryId} thread={comment} />
+        <ResolvedCommentEntry
+          key={comment.entryId}
+          thread={comment}
+          permissions={permissions}
+        />
       ))}
       {!resolvedComments.length && (
         <div className="rp-loading">{t('no_resolved_threads')}</div>

+ 6 - 4
services/web/frontend/js/features/source-editor/components/review-panel/toolbar/toggle-menu.tsx

@@ -23,14 +23,16 @@ const sendAnalytics = () => {
 function ToggleMenu() {
   const { t } = useTranslation()
   const project = useProjectContext()
-  const { setShouldCollapse } = useReviewPanelUpdaterFnsContext()
   const {
-    permissions,
-    wantTrackChanges,
-    shouldCollapse,
+    setShouldCollapse,
     toggleTrackChangesForEveryone,
     toggleTrackChangesForUser,
     toggleTrackChangesForGuests,
+  } = useReviewPanelUpdaterFnsContext()
+  const {
+    permissions,
+    wantTrackChanges,
+    shouldCollapse,
     trackChangesState,
     trackChangesOnForEveryone,
     trackChangesOnForGuests,

+ 21 - 0
services/web/frontend/js/features/source-editor/components/review-panel/types/base-change-entry-props.ts

@@ -0,0 +1,21 @@
+import { ReviewPanelChangeEntry } from '../../../../../../../types/review-panel/entry'
+import { DocId } from '../../../../../../../types/project-settings'
+import {
+  ReviewPanelPermissions,
+  ReviewPanelUser,
+  ThreadId,
+} from '../../../../../../../types/review-panel/review-panel'
+
+export interface BaseChangeEntryProps
+  extends Pick<ReviewPanelChangeEntry, 'content' | 'offset' | 'focused'> {
+  docId: DocId
+  entryId: ThreadId
+  permissions: ReviewPanelPermissions
+  user: ReviewPanelUser | undefined
+  timestamp: ReviewPanelChangeEntry['metadata']['ts']
+  contentLimit?: number
+  entryIds: ReviewPanelChangeEntry['entry_ids']
+  onMouseEnter?: () => void
+  onMouseLeave?: () => void
+  onIndicatorClick?: () => void
+}

+ 2 - 1
services/web/frontend/js/features/source-editor/components/review-panel/upgrade-track-changes-modal.tsx

@@ -5,6 +5,7 @@ import Icon from '../../../../shared/components/icon'
 import { useProjectContext } from '../../../../shared/context/project-context'
 import { useUserContext } from '../../../../shared/context/user-context'
 import { startFreeTrial, upgradePlan } from '../../../../main/account-upgrade'
+import { memo } from 'react'
 
 type UpgradeTrackChangesModalProps = {
   show: boolean
@@ -102,4 +103,4 @@ function UpgradeTrackChangesModal({
   )
 }
 
-export default UpgradeTrackChangesModal
+export default memo(UpgradeTrackChangesModal)

+ 31 - 0
services/web/frontend/js/features/source-editor/components/review-panel/utils/compare-props-with-shallow-array-compare.ts

@@ -0,0 +1,31 @@
+const shallowEqual = (arr1: unknown[], arr2: unknown[]) =>
+  arr1.length === arr2.length && !arr1.some((val, index) => val !== arr2[index])
+
+// Compares props for a component, but comparing the specified props using
+// shallow array comparison rather than identity
+export default function comparePropsWithShallowArrayCompare<
+  T extends Record<string, unknown>
+>(...args: Array<keyof T>) {
+  return (prevProps: T, nextProps: T) => {
+    for (const k in prevProps) {
+      const prev = prevProps[k]
+      const next = nextProps[k]
+      if (Object.is(prev, next)) {
+        continue
+      }
+
+      if (!args.includes(k)) {
+        return false
+      }
+
+      if (
+        !Array.isArray(prev) ||
+        !Array.isArray(next) ||
+        !shallowEqual(prev, next)
+      ) {
+        return false
+      }
+    }
+    return true
+  }
+}

+ 72 - 55
services/web/frontend/js/features/source-editor/context/review-panel/hooks/use-angular-review-panel-state.ts

@@ -3,8 +3,10 @@ import useScopeValue from '../../../../../shared/hooks/use-scope-value'
 import { sendMB } from '../../../../../infrastructure/event-tracking'
 import { ReviewPanelState } from '../types/review-panel-state'
 import * as ReviewPanel from '../types/review-panel-state'
-import { SubView } from '../../../../../../../types/review-panel/review-panel'
-import { ReviewPanelCommentEntry } from '../../../../../../../types/review-panel/entry'
+import {
+  SubView,
+  ThreadId,
+} from '../../../../../../../types/review-panel/review-panel'
 import { dispatchReviewPanelLayout as handleLayoutChange } from '../../../extensions/changes/change-manager'
 
 function useAngularReviewPanelState(): ReviewPanelState {
@@ -47,15 +49,18 @@ function useAngularReviewPanelState(): ReviewPanelState {
   const [shouldCollapse, setShouldCollapse] = useScopeValue<
     ReviewPanel.Value<'shouldCollapse'>
   >('reviewPanel.fullTCStateCollapsed')
+  const [lineHeight] = useScopeValue<number>(
+    'reviewPanel.rendererData.lineHeight'
+  )
 
   const [toggleTrackChangesForEveryone] = useScopeValue<
-    ReviewPanel.Value<'toggleTrackChangesForEveryone'>
+    ReviewPanel.UpdaterFn<'toggleTrackChangesForEveryone'>
   >('toggleTrackChangesForEveryone')
   const [toggleTrackChangesForUser] = useScopeValue<
-    ReviewPanel.Value<'toggleTrackChangesForUser'>
+    ReviewPanel.UpdaterFn<'toggleTrackChangesForUser'>
   >('toggleTrackChangesForUser')
   const [toggleTrackChangesForGuests] = useScopeValue<
-    ReviewPanel.Value<'toggleTrackChangesForGuests'>
+    ReviewPanel.UpdaterFn<'toggleTrackChangesForGuests'>
   >('toggleTrackChangesForGuests')
 
   const [trackChangesState] = useScopeValue<
@@ -71,37 +76,47 @@ function useAngularReviewPanelState(): ReviewPanelState {
     ReviewPanel.Value<'trackChangesForGuestsAvailable'>
   >('reviewPanel.trackChangesForGuestsAvailable')
   const [resolveComment] =
-    useScopeValue<ReviewPanel.Value<'resolveComment'>>('resolveComment')
+    useScopeValue<ReviewPanel.UpdaterFn<'resolveComment'>>('resolveComment')
   const [submitNewComment] =
-    useScopeValue<ReviewPanel.Value<'submitNewComment'>>('submitNewComment')
+    useScopeValue<ReviewPanel.UpdaterFn<'submitNewComment'>>('submitNewComment')
   const [deleteComment] =
-    useScopeValue<ReviewPanel.Value<'deleteComment'>>('deleteComment')
-  const [gotoEntry] = useScopeValue<ReviewPanel.Value<'gotoEntry'>>('gotoEntry')
-  const [saveEdit] = useScopeValue<ReviewPanel.Value<'saveEdit'>>('saveEdit')
+    useScopeValue<ReviewPanel.UpdaterFn<'deleteComment'>>('deleteComment')
+  const [gotoEntry] =
+    useScopeValue<ReviewPanel.UpdaterFn<'gotoEntry'>>('gotoEntry')
+  const [saveEdit] =
+    useScopeValue<ReviewPanel.UpdaterFn<'saveEdit'>>('saveEdit')
   const [submitReplyAngular] =
-    useScopeValue<(entry: ReviewPanelCommentEntry) => void>('submitReply')
+    useScopeValue<
+      (entry: { thread_id: ThreadId; replyContent: string }) => void
+    >('submitReply')
 
   const [formattedProjectMembers] = useScopeValue<
     ReviewPanel.Value<'formattedProjectMembers'>
   >('reviewPanel.formattedProjectMembers')
 
   const [toggleReviewPanel] =
-    useScopeValue<ReviewPanel.Value<'toggleReviewPanel'>>('toggleReviewPanel')
+    useScopeValue<ReviewPanel.UpdaterFn<'toggleReviewPanel'>>(
+      'toggleReviewPanel'
+    )
   const [unresolveComment] =
-    useScopeValue<ReviewPanel.Value<'unresolveComment'>>('unresolveComment')
+    useScopeValue<ReviewPanel.UpdaterFn<'unresolveComment'>>('unresolveComment')
   const [deleteThread] =
-    useScopeValue<ReviewPanel.Value<'deleteThread'>>('deleteThread')
+    useScopeValue<ReviewPanel.UpdaterFn<'deleteThread'>>('deleteThread')
   const [refreshResolvedCommentsDropdown] = useScopeValue<
-    ReviewPanel.Value<'refreshResolvedCommentsDropdown'>
+    ReviewPanel.UpdaterFn<'refreshResolvedCommentsDropdown'>
   >('refreshResolvedCommentsDropdown')
   const [acceptChanges] =
-    useScopeValue<ReviewPanel.Value<'acceptChanges'>>('acceptChanges')
+    useScopeValue<ReviewPanel.UpdaterFn<'acceptChanges'>>('acceptChanges')
   const [rejectChanges] =
-    useScopeValue<ReviewPanel.Value<'rejectChanges'>>('rejectChanges')
+    useScopeValue<ReviewPanel.UpdaterFn<'rejectChanges'>>('rejectChanges')
   const [bulkAcceptActions] =
-    useScopeValue<ReviewPanel.Value<'bulkAcceptActions'>>('bulkAcceptActions')
+    useScopeValue<ReviewPanel.UpdaterFn<'bulkAcceptActions'>>(
+      'bulkAcceptActions'
+    )
   const [bulkRejectActions] =
-    useScopeValue<ReviewPanel.Value<'bulkRejectActions'>>('bulkRejectActions')
+    useScopeValue<ReviewPanel.UpdaterFn<'bulkRejectActions'>>(
+      'bulkRejectActions'
+    )
 
   const handleSetSubview = useCallback(
     (subView: SubView) => {
@@ -112,8 +127,8 @@ function useAngularReviewPanelState(): ReviewPanelState {
   )
 
   const submitReply = useCallback(
-    (entry: ReviewPanelCommentEntry, replyContent: string) => {
-      submitReplyAngular({ ...entry, replyContent })
+    (threadId: ThreadId, replyContent: string) => {
+      submitReplyAngular({ thread_id: threadId, replyContent })
     },
     [submitReplyAngular]
   )
@@ -127,86 +142,54 @@ function useAngularReviewPanelState(): ReviewPanelState {
     () => ({
       collapsed,
       commentThreads,
-      deleteComment,
       docs,
       entries,
       entryHover,
       isAddingComment,
-      gotoEntry,
       loadingThreads,
       nVisibleSelectedChanges,
       permissions,
       users,
-      resolveComment,
       resolvedComments,
-      saveEdit,
       shouldCollapse,
       navHeight,
       toolbarHeight,
-      submitReply,
       subView,
       wantTrackChanges,
       loading,
       openDocId,
-      toggleTrackChangesForEveryone,
-      toggleTrackChangesForUser,
-      toggleTrackChangesForGuests,
+      lineHeight,
       trackChangesState,
       trackChangesOnForEveryone,
       trackChangesOnForGuests,
       trackChangesForGuestsAvailable,
       formattedProjectMembers,
-      toggleReviewPanel,
-      bulkAcceptActions,
-      bulkRejectActions,
-      unresolveComment,
-      deleteThread,
-      refreshResolvedCommentsDropdown,
-      acceptChanges,
-      rejectChanges,
-      submitNewComment,
     }),
     [
       collapsed,
       commentThreads,
-      deleteComment,
       docs,
       entries,
       entryHover,
       isAddingComment,
-      gotoEntry,
       loadingThreads,
       nVisibleSelectedChanges,
       permissions,
       users,
-      resolveComment,
       resolvedComments,
-      saveEdit,
       shouldCollapse,
       navHeight,
       toolbarHeight,
-      submitReply,
       subView,
       wantTrackChanges,
       loading,
       openDocId,
-      toggleTrackChangesForEveryone,
-      toggleTrackChangesForUser,
-      toggleTrackChangesForGuests,
+      lineHeight,
       trackChangesState,
       trackChangesOnForEveryone,
       trackChangesOnForGuests,
       trackChangesForGuestsAvailable,
       formattedProjectMembers,
-      toggleReviewPanel,
-      bulkAcceptActions,
-      bulkRejectActions,
-      unresolveComment,
-      deleteThread,
-      refreshResolvedCommentsDropdown,
-      acceptChanges,
-      rejectChanges,
-      submitNewComment,
     ]
   )
 
@@ -214,6 +197,23 @@ function useAngularReviewPanelState(): ReviewPanelState {
     () => ({
       handleSetSubview,
       handleLayoutChange,
+      gotoEntry,
+      resolveComment,
+      submitReply,
+      acceptChanges,
+      rejectChanges,
+      toggleReviewPanel,
+      bulkAcceptActions,
+      bulkRejectActions,
+      saveEdit,
+      submitNewComment,
+      deleteComment,
+      unresolveComment,
+      refreshResolvedCommentsDropdown,
+      deleteThread,
+      toggleTrackChangesForEveryone,
+      toggleTrackChangesForUser,
+      toggleTrackChangesForGuests,
       setEntryHover,
       setCollapsed,
       setShouldCollapse,
@@ -223,6 +223,23 @@ function useAngularReviewPanelState(): ReviewPanelState {
     }),
     [
       handleSetSubview,
+      gotoEntry,
+      resolveComment,
+      submitReply,
+      acceptChanges,
+      rejectChanges,
+      toggleReviewPanel,
+      bulkAcceptActions,
+      bulkRejectActions,
+      saveEdit,
+      submitNewComment,
+      deleteComment,
+      unresolveComment,
+      refreshResolvedCommentsDropdown,
+      deleteThread,
+      toggleTrackChangesForEveryone,
+      toggleTrackChangesForUser,
+      toggleTrackChangesForGuests,
       setCollapsed,
       setEntryHover,
       setShouldCollapse,

+ 23 - 19
services/web/frontend/js/features/source-editor/context/review-panel/types/review-panel-state.ts

@@ -7,7 +7,6 @@ import {
   SubView,
   ThreadId,
 } from '../../../../../../../types/review-panel/review-panel'
-import { ReviewPanelCommentEntry } from '../../../../../../../types/review-panel/entry'
 import {
   DocId,
   MainDocument,
@@ -18,34 +17,23 @@ export interface ReviewPanelState {
   values: {
     collapsed: Record<DocId, boolean>
     commentThreads: ReviewPanelCommentThreads
-    deleteComment: (threadId: ThreadId, commentId: CommentId) => void
     docs: MainDocument[] | undefined
     entries: ReviewPanelEntries
     entryHover: boolean
     isAddingComment: boolean
-    gotoEntry: (docId: DocId, entryOffset: number) => void
     loadingThreads: boolean
     nVisibleSelectedChanges: number
     permissions: ReviewPanelPermissions
     users: ReviewPanelUsers
-    resolveComment: (docId: DocId, entryId: ThreadId) => void
     resolvedComments: ReviewPanelEntries
-    saveEdit: (
-      threadId: ThreadId,
-      commentId: CommentId,
-      content: string
-    ) => void
     shouldCollapse: boolean
     navHeight: number
     toolbarHeight: number
-    submitReply: (entry: ReviewPanelCommentEntry, replyContent: string) => void
     subView: SubView
     wantTrackChanges: boolean
     loading: boolean
     openDocId: DocId | null
-    toggleTrackChangesForEveryone: (isOn: boolean) => unknown
-    toggleTrackChangesForUser: (isOn: boolean, memberId: string) => unknown
-    toggleTrackChangesForGuests: (isOn: boolean) => unknown
+    lineHeight: number
     trackChangesState: Record<string, { value: boolean; syncState: string }>
     trackChangesOnForEveryone: boolean
     trackChangesOnForGuests: boolean
@@ -57,19 +45,31 @@ export interface ReviewPanelState {
         name: string
       }
     >
+  }
+  updaterFns: {
+    handleSetSubview: (subView: SubView) => void
+    handleLayoutChange: () => void
+    gotoEntry: (docId: DocId, entryOffset: number) => void
+    resolveComment: (docId: DocId, entryId: ThreadId) => void
+    deleteComment: (threadId: ThreadId, commentId: CommentId) => void
+    submitReply: (threadId: ThreadId, replyContent: string) => void
+    acceptChanges: (entryIds: unknown) => void
+    rejectChanges: (entryIds: unknown) => void
+    toggleTrackChangesForEveryone: (isOn: boolean) => unknown
+    toggleTrackChangesForUser: (isOn: boolean, memberId: string) => unknown
+    toggleTrackChangesForGuests: (isOn: boolean) => unknown
     toggleReviewPanel: () => void
     bulkAcceptActions: () => void
     bulkRejectActions: () => void
+    saveEdit: (
+      threadId: ThreadId,
+      commentId: CommentId,
+      content: string
+    ) => void
     unresolveComment: (threadId: ThreadId) => void
     deleteThread: (_entryId: unknown, docId: DocId, threadId: ThreadId) => void
     refreshResolvedCommentsDropdown: () => Promise<void>
-    acceptChanges: (entryIds: unknown) => void
-    rejectChanges: (entryIds: unknown) => void
     submitNewComment: (content: string) => void
-  }
-  updaterFns: {
-    handleSetSubview: (subView: SubView) => void
-    handleLayoutChange: () => void
     setEntryHover: React.Dispatch<React.SetStateAction<Value<'entryHover'>>>
     setIsAddingComment: React.Dispatch<
       React.SetStateAction<Value<'isAddingComment'>>
@@ -89,3 +89,7 @@ export interface ReviewPanelState {
 // Getter for values
 export type Value<T extends keyof ReviewPanelState['values']> =
   ReviewPanelState['values'][T]
+
+// Getter for stable functions
+export type UpdaterFn<T extends keyof ReviewPanelState['updaterFns']> =
+  ReviewPanelState['updaterFns'][T]

+ 5 - 18
services/web/frontend/stylesheets/app/editor/review-panel.less

@@ -27,8 +27,6 @@
 @rp-toolbar-height: 32px;
 
 @rp-entry-animation-speed: 0.3s;
-// Move a little faster in React to compensate for the delay before moving the vertical position
-@rp-entry-animation-speed-react: 0.2s;
 
 .rp-button() {
   display: block; // IE doesn't do flex with inline items.
@@ -67,8 +65,6 @@
 }
 
 #review-panel {
-  --rp-animation-speed: @rp-entry-animation-speed;
-
   display: block;
 
   .rp-size-expanded & {
@@ -252,7 +248,7 @@
   border-radius: 3px;
   color: #fff;
   cursor: pointer;
-  transition: top var(--rp-animation-speed), left 0.1s, right 0.1s;
+  transition: top @rp-entry-animation-speed, left 0.1s, right 0.1s;
 
   .no-animate & {
     transition: left 0.1s, right 0.1s;
@@ -376,7 +372,7 @@
   border-left: solid @rp-entry-ribbon-width transparent;
   border-radius: 3px;
   background-color: #fff;
-  transition: top var(--rp-animation-speed), left 0.1s, right 0.1s;
+  transition: top @rp-entry-animation-speed, left 0.1s, right 0.1s;
 
   .no-animate & {
     transition: left 0.1s, right 0.1s;
@@ -644,7 +640,7 @@
 }
 
 .rp-entry-callout {
-  transition: top var(--rp-animation-speed), height var(--rp-animation-speed);
+  transition: top @rp-entry-animation-speed, height @rp-entry-animation-speed;
 
   .rp-state-current-file & {
     position: absolute;
@@ -1197,8 +1193,6 @@ button when (@is-overleaf-light = true) {
 
 // CM6-specific review panel rules
 .ol-cm-review-panel {
-  --rp-animation-speed: @rp-entry-animation-speed-react;
-
   position: relative;
   z-index: 6;
   display: block;
@@ -1262,13 +1256,7 @@ button when (@is-overleaf-light = true) {
 
   .rp-entry-list-react {
     position: relative;
-
-    .rp-entry-list-react-inner {
-      position: absolute;
-      top: 0;
-      left: 0;
-      right: 0;
-    }
+    overflow-x: hidden;
   }
 
   .rp-state-current-file & {
@@ -1294,8 +1282,7 @@ button when (@is-overleaf-light = true) {
 
   .rp-overview-file {
     .rp-overview-file-entries {
-      //height: auto;
-      transition: height ease-in-out 0.15s; //, display 0.15s 0s;
+      transition: height ease-in-out 0.15s;
     }
   }
 

+ 81 - 0
services/web/test/frontend/features/review-panel/unit/compare-props-with-shallow-array-compare.tests.ts

@@ -0,0 +1,81 @@
+import { expect } from 'chai'
+import comparePropsWithShallowArrayCompare from '../../../../../frontend/js/features/source-editor/components/review-panel/utils/compare-props-with-shallow-array-compare'
+
+describe('comparePropsWithShallowArrayCompare', function () {
+  it('is true with all equal non-array props', function () {
+    type NoArrayProps = { prop1: string; prop2: number }
+
+    const props1: NoArrayProps = { prop1: 'wombat', prop2: 1 }
+    const props2: NoArrayProps = { prop1: 'wombat', prop2: 1 }
+
+    expect(comparePropsWithShallowArrayCompare()(props1, props2)).to.be.true
+  })
+
+  it('is false with non-equal non-array props', function () {
+    type NoArrayProps = { prop1: string; prop2: number }
+
+    const props1: NoArrayProps = { prop1: 'wombat', prop2: 1 }
+    const props2: NoArrayProps = { prop1: 'squirrel', prop2: 1 }
+
+    expect(comparePropsWithShallowArrayCompare()(props1, props2)).to.be.false
+  })
+
+  it('is false with similar but not specified array prop', function () {
+    type ArrayProps = { prop1: string; prop2: number[] }
+
+    const props1: ArrayProps = { prop1: 'wombat', prop2: [1] }
+    const props2: ArrayProps = { prop1: 'wombat', prop2: [1] }
+
+    expect(comparePropsWithShallowArrayCompare()(props1, props2)).to.be.false
+  })
+
+  it('is true with similar and specified array prop', function () {
+    type ArrayProps = { prop1: string; prop2: number[] }
+
+    const props1: ArrayProps = { prop1: 'wombat', prop2: [1] }
+    const props2: ArrayProps = { prop1: 'wombat', prop2: [1] }
+
+    expect(
+      comparePropsWithShallowArrayCompare<ArrayProps>('prop2')(props1, props2)
+    ).to.be.true
+  })
+
+  it('is false with non-similar and specified array prop', function () {
+    type ArrayProps = { prop1: string; prop2: number[] }
+
+    const props1: ArrayProps = { prop1: 'wombat', prop2: [1] }
+    const props2: ArrayProps = { prop1: 'wombat', prop2: [2] }
+
+    expect(
+      comparePropsWithShallowArrayCompare<ArrayProps>('prop2')(props1, props2)
+    ).to.be.false
+  })
+
+  it('is false with multiple similar array props with not all specified', function () {
+    type MultipleArrayProps = { prop1: number[]; prop2: number[] }
+
+    const props1: MultipleArrayProps = { prop1: [1], prop2: [2] }
+    const props2: MultipleArrayProps = { prop1: [1], prop2: [2] }
+
+    expect(
+      comparePropsWithShallowArrayCompare<MultipleArrayProps>('prop1')(
+        props1,
+        props2
+      )
+    ).to.be.false
+  })
+
+  it('is true with multiple similar array props with all specified', function () {
+    type MultipleArrayProps = { prop1: number[]; prop2: number[] }
+
+    const props1: MultipleArrayProps = { prop1: [1], prop2: [2] }
+    const props2: MultipleArrayProps = { prop1: [1], prop2: [2] }
+
+    expect(
+      comparePropsWithShallowArrayCompare<MultipleArrayProps>('prop1', 'prop2')(
+        props1,
+        props2
+      )
+    ).to.be.true
+  })
+})

+ 4 - 0
services/web/types/review-panel/entry.ts

@@ -34,6 +34,10 @@ export interface ReviewPanelDeleteEntry
   type: 'delete'
 }
 
+export type ReviewPanelChangeEntry =
+  | ReviewPanelInsertEntry
+  | ReviewPanelDeleteEntry
+
 export interface ReviewPanelCommentEntry extends ReviewPanelBaseEntry {
   type: 'comment'
   content: string