Merge pull request #21283 from overleaf/dp-keyboard-shortcuts

Add missing keyboard shortcuts to new review panel

GitOrigin-RevId: 78e3a63284b62c90e8a3803bd81fdf273f1a2ec9

services/web/frontend/js/features/review-panel-new/components/review-panel-track-changes-menu.tsx

@@ -1,12 +1,13 @@
-import { FC, useCallback } from 'react'
+import { FC } from 'react'
 import TrackChangesToggle from '@/features/source-editor/components/review-panel/toolbar/track-changes-toggle'
 import { useProjectContext } from '@/shared/context/project-context'
 import { usePermissionsContext } from '@/features/ide-react/context/permissions-context'
 import { useTranslation } from 'react-i18next'
-import { useTrackChangesStateContext } from '../context/track-changes-state-context'
+import {
-import { postJSON } from '@/infrastructure/fetch-json'
+  useTrackChangesStateActionsContext,
+  useTrackChangesStateContext,
+} from '../context/track-changes-state-context'
 import { useChangesUsersContext } from '../context/changes-users-context'
-import { UserId } from '../../../../../types/user'
 import { buildName } from '../utils/build-name'
 
 export const ReviewPanelTrackChangesMenu: FC = () => {
@@ -14,34 +15,14 @@ export const ReviewPanelTrackChangesMenu: FC = () => {
   const permissions = usePermissionsContext()
   const project = useProjectContext()
   const trackChanges = useTrackChangesStateContext()
+  const { saveTrackChanges } = useTrackChangesStateActionsContext()
   const changesUsers = useChangesUsersContext()
 
-  const saveTrackChanges = useCallback(
-    body => {
-      postJSON(`/project/${project._id}/track_changes`, {
-        body,
-      })
-    },
-    [project._id]
-  )
-
   if (trackChanges === undefined || !changesUsers) {
     return null
   }
 
-  const trackChangesIsObject = trackChanges !== true && trackChanges !== false
+  const { onForEveryone, onForGuests, onForMembers } = trackChanges
-  const onForEveryone = trackChanges === true
-  const onForGuests =
-    onForEveryone || (trackChangesIsObject && trackChanges.__guests__ === true)
-
-  const trackChangesValues: Record<UserId, boolean | undefined> = {}
-  if (trackChangesIsObject) {
-    for (const key of Object.keys(trackChanges)) {
-      if (key !== '__guests__') {
-        trackChangesValues[key as UserId] = trackChanges[key as UserId]
-      }
-    }
-  }
 
   const canToggle = project.features.trackChanges && permissions.write
 
@@ -65,8 +46,7 @@ export const ReviewPanelTrackChangesMenu: FC = () => {
         const user = changesUsers.get(member._id) ?? member
         const name = buildName(user)
 
-        const value =
+        const value = onForEveryone || onForMembers[member._id] === true
-          trackChanges === true || trackChangesValues[member._id] === true
 
         return (
           <div key={member._id} className="rp-tc-state-item">
@@ -78,7 +58,7 @@ export const ReviewPanelTrackChangesMenu: FC = () => {
               handleToggle={() => {
                 saveTrackChanges({
                   on_for: {
-                    ...trackChangesValues,
+                    ...onForMembers,
                     [member._id]: !value,
                   },
                   on_for_guests: onForGuests,
@@ -99,7 +79,7 @@ export const ReviewPanelTrackChangesMenu: FC = () => {
           description={t('track_changes_for_guests')}
           handleToggle={() =>
             saveTrackChanges({
-              on_for: trackChangesValues,
+              on_for: onForMembers,
               on_for_guests: !onForGuests,
             })
           }

services/web/frontend/js/features/review-panel-new/context/track-changes-state-context.tsx

@@ -1,43 +1,152 @@
 import { UserId } from '../../../../../types/user'
-import { createContext, FC, useContext, useEffect, useState } from 'react'
+import {
+  createContext,
+  FC,
+  useCallback,
+  useContext,
+  useEffect,
+  useMemo,
+  useState,
+} from 'react'
 import useSocketListener from '@/features/ide-react/hooks/use-socket-listener'
 import { useConnectionContext } from '@/features/ide-react/context/connection-context'
 import { useProjectContext } from '@/shared/context/project-context'
 import { useEditorManagerContext } from '@/features/ide-react/context/editor-manager-context'
 import { useUserContext } from '@/shared/context/user-context'
+import { postJSON } from '@/infrastructure/fetch-json'
+import useEventListener from '@/shared/hooks/use-event-listener'
+import { ProjectContextValue } from '@/shared/context/types/project-context'
+import { usePermissionsContext } from '@/features/ide-react/context/permissions-context'
 
-export type TrackChangesState = boolean | Record<UserId | '__guests__', boolean>
+export type TrackChangesState = {
+  onForEveryone: boolean
+  onForGuests: boolean
+  onForMembers: Record<UserId, boolean | undefined>
+}
 
 export const TrackChangesStateContext = createContext<
   TrackChangesState | undefined
 >(undefined)
 
+type SaveTrackChangesRequestBody = {
+  on?: boolean
+  on_for?: Record<UserId, boolean | undefined>
+  on_for_guests?: boolean
+}
+
+type TrackChangesStateActions = {
+  saveTrackChanges: (trackChangesBody: SaveTrackChangesRequestBody) => void
+}
+
+const TrackChangesStateActionsContext = createContext<
+  TrackChangesStateActions | undefined
+>(undefined)
+
 export const TrackChangesStateProvider: FC = ({ children }) => {
+  const permissions = usePermissionsContext()
   const { socket } = useConnectionContext()
   const project = useProjectContext()
   const user = useUserContext()
   const { setWantTrackChanges } = useEditorManagerContext()
 
   // TODO: update project.trackChangesState instead?
-  const [value, setValue] = useState<TrackChangesState>(
+  const [trackChangesValue, setTrackChangesValue] = useState<
-    project.trackChangesState ?? false
+    ProjectContextValue['trackChangesState']
-  )
+  >(project.trackChangesState ?? false)
 
-  useSocketListener(socket, 'toggle-track-changes', setValue)
+  useSocketListener(socket, 'toggle-track-changes', setTrackChangesValue)
 
   useEffect(() => {
     setWantTrackChanges(
-      value === true || (value !== false && value[user.id ?? '__guests__'])
+      trackChangesValue === true ||
+        (trackChangesValue !== false &&
+          trackChangesValue[user.id ?? '__guests__'])
+    )
+  }, [setWantTrackChanges, trackChangesValue, user.id])
+
+  const actions = useMemo(
+    () => ({
+      async saveTrackChanges(trackChangesBody: SaveTrackChangesRequestBody) {
+        postJSON(`/project/${project._id}/track_changes`, {
+          body: trackChangesBody,
+        })
+      },
+    }),
+    [project._id]
+  )
+
+  const trackChangesIsObject =
+    trackChangesValue !== true && trackChangesValue !== false
+  const onForEveryone = trackChangesValue === true
+  const onForGuests =
+    onForEveryone ||
+    (trackChangesIsObject && trackChangesValue.__guests__ === true)
+
+  const onForMembers = useMemo(() => {
+    const onForMembers: Record<UserId, boolean | undefined> = {}
+    if (trackChangesIsObject) {
+      for (const key of Object.keys(trackChangesValue)) {
+        if (key !== '__guests__') {
+          onForMembers[key as UserId] = trackChangesValue[key as UserId]
+        }
+      }
+    }
+    return onForMembers
+  }, [trackChangesIsObject, trackChangesValue])
+
+  useEventListener(
+    'toggle-track-changes',
+    useCallback(() => {
+      if (
+        user.id &&
+        project.features.trackChanges &&
+        permissions.write &&
+        !onForEveryone
+      ) {
+        const value = onForMembers[user.id]
+        actions.saveTrackChanges({
+          on_for: {
+            ...onForMembers,
+            [user.id]: !value,
+          },
+          on_for_guests: onForGuests,
+        })
+      }
+    }, [
+      actions,
+      onForMembers,
+      onForGuests,
+      onForEveryone,
+      permissions.write,
+      project.features.trackChanges,
+      user.id,
+    ])
+  )
+
+  const value = useMemo(
+    () => ({ onForEveryone, onForGuests, onForMembers }),
+    [onForEveryone, onForGuests, onForMembers]
   )
-  }, [setWantTrackChanges, value, user.id])
 
   return (
+    <TrackChangesStateActionsContext.Provider value={actions}>
       <TrackChangesStateContext.Provider value={value}>
         {children}
       </TrackChangesStateContext.Provider>
+    </TrackChangesStateActionsContext.Provider>
   )
 }
 
 export const useTrackChangesStateContext = () => {
   return useContext(TrackChangesStateContext)
 }
+
+export const useTrackChangesStateActionsContext = () => {
+  const context = useContext(TrackChangesStateActionsContext)
+  if (!context) {
+    throw new Error(
+      'useTrackChangesStateActionsContext is only available inside TrackChangesStateProvider'
+    )
+  }
+  return context
+}

services/web/frontend/js/features/source-editor/extensions/shortcuts.ts

@@ -26,7 +26,12 @@ import {
 import { isSplitTestEnabled } from '@/utils/splitTestUtils'
 
 const toggleReviewPanel = () => {
+  if (isSplitTestEnabled('review-panel-redesign')) {
+    window.dispatchEvent(new Event('ui.toggle-review-panel'))
+  } else {
     dispatchEditorEvent('toggle-review-panel')
+  }
+
   return true
 }
 
@@ -40,7 +45,11 @@ const addNewCommentFromKbdShortcut = (view: EditorView) => {
 }
 
 const toggleTrackChangesFromKbdShortcut = () => {
+  if (isSplitTestEnabled('review-panel-redesign')) {
+    window.dispatchEvent(new Event('toggle-track-changes'))
+  } else {
     dispatchEditorEvent('toggle-track-changes')
+  }
   return true
 }
 

services/web/frontend/js/shared/context/layout-context.tsx

@@ -97,7 +97,7 @@ export const LayoutProvider: FC = ({ children }) => {
 
   // whether the review pane is open
   const [reviewPanelOpen, setReviewPanelOpen] =
-    useScopeValue('ui.reviewPanelOpen')
+    useScopeValue<boolean>('ui.reviewPanelOpen')
 
   // whether the review pane is collapsed
   const [miniReviewPanelVisible, setMiniReviewPanelVisible] =
@@ -117,6 +117,13 @@ export const LayoutProvider: FC = ({ children }) => {
     )
   )
 
+  useEventListener(
+    'ui.toggle-review-panel',
+    useCallback(() => {
+      setReviewPanelOpen(open => !open)
+    }, [setReviewPanelOpen])
+  )
+
   // whether to display the editor and preview side-by-side or full-width ("flat")
   const [pdfLayout, setPdfLayout] = useScopeValue<IdeLayout>('ui.pdfLayout')