From d74981775cebf852e3c6fe5c688ba438ed69401c Mon Sep 17 00:00:00 2001 From: David <33458145+davidmcpowell@users.noreply.github.com> Date: Wed, 23 Oct 2024 11:14:35 +0100 Subject: [PATCH] Merge pull request #21283 from overleaf/dp-keyboard-shortcuts Add missing keyboard shortcuts to new review panel GitOrigin-RevId: 78e3a63284b62c90e8a3803bd81fdf273f1a2ec9 --- .../review-panel-track-changes-menu.tsx | 40 ++---- .../context/track-changes-state-context.tsx | 131 ++++++++++++++++-- .../source-editor/extensions/shortcuts.ts | 13 +- .../js/shared/context/layout-context.tsx | 9 +- 4 files changed, 149 insertions(+), 44 deletions(-) diff --git a/services/web/frontend/js/features/review-panel-new/components/review-panel-track-changes-menu.tsx b/services/web/frontend/js/features/review-panel-new/components/review-panel-track-changes-menu.tsx index c07564782c..57abc3e3b3 100644 --- a/services/web/frontend/js/features/review-panel-new/components/review-panel-track-changes-menu.tsx +++ b/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 { postJSON } from '@/infrastructure/fetch-json' +import { + 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 = trackChanges === true - const onForGuests = - onForEveryone || (trackChangesIsObject && trackChanges.__guests__ === true) - - const trackChangesValues: Record = {} - if (trackChangesIsObject) { - for (const key of Object.keys(trackChanges)) { - if (key !== '__guests__') { - trackChangesValues[key as UserId] = trackChanges[key as UserId] - } - } - } + const { onForEveryone, onForGuests, onForMembers } = trackChanges 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 = - trackChanges === true || trackChangesValues[member._id] === true + const value = onForEveryone || onForMembers[member._id] === true return (
@@ -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, }) } diff --git a/services/web/frontend/js/features/review-panel-new/context/track-changes-state-context.tsx b/services/web/frontend/js/features/review-panel-new/context/track-changes-state-context.tsx index f88147dbb5..e6f1783be8 100644 --- a/services/web/frontend/js/features/review-panel-new/context/track-changes-state-context.tsx +++ b/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 +export type TrackChangesState = { + onForEveryone: boolean + onForGuests: boolean + onForMembers: Record +} export const TrackChangesStateContext = createContext< TrackChangesState | undefined >(undefined) +type SaveTrackChangesRequestBody = { + on?: boolean + on_for?: Record + 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( - project.trackChangesState ?? false - ) + const [trackChangesValue, setTrackChangesValue] = useState< + 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, value, user.id]) + }, [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 = {} + 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] + ) return ( - - {children} - + + + {children} + + ) } 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 +} diff --git a/services/web/frontend/js/features/source-editor/extensions/shortcuts.ts b/services/web/frontend/js/features/source-editor/extensions/shortcuts.ts index cfe20e563d..027f28fbfa 100644 --- a/services/web/frontend/js/features/source-editor/extensions/shortcuts.ts +++ b/services/web/frontend/js/features/source-editor/extensions/shortcuts.ts @@ -26,7 +26,12 @@ import { import { isSplitTestEnabled } from '@/utils/splitTestUtils' const toggleReviewPanel = () => { - dispatchEditorEvent('toggle-review-panel') + 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 = () => { - dispatchEditorEvent('toggle-track-changes') + if (isSplitTestEnabled('review-panel-redesign')) { + window.dispatchEvent(new Event('toggle-track-changes')) + } else { + dispatchEditorEvent('toggle-track-changes') + } return true } diff --git a/services/web/frontend/js/shared/context/layout-context.tsx b/services/web/frontend/js/shared/context/layout-context.tsx index 5be1cc7aa9..e238a69154 100644 --- a/services/web/frontend/js/shared/context/layout-context.tsx +++ b/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('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('ui.pdfLayout')