From 0d3af56efa2c58ed4190f0b0fb97df6ce951b8b8 Mon Sep 17 00:00:00 2001 From: Tim Down <158919+timdown@users.noreply.github.com> Date: Thu, 20 Jul 2023 11:48:02 +0100 Subject: [PATCH] Merge pull request #13911 from overleaf/td-review-panel-performance Review panel: memoize entry views for performance GitOrigin-RevId: 3c305845ad0914a7ffeb595e7235d7dceb4c780a --- .../review-panel/current-file-container.tsx | 24 +++- .../editor-widgets/toggle-widget.tsx | 4 +- .../entries/add-comment-entry.tsx | 4 +- .../entries/aggregate-change-entry.tsx | 59 ++++---- .../bulk-actions-entry/bulk-actions-entry.tsx | 20 +-- .../entries/bulk-actions-entry/modal.tsx | 5 +- .../review-panel/entries/change-entry.tsx | 92 ++++++------- .../review-panel/entries/comment-entry.tsx | 45 +++---- .../review-panel/entries/comment.tsx | 13 +- .../entries/resolved-comment-entry.tsx | 12 +- .../components/review-panel/overview-file.tsx | 23 +++- .../review-panel/positioned-entries.tsx | 127 ++++++++++++------ .../components/review-panel/toggler.tsx | 4 +- .../toolbar/resolved-comments-dropdown.tsx | 20 +-- .../toolbar/resolved-comments-scroller.tsx | 9 +- .../review-panel/toolbar/toggle-menu.tsx | 10 +- .../types/base-change-entry-props.ts | 21 +++ .../upgrade-track-changes-modal.tsx | 3 +- ...ompare-props-with-shallow-array-compare.ts | 31 +++++ .../hooks/use-angular-review-panel-state.ts | 127 ++++++++++-------- .../review-panel/types/review-panel-state.ts | 48 ++++--- .../stylesheets/app/editor/review-panel.less | 23 +--- ...-props-with-shallow-array-compare.tests.ts | 81 +++++++++++ services/web/types/review-panel/entry.ts | 4 + 24 files changed, 497 insertions(+), 312 deletions(-) create mode 100644 services/web/frontend/js/features/source-editor/components/review-panel/types/base-change-entry-props.ts create mode 100644 services/web/frontend/js/features/source-editor/components/review-panel/utils/compare-props-with-shallow-array-compare.ts create mode 100644 services/web/test/frontend/features/review-panel/unit/compare-props-with-shallow-array-compare.tests.ts diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/current-file-container.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/current-file-container.tsx index 23ddbac84e..59f64f954a 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/current-file-container.tsx +++ b/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() { diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/editor-widgets/toggle-widget.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/editor-widgets/toggle-widget.tsx index 4ed3fc417b..fbad4ea4fe 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/editor-widgets/toggle-widget.tsx +++ b/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) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/add-comment-entry.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/add-comment-entry.tsx index 02d80a96df..d4a4aef76d 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/add-comment-entry.tsx +++ b/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('') diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/aggregate-change-entry.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/aggregate-change-entry.tsx index cbf458987e..ef33626a3d 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/aggregate-change-entry.tsx +++ b/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 */}
@@ -106,7 +92,7 @@ function AggregateChangeEntry({
@@ -141,7 +127,7 @@ function AggregateChangeEntry({ )}
- {formatTime(entry.metadata.ts, 'MMM D, Y h:mm A')} + {formatTime(timestamp, 'MMM D, Y h:mm A')}  •  {user && ( {permissions.write && ( - rejectChanges(entry.entry_ids)}> + rejectChanges(entryIds)}> {t('reject')} - acceptChanges(entry.entry_ids)}> + acceptChanges(entryIds)}> {t('accept')} @@ -169,4 +155,7 @@ function AggregateChangeEntry({ ) } -export default AggregateChangeEntry +export default memo( + AggregateChangeEntry, + comparePropsWithShallowArrayCompare('entryIds') +) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/bulk-actions-entry/bulk-actions-entry.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/bulk-actions-entry/bulk-actions-entry.tsx index 302df036bb..2be53c1748 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/bulk-actions-entry/bulk-actions-entry.tsx +++ b/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) { {nChanges > 1 && ( <> - - + + {t('reject_all')} ({nChanges}) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/bulk-actions-entry/modal.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/bulk-actions-entry/modal.tsx index 325259a38b..d05b67d83e 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/bulk-actions-entry/modal.tsx +++ b/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) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/change-entry.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/change-entry.tsx index 3250eb8db0..644cd882b0 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/change-entry.tsx +++ b/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) => { 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} > - + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */}
- {entry.type === 'insert' ? ( - - ) : ( - - )} + {isInsert ? : }
- {entry.type === 'insert' ? ( + {isInsert ? ( ) : ( @@ -114,15 +97,19 @@ function ChangeEntry({
- {entry.type === 'insert' ? ( + {isInsert ? ( <> {t('tracked_change_added')}  - {content} + + {contentToDisplay} + ) : ( <> {t('tracked_change_deleted')}  - {content} + + {contentToDisplay} + )} {needsCollapsing && ( @@ -138,7 +125,7 @@ function ChangeEntry({
- {formatTime(entry.metadata.ts, 'MMM D, Y h:mm A')} + {formatTime(timestamp, 'MMM D, Y h:mm A')}  •  {user && ( {permissions.write && ( - rejectChanges(entry.entry_ids)}> + rejectChanges(entryIds)}> {t('reject')} - acceptChanges(entry.entry_ids)}> + acceptChanges(entryIds)}> {t('accept')} @@ -166,4 +153,7 @@ function ChangeEntry({ ) } -export default ChangeEntry +export default memo( + ChangeEntry, + comparePropsWithShallowArrayCompare('entryIds') +) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/comment-entry.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/comment-entry.tsx index 5d06dc5766..59f726c3aa 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/comment-entry.tsx +++ b/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 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(null) - const thread = - entry.thread_id in threads ? threads[entry.thread_id] : undefined - const handleEntryClick = (e: React.MouseEvent) => { 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 */}
@@ -147,7 +142,7 @@ function CommentEntry({
))} @@ -204,4 +199,4 @@ function CommentEntry({ ) } -export default CommentEntry +export default memo(CommentEntry) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/comment.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/comment.tsx index ac3a8538bc..bec6e40bab 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/comment.tsx +++ b/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) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/resolved-comment-entry.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/resolved-comment-entry.tsx index effe9d344a..9da9830190 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/resolved-comment-entry.tsx +++ b/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) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/overview-file.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/overview-file.tsx index a4b293bce4..de479c886a 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/overview-file.tsx +++ b/services/web/frontend/js/features/source-editor/components/review-panel/overview-file.tsx @@ -81,10 +81,15 @@ function OverviewFile({ docId, docPath }: OverviewFileProps) { ) } @@ -94,24 +99,32 @@ function OverviewFile({ docId, docPath }: OverviewFileProps) { ) } if (entry.type === 'comment') { - if (!commentThreads[entry.thread_id]?.resolved) { + const thread = commentThreads[entry.thread_id] + if (!thread?.resolved) { return ( ) } diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/positioned-entries.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/positioned-entries.tsx index 116aa51237..7095984f38 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/positioned-entries.tsx +++ b/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 + +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(null) const { reviewPanelOpen } = useLayoutContext() - const [lineHeight] = useScopeValue( - 'reviewPanel.rendererData.lineHeight' - ) const animationTimerRef = useRef(null) - const previousFocusedEntryIndexRef = useRef(0) + const previousLayoutInfoRef = useRef({ + 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( '.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(() => { diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/toggler.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/toggler.tsx index a465ddfd84..1ab93329f1 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/toggler.tsx +++ b/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) => { const target = event.target as HTMLButtonElement diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-dropdown.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-dropdown.tsx index fb23091bf4..a18c4edee6 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-dropdown.tsx +++ b/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() {
- ) : ( + ) : isOpen ? ( - )} + ) : null}
) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-scroller.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-scroller.tsx index 39cc57002a..f62f2a36dd 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-scroller.tsx +++ b/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 (
{sortedResolvedComments.map(comment => ( - + ))} {!resolvedComments.length && (
{t('no_resolved_threads')}
diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/toggle-menu.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/toggle-menu.tsx index 2ba2a27c52..050e5882d8 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/toggle-menu.tsx +++ b/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 { + setShouldCollapse, + toggleTrackChangesForEveryone, + toggleTrackChangesForUser, + toggleTrackChangesForGuests, + } = useReviewPanelUpdaterFnsContext() const { permissions, wantTrackChanges, shouldCollapse, - toggleTrackChangesForEveryone, - toggleTrackChangesForUser, - toggleTrackChangesForGuests, trackChangesState, trackChangesOnForEveryone, trackChangesOnForGuests, diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/types/base-change-entry-props.ts b/services/web/frontend/js/features/source-editor/components/review-panel/types/base-change-entry-props.ts new file mode 100644 index 0000000000..79bf78c830 --- /dev/null +++ b/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 { + 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 +} diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/upgrade-track-changes-modal.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/upgrade-track-changes-modal.tsx index f191aaf171..657e18c1e7 100644 --- a/services/web/frontend/js/features/source-editor/components/review-panel/upgrade-track-changes-modal.tsx +++ b/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) diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/utils/compare-props-with-shallow-array-compare.ts b/services/web/frontend/js/features/source-editor/components/review-panel/utils/compare-props-with-shallow-array-compare.ts new file mode 100644 index 0000000000..07ad6def04 --- /dev/null +++ b/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 +>(...args: Array) { + 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 + } +} diff --git a/services/web/frontend/js/features/source-editor/context/review-panel/hooks/use-angular-review-panel-state.ts b/services/web/frontend/js/features/source-editor/context/review-panel/hooks/use-angular-review-panel-state.ts index 4aec1a2b42..d1a96ac390 100644 --- a/services/web/frontend/js/features/source-editor/context/review-panel/hooks/use-angular-review-panel-state.ts +++ b/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( + '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>('resolveComment') + useScopeValue>('resolveComment') const [submitNewComment] = - useScopeValue>('submitNewComment') + useScopeValue>('submitNewComment') const [deleteComment] = - useScopeValue>('deleteComment') - const [gotoEntry] = useScopeValue>('gotoEntry') - const [saveEdit] = useScopeValue>('saveEdit') + useScopeValue>('deleteComment') + const [gotoEntry] = + useScopeValue>('gotoEntry') + const [saveEdit] = + useScopeValue>('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>('toggleReviewPanel') + useScopeValue>( + 'toggleReviewPanel' + ) const [unresolveComment] = - useScopeValue>('unresolveComment') + useScopeValue>('unresolveComment') const [deleteThread] = - useScopeValue>('deleteThread') + useScopeValue>('deleteThread') const [refreshResolvedCommentsDropdown] = useScopeValue< - ReviewPanel.Value<'refreshResolvedCommentsDropdown'> + ReviewPanel.UpdaterFn<'refreshResolvedCommentsDropdown'> >('refreshResolvedCommentsDropdown') const [acceptChanges] = - useScopeValue>('acceptChanges') + useScopeValue>('acceptChanges') const [rejectChanges] = - useScopeValue>('rejectChanges') + useScopeValue>('rejectChanges') const [bulkAcceptActions] = - useScopeValue>('bulkAcceptActions') + useScopeValue>( + 'bulkAcceptActions' + ) const [bulkRejectActions] = - useScopeValue>('bulkRejectActions') + useScopeValue>( + '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, diff --git a/services/web/frontend/js/features/source-editor/context/review-panel/types/review-panel-state.ts b/services/web/frontend/js/features/source-editor/context/review-panel/types/review-panel-state.ts index 30edb4d02f..ca60e019bf 100644 --- a/services/web/frontend/js/features/source-editor/context/review-panel/types/review-panel-state.ts +++ b/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 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 trackChangesOnForEveryone: boolean trackChangesOnForGuests: boolean @@ -57,19 +45,31 @@ export interface ReviewPanelState { name: string } > - toggleReviewPanel: () => void - bulkAcceptActions: () => void - bulkRejectActions: () => void - unresolveComment: (threadId: ThreadId) => void - deleteThread: (_entryId: unknown, docId: DocId, threadId: ThreadId) => void - refreshResolvedCommentsDropdown: () => Promise - acceptChanges: (entryIds: unknown) => void - rejectChanges: (entryIds: unknown) => void - submitNewComment: (content: string) => void } 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 + submitNewComment: (content: string) => void setEntryHover: React.Dispatch>> setIsAddingComment: React.Dispatch< React.SetStateAction> @@ -89,3 +89,7 @@ export interface ReviewPanelState { // Getter for values export type Value = ReviewPanelState['values'][T] + +// Getter for stable functions +export type UpdaterFn = + ReviewPanelState['updaterFns'][T] diff --git a/services/web/frontend/stylesheets/app/editor/review-panel.less b/services/web/frontend/stylesheets/app/editor/review-panel.less index 9976299ab4..d47a057ebb 100644 --- a/services/web/frontend/stylesheets/app/editor/review-panel.less +++ b/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; } } diff --git a/services/web/test/frontend/features/review-panel/unit/compare-props-with-shallow-array-compare.tests.ts b/services/web/test/frontend/features/review-panel/unit/compare-props-with-shallow-array-compare.tests.ts new file mode 100644 index 0000000000..adfe26b301 --- /dev/null +++ b/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('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('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('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('prop1', 'prop2')( + props1, + props2 + ) + ).to.be.true + }) +}) diff --git a/services/web/types/review-panel/entry.ts b/services/web/types/review-panel/entry.ts index 189d863afe..5a21de64aa 100644 --- a/services/web/types/review-panel/entry.ts +++ b/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