Просмотр исходного кода

Improve review panel entry performance (#25402)

GitOrigin-RevId: 2a6ec8ad432195c6069bb58be37dd93341533817
Alf Eaton 1 год назад
Родитель
Сommit
bd67b4ca13

+ 29 - 0
services/web/frontend/js/features/review-panel-new/components/review-panel-action-icons.tsx

@@ -0,0 +1,29 @@
+import { memo } from 'react'
+import MaterialIcon from '@/shared/components/material-icon'
+
+export const AddIcon = memo(function AddIcon() {
+  return (
+    <MaterialIcon
+      className="review-panel-entry-icon review-panel-entry-change-icon review-panel-entry-icon-accept"
+      type="add_circle"
+    />
+  )
+})
+
+export const DeleteIcon = memo(function DeleteIcon() {
+  return (
+    <MaterialIcon
+      className="review-panel-entry-icon review-panel-entry-change-icon review-panel-entry-icon-reject"
+      type="delete"
+    />
+  )
+})
+
+export const EditIcon = memo(function EditIcon() {
+  return (
+    <MaterialIcon
+      className="review-panel-entry-icon review-panel-entry-change-icon review-panel-entry-icon-changed"
+      type="edit"
+    />
+  )
+})

+ 36 - 0
services/web/frontend/js/features/review-panel-new/components/review-panel-change-action.tsx

@@ -0,0 +1,36 @@
+import Tooltip from '@/features/ui/components/bootstrap-5/tooltip'
+import { ComponentProps, memo, MouseEventHandler } from 'react'
+import { PreventSelectingEntry } from '@/features/review-panel-new/components/review-panel-prevent-selecting'
+import OLTooltip from '@/features/ui/components/ol/ol-tooltip'
+import MaterialIcon from '@/shared/components/material-icon'
+
+const changeActionTooltipProps: Partial<ComponentProps<typeof Tooltip>> = {
+  overlayProps: { placement: 'bottom' },
+  tooltipProps: { className: 'review-panel-tooltip' },
+}
+
+export const ChangeAction = memo<{
+  id: string
+  label: string
+  type: string
+  handleClick: MouseEventHandler<HTMLButtonElement>
+}>(function ChangeAction({ id, label, type, handleClick }) {
+  return (
+    <PreventSelectingEntry>
+      <OLTooltip id={id} description={label} {...changeActionTooltipProps}>
+        <button
+          type="button"
+          className="btn"
+          onClick={handleClick}
+          tabIndex={0}
+        >
+          <MaterialIcon
+            type={type}
+            className="review-panel-entry-actions-icon"
+            accessibilityLabel={label}
+          />
+        </button>
+      </OLTooltip>
+    </PreventSelectingEntry>
+  )
+})

+ 61 - 78
services/web/frontend/js/features/review-panel-new/components/review-panel-change.tsx

@@ -1,4 +1,4 @@
-import { memo, useCallback, useState } from 'react'
+import { memo, useCallback, useMemo, useState } from 'react'
 import { useRangesActionsContext } from '../context/ranges-context'
 import {
   Change,
@@ -8,8 +8,6 @@ import {
 import { useTranslation } from 'react-i18next'
 import classnames from 'classnames'
 import { usePermissionsContext } from '@/features/ide-react/context/permissions-context'
-import OLTooltip from '@/features/ui/components/ol/ol-tooltip'
-import MaterialIcon from '@/shared/components/material-icon'
 import { FormatTimeBasedOnYear } from '@/shared/components/format-time-based-on-year'
 import { useChangesUsersContext } from '../context/changes-users-context'
 import { ReviewPanelChangeUser } from './review-panel-change-user'
@@ -17,7 +15,12 @@ import { ReviewPanelEntry } from './review-panel-entry'
 import { useModalsContext } from '@/features/ide-react/context/modals-context'
 import { ExpandableContent } from './review-panel-expandable-content'
 import { useUserContext } from '@/shared/context/user-context'
-import { PreventSelectingEntry } from './review-panel-prevent-selecting'
+import { ChangeAction } from '@/features/review-panel-new/components/review-panel-change-action'
+import {
+  AddIcon,
+  DeleteIcon,
+  EditIcon,
+} from '@/features/review-panel-new/components/review-panel-action-icons'
 
 export const ReviewPanelChange = memo<{
   change: Change<EditOperation>
@@ -27,8 +30,8 @@ export const ReviewPanelChange = memo<{
   docId: string
   hoverRanges?: boolean
   hovered?: boolean
-  onEnter?: (changeId: string) => void
-  onLeave?: (changeId: string) => void
+  handleEnter?: (changeId: string) => void
+  handleLeave?: () => void
 }>(
   ({
     change,
@@ -38,8 +41,8 @@ export const ReviewPanelChange = memo<{
     hoverRanges,
     editable = true,
     hovered,
-    onEnter,
-    onLeave,
+    handleEnter,
+    handleLeave,
   }) => {
     const { t } = useTranslation()
     const { acceptChanges, rejectChanges } = useRangesActionsContext()
@@ -68,6 +71,34 @@ export const ReviewPanelChange = memo<{
       }
     }, [acceptChanges, aggregate, change.id, showGenericMessageModal, t])
 
+    const rejectHandler = useCallback(async () => {
+      if (aggregate) {
+        await rejectChanges(change.id, aggregate.id)
+      } else {
+        await rejectChanges(change.id)
+      }
+    }, [aggregate, change, rejectChanges])
+
+    const translations = useMemo(
+      () => ({
+        accept_change: t('accept_change'),
+        reject_change: t('reject_change'),
+        aggregate_changed: t('aggregate_changed'),
+        aggregate_to: t('aggregate_to'),
+        tracked_change_added: t('tracked_change_added'),
+        tracked_change_deleted: t('tracked_change_deleted'),
+      }),
+      [t]
+    )
+
+    const { handleMouseEnter, handleMouseLeave } = useMemo(
+      () => ({
+        handleMouseEnter: handleEnter && (() => handleEnter(change.id)),
+        handleMouseLeave: handleLeave && (() => handleLeave()),
+      }),
+      [change.id, handleEnter, handleLeave]
+    )
+
     if (!changesUsers) {
       // if users are not loaded yet, do not show "Unknown" user
       return null
@@ -90,14 +121,14 @@ export const ReviewPanelChange = memo<{
         docId={docId}
         hoverRanges={hoverRanges}
         disabled={accepting}
-        onEnterEntryIndicator={onEnter && (() => onEnter(change.id))}
-        onLeaveEntryIndicator={onLeave && (() => onLeave(change.id))}
+        handleEnter={handleMouseEnter}
+        handleLeave={handleMouseLeave}
         entryIndicator="edit"
       >
         <div
           className="review-panel-entry-content"
-          onMouseEnter={onEnter && (() => onEnter(change.id))}
-          onMouseLeave={onLeave && (() => onLeave(change.id))}
+          onMouseEnter={handleMouseEnter}
+          onMouseLeave={handleMouseLeave}
         >
           <div className="review-panel-entry-header">
             <div>
@@ -111,56 +142,22 @@ export const ReviewPanelChange = memo<{
             {editable && (
               <div className="review-panel-entry-actions">
                 {permissions.write && (
-                  <PreventSelectingEntry>
-                    <OLTooltip
-                      id="accept-change"
-                      overlayProps={{ placement: 'bottom' }}
-                      description={t('accept_change')}
-                      tooltipProps={{ className: 'review-panel-tooltip' }}
-                    >
-                      <button
-                        type="button"
-                        className="btn"
-                        onClick={acceptHandler}
-                        tabIndex={0}
-                      >
-                        <MaterialIcon
-                          type="check"
-                          className="review-panel-entry-actions-icon"
-                          accessibilityLabel={t('accept_change')}
-                        />
-                      </button>
-                    </OLTooltip>
-                  </PreventSelectingEntry>
+                  <ChangeAction
+                    id="accept-change"
+                    label={translations.accept_change}
+                    type="check"
+                    handleClick={acceptHandler}
+                  />
                 )}
 
                 {(permissions.write ||
                   (permissions.trackedWrite && isChangeAuthor)) && (
-                  <PreventSelectingEntry>
-                    <OLTooltip
-                      id="reject-change"
-                      description={t('reject_change')}
-                      overlayProps={{ placement: 'bottom' }}
-                      tooltipProps={{ className: 'review-panel-tooltip' }}
-                    >
-                      <button
-                        tabIndex={0}
-                        type="button"
-                        className="btn"
-                        onClick={() =>
-                          aggregate
-                            ? rejectChanges(change.id, aggregate.id)
-                            : rejectChanges(change.id)
-                        }
-                      >
-                        <MaterialIcon
-                          className="review-panel-entry-actions-icon"
-                          accessibilityLabel={t('reject_change')}
-                          type="close"
-                        />
-                      </button>
-                    </OLTooltip>
-                  </PreventSelectingEntry>
+                  <ChangeAction
+                    id="reject-change"
+                    label={translations.reject_change}
+                    type="close"
+                    handleClick={rejectHandler}
+                  />
                 )}
               </div>
             )}
@@ -169,21 +166,11 @@ export const ReviewPanelChange = memo<{
           <div className="review-panel-change-body">
             {'i' in change.op && (
               <>
-                {aggregateChange ? (
-                  <MaterialIcon
-                    className="review-panel-entry-icon review-panel-entry-change-icon review-panel-entry-icon-changed"
-                    type="edit"
-                  />
-                ) : (
-                  <MaterialIcon
-                    className="review-panel-entry-icon review-panel-entry-change-icon review-panel-entry-icon-accept"
-                    type="add_circle"
-                  />
-                )}
+                {aggregateChange ? <EditIcon /> : <AddIcon />}
 
                 {aggregateChange ? (
                   <span>
-                    {t('aggregate_changed')}:{' '}
+                    {translations.aggregate_changed}:{' '}
                     <del className="review-panel-content-highlight">
                       <ExpandableContent
                         inline
@@ -191,7 +178,7 @@ export const ReviewPanelChange = memo<{
                         checkNewLines={false}
                       />
                     </del>{' '}
-                    {t('aggregate_to')}{' '}
+                    {translations.aggregate_to}{' '}
                     <ExpandableContent
                       inline
                       content={change.op.i}
@@ -200,7 +187,7 @@ export const ReviewPanelChange = memo<{
                   </span>
                 ) : (
                   <span>
-                    {t('tracked_change_added')}:&nbsp;
+                    {translations.tracked_change_added}:&nbsp;
                     <ins className="review-panel-content-highlight">
                       <ExpandableContent
                         content={change.op.i}
@@ -214,13 +201,9 @@ export const ReviewPanelChange = memo<{
 
             {'d' in change.op && (
               <>
-                <MaterialIcon
-                  className="review-panel-entry-icon review-panel-entry-change-icon review-panel-entry-icon-reject"
-                  type="delete"
-                />
-
+                <DeleteIcon />
                 <span>
-                  {t('tracked_change_deleted')}:&nbsp;
+                  {translations.tracked_change_deleted}:&nbsp;
                   <del className="review-panel-content-highlight">
                     <ExpandableContent
                       content={change.op.d}

+ 4 - 4
services/web/frontend/js/features/review-panel-new/components/review-panel-comment-content.tsx

@@ -21,8 +21,8 @@ export const ReviewPanelCommentContent = memo<{
   onDeleteMessage?: (commentId: CommentId) => Promise<void>
   onDeleteThread?: (threadId: ThreadId) => Promise<void>
   onResolve?: () => Promise<void>
-  onLeave?: (changeId: string) => void
-  onEnter?: (changeId: string) => void
+  onLeave?: () => void
+  onEnter?: () => void
 }>(
   ({
     comment,
@@ -69,8 +69,8 @@ export const ReviewPanelCommentContent = memo<{
     return (
       <div
         className="review-panel-entry-content"
-        onMouseEnter={onEnter && (() => onEnter(comment.id))}
-        onMouseLeave={onLeave && (() => onLeave(comment.id))}
+        onMouseEnter={onEnter}
+        onMouseLeave={onLeave}
       >
         {thread.messages.map((message, i) => {
           const isReply = i !== 0

+ 147 - 138
services/web/frontend/js/features/review-panel-new/components/review-panel-comment.tsx

@@ -1,4 +1,4 @@
-import { memo, useCallback, useState } from 'react'
+import { memo, useCallback, useMemo, useState } from 'react'
 import { Change, CommentOperation } from '../../../../../types/change'
 import {
   useThreadsActionsContext,
@@ -21,160 +21,169 @@ export const ReviewPanelComment = memo<{
   docId: string
   top?: number
   hoverRanges?: boolean
-  onEnter?: (changeId: string) => void
-  onLeave?: (changeId: string) => void
+  handleEnter?: (changeId: string) => void
+  handleLeave?: (changeId: string) => void
   hovered?: boolean
-}>(({ comment, top, hovered, onEnter, onLeave, docId, hoverRanges }) => {
-  const threads = useThreadsContext()
-  const {
-    resolveThread,
-    editMessage,
-    deleteMessage,
-    deleteOwnMessage,
-    deleteThread,
-    addMessage,
-  } = useThreadsActionsContext()
-  const { showGenericMessageModal } = useModalsContext()
-  const { t } = useTranslation()
-  const permissions = usePermissionsContext()
+}>(
+  ({ comment, top, hovered, handleEnter, handleLeave, docId, hoverRanges }) => {
+    const threads = useThreadsContext()
+    const {
+      resolveThread,
+      editMessage,
+      deleteMessage,
+      deleteOwnMessage,
+      deleteThread,
+      addMessage,
+    } = useThreadsActionsContext()
+    const { showGenericMessageModal } = useModalsContext()
+    const { t } = useTranslation()
+    const permissions = usePermissionsContext()
 
-  const [processing, setProcessing] = useState(false)
+    const [processing, setProcessing] = useState(false)
 
-  const handleResolveComment = useCallback(async () => {
-    setProcessing(true)
-    try {
-      await resolveThread(comment.op.t)
-    } catch (err) {
-      debugConsole.error(err)
-      showGenericMessageModal(
-        t('resolve_comment_error_title'),
-        t('resolve_comment_error_message')
-      )
-    } finally {
-      setProcessing(false)
-    }
-  }, [comment.op.t, resolveThread, showGenericMessageModal, t])
-
-  const handleEditMessage = useCallback(
-    async (commentId: CommentId, content: string) => {
+    const handleResolveComment = useCallback(async () => {
       setProcessing(true)
       try {
-        await editMessage(comment.op.t, commentId, content)
+        await resolveThread(comment.op.t)
       } catch (err) {
         debugConsole.error(err)
         showGenericMessageModal(
-          t('edit_comment_error_title'),
-          t('edit_comment_error_message')
+          t('resolve_comment_error_title'),
+          t('resolve_comment_error_message')
         )
       } finally {
         setProcessing(false)
       }
-    },
-    [comment.op.t, editMessage, showGenericMessageModal, t]
-  )
+    }, [comment.op.t, resolveThread, showGenericMessageModal, t])
 
-  const handleDeleteMessage = useCallback(
-    async (commentId: CommentId) => {
-      setProcessing(true)
-      try {
-        if (permissions.resolveAllComments) {
-          // Owners and editors can delete any message
-          await deleteMessage(comment.op.t, commentId)
-        } else if (permissions.resolveOwnComments) {
-          // Reviewers can only delete their own messages
-          await deleteOwnMessage(comment.op.t, commentId)
+    const handleEditMessage = useCallback(
+      async (commentId: CommentId, content: string) => {
+        setProcessing(true)
+        try {
+          await editMessage(comment.op.t, commentId, content)
+        } catch (err) {
+          debugConsole.error(err)
+          showGenericMessageModal(
+            t('edit_comment_error_title'),
+            t('edit_comment_error_message')
+          )
+        } finally {
+          setProcessing(false)
         }
-      } catch (err) {
-        debugConsole.error(err)
-        showGenericMessageModal(
-          t('delete_comment_error_title'),
-          t('delete_comment_error_message')
-        )
-      } finally {
-        setProcessing(false)
-      }
-    },
-    [
-      comment.op.t,
-      deleteMessage,
-      deleteOwnMessage,
-      showGenericMessageModal,
-      t,
-      permissions.resolveOwnComments,
-      permissions.resolveAllComments,
-    ]
-  )
+      },
+      [comment.op.t, editMessage, showGenericMessageModal, t]
+    )
 
-  const handleDeleteThread = useCallback(
-    async (commentId: ThreadId) => {
-      setProcessing(true)
-      try {
-        await deleteThread(commentId)
-      } catch (err) {
-        debugConsole.error(err)
-        showGenericMessageModal(
-          t('delete_comment_error_title'),
-          t('delete_comment_error_message')
-        )
-      } finally {
-        setProcessing(false)
-      }
-    },
-    [deleteThread, showGenericMessageModal, t]
-  )
+    const handleDeleteMessage = useCallback(
+      async (commentId: CommentId) => {
+        setProcessing(true)
+        try {
+          if (permissions.resolveAllComments) {
+            // Owners and editors can delete any message
+            await deleteMessage(comment.op.t, commentId)
+          } else if (permissions.resolveOwnComments) {
+            // Reviewers can only delete their own messages
+            await deleteOwnMessage(comment.op.t, commentId)
+          }
+        } catch (err) {
+          debugConsole.error(err)
+          showGenericMessageModal(
+            t('delete_comment_error_title'),
+            t('delete_comment_error_message')
+          )
+        } finally {
+          setProcessing(false)
+        }
+      },
+      [
+        comment.op.t,
+        deleteMessage,
+        deleteOwnMessage,
+        showGenericMessageModal,
+        t,
+        permissions.resolveOwnComments,
+        permissions.resolveAllComments,
+      ]
+    )
 
-  const handleSubmitReply = useCallback(
-    async (content: string) => {
-      setProcessing(true)
-      try {
-        await addMessage(comment.op.t, content)
-      } catch (err) {
-        debugConsole.error(err)
-        showGenericMessageModal(
-          t('add_comment_error_title'),
-          t('add_comment_error_message')
-        )
-        throw err
-      } finally {
-        setProcessing(false)
+    const handleDeleteThread = useCallback(
+      async (commentId: ThreadId) => {
+        setProcessing(true)
+        try {
+          await deleteThread(commentId)
+        } catch (err) {
+          debugConsole.error(err)
+          showGenericMessageModal(
+            t('delete_comment_error_title'),
+            t('delete_comment_error_message')
+          )
+        } finally {
+          setProcessing(false)
+        }
+      },
+      [deleteThread, showGenericMessageModal, t]
+    )
+
+    const handleSubmitReply = useCallback(
+      async (content: string) => {
+        setProcessing(true)
+        try {
+          await addMessage(comment.op.t, content)
+        } catch (err) {
+          debugConsole.error(err)
+          showGenericMessageModal(
+            t('add_comment_error_title'),
+            t('add_comment_error_message')
+          )
+          throw err
+        } finally {
+          setProcessing(false)
+        }
+      },
+      [addMessage, comment.op.t, showGenericMessageModal, t]
+    )
+
+    const { handleMouseEnter, handleMouseLeave } = useMemo(() => {
+      return {
+        handleMouseEnter: handleEnter && (() => handleEnter(comment.id)),
+        handleMouseLeave: handleLeave && (() => handleLeave(comment.id)),
       }
-    },
-    [addMessage, comment.op.t, showGenericMessageModal, t]
-  )
+    }, [comment.id, handleEnter, handleLeave])
 
-  const thread = threads?.[comment.op.t]
-  if (!thread || thread.resolved || thread.messages.length === 0) {
-    return null
-  }
+    const thread = threads?.[comment.op.t]
+    if (!thread || thread.resolved || thread.messages.length === 0) {
+      return null
+    }
 
-  return (
-    <ReviewPanelEntry
-      className={classnames('review-panel-entry-comment', {
-        'review-panel-entry-loaded': !!threads?.[comment.op.t],
-        'review-panel-entry-hover': hovered,
-      })}
-      docId={docId}
-      top={top}
-      op={comment.op}
-      position={comment.op.p}
-      hoverRanges={hoverRanges}
-      disabled={processing}
-      onEnterEntryIndicator={onEnter && (() => onEnter(comment.id))}
-      onLeaveEntryIndicator={onLeave && (() => onLeave(comment.id))}
-      entryIndicator="comment"
-    >
-      <ReviewPanelCommentContent
-        comment={comment}
-        isResolved={false}
-        onLeave={onLeave}
-        onEnter={onEnter}
-        onResolve={handleResolveComment}
-        onEdit={handleEditMessage}
-        onDeleteMessage={handleDeleteMessage}
-        onDeleteThread={handleDeleteThread}
-        onReply={handleSubmitReply}
-      />
-    </ReviewPanelEntry>
-  )
-})
+    return (
+      <ReviewPanelEntry
+        className={classnames('review-panel-entry-comment', {
+          'review-panel-entry-loaded': !!threads?.[comment.op.t],
+          'review-panel-entry-hover': hovered,
+        })}
+        docId={docId}
+        top={top}
+        op={comment.op}
+        position={comment.op.p}
+        hoverRanges={hoverRanges}
+        disabled={processing}
+        handleEnter={handleMouseEnter}
+        handleLeave={handleMouseLeave}
+        entryIndicator="comment"
+      >
+        <ReviewPanelCommentContent
+          comment={comment}
+          isResolved={false}
+          onLeave={handleMouseLeave}
+          onEnter={handleMouseEnter}
+          onResolve={handleResolveComment}
+          onEdit={handleEditMessage}
+          onDeleteMessage={handleDeleteMessage}
+          onDeleteThread={handleDeleteThread}
+          onReply={handleSubmitReply}
+        />
+      </ReviewPanelEntry>
+    )
+  }
+)
 ReviewPanelComment.displayName = 'ReviewPanelComment'

+ 5 - 5
services/web/frontend/js/features/review-panel-new/components/review-panel-current-file.tsx

@@ -53,7 +53,7 @@ const ReviewPanelCurrentFile: FC = () => {
     setHoveredEntry(id)
   }, [])
 
-  const handleEntryLeave = useCallback((id: string) => {
+  const handleEntryLeave = useCallback(() => {
     clearTimeout(hoverTimeout.current)
     hoverTimeout.current = window.setTimeout(() => {
       setHoveredEntry(null)
@@ -333,8 +333,8 @@ const ReviewPanelCurrentFile: FC = () => {
                 top={positions.get(change.id)}
                 aggregate={aggregatedRanges.aggregates.get(change.id)}
                 hovered={hoveredEntry === change.id}
-                onEnter={handleEntryEnter}
-                onLeave={handleEntryLeave}
+                handleEnter={handleEntryEnter}
+                handleLeave={handleEntryLeave}
               />
             )
         )}
@@ -348,8 +348,8 @@ const ReviewPanelCurrentFile: FC = () => {
                 comment={comment}
                 top={positions.get(comment.id)}
                 hovered={hoveredEntry === comment.id}
-                onEnter={handleEntryEnter}
-                onLeave={handleEntryLeave}
+                handleEnter={handleEntryEnter}
+                handleLeave={handleEntryLeave}
               />
             )
         )}

+ 27 - 0
services/web/frontend/js/features/review-panel-new/components/review-panel-entry-indicator.tsx

@@ -0,0 +1,27 @@
+import { memo, MouseEventHandler } from 'react'
+import MaterialIcon from '@/shared/components/material-icon'
+
+export const EntryIndicator = memo<{
+  handleMouseEnter?: MouseEventHandler<HTMLDivElement>
+  handleMouseLeave?: MouseEventHandler<HTMLDivElement>
+  handleMouseDown?: MouseEventHandler<HTMLDivElement>
+  type: string
+}>(function EntryIndicator({
+  handleMouseEnter,
+  handleMouseLeave,
+  handleMouseDown,
+  type,
+}) {
+  return (
+    <div
+      className="review-panel-entry-indicator"
+      onMouseEnter={handleMouseEnter}
+      onMouseLeave={handleMouseLeave}
+      onMouseDown={handleMouseDown} // Using onMouseDown rather than onClick to guarantee that it fires before onFocus
+      role="button"
+      tabIndex={0}
+    >
+      <MaterialIcon type={type} className="review-panel-entry-icon" />
+    </div>
+  )
+})

+ 11 - 18
services/web/frontend/js/features/review-panel-new/components/review-panel-entry.tsx

@@ -12,9 +12,9 @@ import {
 } from '@/features/source-editor/extensions/ranges'
 import { useEditorManagerContext } from '@/features/ide-react/context/editor-manager-context'
 import { EditorSelection } from '@codemirror/state'
-import MaterialIcon from '@/shared/components/material-icon'
 import { OFFSET_FOR_ENTRIES_ABOVE } from '../utils/position-items'
 import useReviewPanelLayout from '../hooks/use-review-panel-layout'
+import { EntryIndicator } from './review-panel-entry-indicator'
 
 export const ReviewPanelEntry: FC<
   React.PropsWithChildren<{
@@ -26,8 +26,8 @@ export const ReviewPanelEntry: FC<
     selectLineOnFocus?: boolean
     hoverRanges?: boolean
     disabled?: boolean
-    onEnterEntryIndicator?: () => void
-    onLeaveEntryIndicator?: () => void
+    handleEnter?: () => void
+    handleLeave?: () => void
     entryIndicator?: 'comment' | 'edit'
   }>
 > = ({
@@ -40,8 +40,8 @@ export const ReviewPanelEntry: FC<
   docId,
   hoverRanges = true,
   disabled,
-  onEnterEntryIndicator,
-  onLeaveEntryIndicator,
+  handleEnter,
+  handleLeave,
   entryIndicator,
 }) => {
   const state = useCodeMirrorStateContext()
@@ -194,19 +194,12 @@ export const ReviewPanelEntry: FC<
       }}
     >
       {entryIndicator && (
-        <div
-          className="review-panel-entry-indicator"
-          onMouseEnter={onEnterEntryIndicator}
-          onMouseLeave={onLeaveEntryIndicator}
-          onMouseDown={openReviewPanel} // Using onMouseDown rather than onClick to guarantee that it fires before onFocus
-          role="button"
-          tabIndex={0}
-        >
-          <MaterialIcon
-            type={entryIndicator}
-            className="review-panel-entry-icon"
-          />
-        </div>
+        <EntryIndicator
+          type={entryIndicator}
+          handleMouseEnter={handleEnter}
+          handleMouseLeave={handleLeave}
+          handleMouseDown={openReviewPanel}
+        />
       )}
       {children}
     </div>

+ 5 - 5
services/web/frontend/js/features/review-panel-new/components/review-panel-expandable-content.tsx

@@ -1,24 +1,24 @@
-import { FC, useCallback, useRef, useState } from 'react'
+import { memo, useCallback, useRef, useState } from 'react'
 import OLButton from '@/features/ui/components/ol/ol-button'
 import { useTranslation } from 'react-i18next'
 import classNames from 'classnames'
 import { PreventSelectingEntry } from './review-panel-prevent-selecting'
 
-export const ExpandableContent: FC<{
+export const ExpandableContent = memo<{
   className?: string
   content: string
   contentLimit?: number
   newLineCharsLimit?: number
   checkNewLines?: boolean
   inline?: boolean
-}> = ({
+}>(function ExpandableContent({
   content,
   className,
   contentLimit = 50,
   newLineCharsLimit = 3,
   checkNewLines = true,
   inline = false,
-}) => {
+}) {
   const { t } = useTranslation()
   const contentRef = useRef<HTMLDivElement>(null)
   const [isExpanded, setIsExpanded] = useState(false)
@@ -83,7 +83,7 @@ export const ExpandableContent: FC<{
       </div>
     </>
   )
-}
+})
 
 function indexOfNthLine(content: string, n: number) {
   if (n < 1) return null

+ 4 - 4
services/web/frontend/js/features/review-panel-new/components/review-tooltip-menu.tsx

@@ -131,8 +131,8 @@ const ReviewTooltipMenuContent: FC<{ onAddComment: () => void }> = ({
     showGenericConfirmModal({
       message: t('confirm_accept_selected_changes', { count: nChanges }),
       title: t('accept_selected_changes'),
-      onConfirm: () => {
-        acceptChanges(...changeIdsInSelection)
+      onConfirm: async () => {
+        await acceptChanges(...changeIdsInSelection)
       },
       primaryVariant: 'danger',
     })
@@ -153,8 +153,8 @@ const ReviewTooltipMenuContent: FC<{ onAddComment: () => void }> = ({
     showGenericConfirmModal({
       message: t('confirm_reject_selected_changes', { count: nChanges }),
       title: t('reject_selected_changes'),
-      onConfirm: () => {
-        rejectChanges(...changeIdsInSelection)
+      onConfirm: async () => {
+        await rejectChanges(...changeIdsInSelection)
       },
       primaryVariant: 'danger',
     })

+ 3 - 3
services/web/frontend/js/features/review-panel-new/context/ranges-context.tsx

@@ -32,8 +32,8 @@ export type Ranges = {
 export const RangesContext = createContext<Ranges | undefined>(undefined)
 
 type RangesActions = {
-  acceptChanges: (...ids: string[]) => void
-  rejectChanges: (...ids: string[]) => void
+  acceptChanges: (...ids: string[]) => Promise<void>
+  rejectChanges: (...ids: string[]) => Promise<void>
 }
 
 const buildRanges = (currentDocument: DocumentContainer | null) => {
@@ -166,7 +166,7 @@ export const RangesProvider: FC<React.PropsWithChildren> = ({ children }) => {
           setRanges(buildRanges(currentDocument))
         }
       },
-      rejectChanges(...ids: string[]) {
+      async rejectChanges(...ids: string[]) {
         if (currentDocument?.ranges) {
           view.dispatch(rejectChanges(view.state, currentDocument.ranges, ids))
         }

+ 13 - 11
services/web/frontend/js/features/review-panel-new/utils/position-items.ts

@@ -44,37 +44,39 @@ export const positionItems = debounce(
 
     const activeItemTop = getTopPosition(activeItem, activeItemIndex === 0)
 
-    activeItem.style.top = `${activeItemTop}px`
-    activeItem.style.visibility = 'visible'
-    const focusedItemRect = activeItem.getBoundingClientRect()
+    const positions: [HTMLElement, number][] = []
+    positions.push([activeItem, activeItemTop])
 
     // above the active item
     let topLimit = activeItemTop
     for (let i = activeItemIndex - 1; i >= 0; i--) {
       const item = items[i]
-      const rect = item.getBoundingClientRect()
+      const height = item.offsetHeight
       let top = getTopPosition(item, i === 0)
-      const bottom = top + rect.height
+      const bottom = top + height
       if (bottom > topLimit) {
-        top = topLimit - rect.height - GAP_BETWEEN_ENTRIES
+        top = topLimit - height - GAP_BETWEEN_ENTRIES
       }
-      item.style.top = `${top}px`
-      item.style.visibility = 'visible'
+      positions.push([item, top])
       topLimit = top
     }
 
     // below the active item
-    let bottomLimit = activeItemTop + focusedItemRect.height
+    let bottomLimit = activeItemTop + activeItem.offsetHeight
     for (let i = activeItemIndex + 1; i < items.length; i++) {
       const item = items[i]
-      const rect = item.getBoundingClientRect()
+      const height = item.offsetHeight
       let top = getTopPosition(item, false)
       if (top < bottomLimit) {
         top = bottomLimit + GAP_BETWEEN_ENTRIES
       }
+      positions.push([item, top])
+      bottomLimit = top + height
+    }
+
+    for (const [item, top] of positions) {
       item.style.top = `${top}px`
       item.style.visibility = 'visible'
-      bottomLimit = top + rect.height
     }
 
     return {