Jelajahi Sumber

Merge pull request #30884 from overleaf/mg-upgrade-modal-tracked-changes

Show modal when user initiates tracked changes from context menu

GitOrigin-RevId: 09ce0aef3eea113cc0b8fc83db00cb8607a6ef9a
Malik Glossop 6 bulan lalu
induk
melakukan
5d47879865

+ 7 - 35
services/web/frontend/js/features/review-panel/components/review-mode-switcher.tsx

@@ -1,4 +1,4 @@
-import { forwardRef, memo, MouseEventHandler, useState } from 'react'
+import { forwardRef, memo, MouseEventHandler } from 'react'
 import {
 import {
   Dropdown,
   Dropdown,
   DropdownMenu,
   DropdownMenu,
@@ -7,10 +7,7 @@ import {
 import OLDropdownMenuItem from '@/shared/components/ol/ol-dropdown-menu-item'
 import OLDropdownMenuItem from '@/shared/components/ol/ol-dropdown-menu-item'
 import MaterialIcon from '@/shared/components/material-icon'
 import MaterialIcon from '@/shared/components/material-icon'
 import classNames from 'classnames'
 import classNames from 'classnames'
-import {
-  useTrackChangesStateActionsContext,
-  useTrackChangesStateContext,
-} from '../context/track-changes-state-context'
+import { useTrackChangesStateActionsContext } from '../context/track-changes-state-context'
 import { useUserContext } from '@/shared/context/user-context'
 import { useUserContext } from '@/shared/context/user-context'
 import { useTranslation } from 'react-i18next'
 import { useTranslation } from 'react-i18next'
 import { usePermissionsContext } from '@/features/ide-react/context/permissions-context'
 import { usePermissionsContext } from '@/features/ide-react/context/permissions-context'
@@ -18,41 +15,20 @@ import usePersistedState from '@/shared/hooks/use-persisted-state'
 import { sendMB } from '@/infrastructure/event-tracking'
 import { sendMB } from '@/infrastructure/event-tracking'
 import { useIdeReactContext } from '@/features/ide-react/context/ide-react-context'
 import { useIdeReactContext } from '@/features/ide-react/context/ide-react-context'
 import { useProjectContext } from '@/shared/context/project-context'
 import { useProjectContext } from '@/shared/context/project-context'
-import UpgradeTrackChangesModal from './upgrade-track-changes-modal'
 import { useCodeMirrorViewContext } from '@/features/source-editor/components/codemirror-context'
 import { useCodeMirrorViewContext } from '@/features/source-editor/components/codemirror-context'
-
-type Mode = 'view' | 'review' | 'edit'
-
-const useCurrentMode = (): Mode => {
-  const trackChanges = useTrackChangesStateContext()
-  const user = useUserContext()
-  const trackChangesForCurrentUser =
-    trackChanges?.onForEveryone ||
-    (user?.id && trackChanges?.onForMembers[user.id]) ||
-    (!user?.id && trackChanges?.onForGuests)
-  const { permissionsLevel } = useIdeReactContext()
-
-  if (permissionsLevel === 'readOnly') {
-    return 'view'
-  } else if (permissionsLevel === 'review') {
-    return 'review'
-  } else if (trackChangesForCurrentUser) {
-    return 'review'
-  } else {
-    return 'edit'
-  }
-}
+import { useEditorContext } from '@/shared/context/editor-context'
+import { useTrackingChangesMode } from '@/shared/hooks/use-tracking-changes-mode'
 
 
 function ReviewModeSwitcher() {
 function ReviewModeSwitcher() {
   const { t } = useTranslation()
   const { t } = useTranslation()
   const user = useUserContext()
   const user = useUserContext()
   const { saveTrackChangesForCurrentUser, saveTrackChanges } =
   const { saveTrackChangesForCurrentUser, saveTrackChanges } =
     useTrackChangesStateActionsContext()
     useTrackChangesStateActionsContext()
-  const mode = useCurrentMode()
+  const mode = useTrackingChangesMode()
   const { permissionsLevel } = useIdeReactContext()
   const { permissionsLevel } = useIdeReactContext()
   const { write, trackedWrite } = usePermissionsContext()
   const { write, trackedWrite } = usePermissionsContext()
   const { features } = useProjectContext()
   const { features } = useProjectContext()
-  const [showUpgradeModal, setShowUpgradeModal] = useState(false)
+  const { setShowUpgradeModal } = useEditorContext()
   const showViewOption = permissionsLevel === 'readOnly'
   const showViewOption = permissionsLevel === 'readOnly'
   const view = useCodeMirrorViewContext()
   const view = useCodeMirrorViewContext()
 
 
@@ -133,10 +109,6 @@ function ReviewModeSwitcher() {
           )}
           )}
         </DropdownMenu>
         </DropdownMenu>
       </Dropdown>
       </Dropdown>
-      <UpgradeTrackChangesModal
-        show={showUpgradeModal}
-        setShow={setShowUpgradeModal}
-      />
     </div>
     </div>
   )
   )
 }
 }
@@ -146,7 +118,7 @@ const ModeSwitcherToggleButton = forwardRef<
   { onClick: MouseEventHandler<HTMLButtonElement>; 'aria-expanded': boolean }
   { onClick: MouseEventHandler<HTMLButtonElement>; 'aria-expanded': boolean }
 >(({ onClick, 'aria-expanded': ariaExpanded }, ref) => {
 >(({ onClick, 'aria-expanded': ariaExpanded }, ref) => {
   const { t } = useTranslation()
   const { t } = useTranslation()
-  const mode = useCurrentMode()
+  const mode = useTrackingChangesMode()
 
 
   if (mode === 'edit') {
   if (mode === 'edit') {
     return (
     return (

+ 12 - 11
services/web/frontend/js/features/review-panel/components/upgrade-track-changes-modal.tsx

@@ -16,22 +16,20 @@ import OLButton from '@/shared/components/ol/ol-button'
 import OLRow from '@/shared/components/ol/ol-row'
 import OLRow from '@/shared/components/ol/ol-row'
 import OLCol from '@/shared/components/ol/ol-col'
 import OLCol from '@/shared/components/ol/ol-col'
 import MaterialIcon from '@/shared/components/material-icon'
 import MaterialIcon from '@/shared/components/material-icon'
+import { useEditorContext } from '@/shared/context/editor-context'
 
 
-type UpgradeTrackChangesModalProps = {
-  show: boolean
-  setShow: React.Dispatch<React.SetStateAction<boolean>>
-}
-
-function UpgradeTrackChangesModal({
-  show,
-  setShow,
-}: UpgradeTrackChangesModalProps) {
+function UpgradeTrackChangesModal() {
   const { t } = useTranslation()
   const { t } = useTranslation()
   const { project } = useProjectContext()
   const { project } = useProjectContext()
   const user = useUserContext()
   const user = useUserContext()
+  const { showUpgradeModal, setShowUpgradeModal } = useEditorContext()
+
+  if (!showUpgradeModal) {
+    return null
+  }
 
 
   return (
   return (
-    <OLModal show={show} onHide={() => setShow(false)}>
+    <OLModal show={showUpgradeModal} onHide={() => setShowUpgradeModal(false)}>
       <OLModalHeader>
       <OLModalHeader>
         <OLModalTitle>{t('upgrade_to_review')}</OLModalTitle>
         <OLModalTitle>{t('upgrade_to_review')}</OLModalTitle>
       </OLModalHeader>
       </OLModalHeader>
@@ -94,7 +92,10 @@ function UpgradeTrackChangesModal({
         )}
         )}
       </OLModalBody>
       </OLModalBody>
       <OLModalFooter>
       <OLModalFooter>
-        <OLButton variant="secondary" onClick={() => setShow(false)}>
+        <OLButton
+          variant="secondary"
+          onClick={() => setShowUpgradeModal(false)}
+        >
           {t('close')}
           {t('close')}
         </OLButton>
         </OLButton>
       </OLModalFooter>
       </OLModalFooter>

+ 2 - 0
services/web/frontend/js/features/source-editor/components/codemirror-editor.tsx

@@ -24,6 +24,7 @@ import { useProjectContext } from '@/shared/context/project-context'
 import { useFeatureFlag } from '@/shared/context/split-test-context'
 import { useFeatureFlag } from '@/shared/context/split-test-context'
 import { useEditorOpenDocContext } from '@/features/ide-react/context/editor-open-doc-context'
 import { useEditorOpenDocContext } from '@/features/ide-react/context/editor-open-doc-context'
 import { useEditorPropertiesContext } from '@/features/ide-react/context/editor-properties-context'
 import { useEditorPropertiesContext } from '@/features/ide-react/context/editor-properties-context'
+import UpgradeTrackChangesModal from '@/features/review-panel/components/upgrade-track-changes-modal'
 
 
 // TODO: remove this when definitely no longer used
 // TODO: remove this when definitely no longer used
 export * from './codemirror-context'
 export * from './codemirror-context'
@@ -100,6 +101,7 @@ function CodeMirrorEditorComponents({
       <EditorContextMenu />
       <EditorContextMenu />
       {features.trackChangesVisible && <ReviewTooltipMenu />}
       {features.trackChangesVisible && <ReviewTooltipMenu />}
       {features.trackChangesVisible && <ReviewPanelRoot />}
       {features.trackChangesVisible && <ReviewPanelRoot />}
+      {features.trackChangesVisible && <UpgradeTrackChangesModal />}
 
 
       {sourceEditorComponents.map(
       {sourceEditorComponents.map(
         ({ import: { default: Component }, path }) => (
         ({ import: { default: Component }, path }) => (

+ 12 - 3
services/web/frontend/js/features/source-editor/hooks/use-context-menu-items.tsx

@@ -24,6 +24,8 @@ import {
   pasteWithFormatting,
   pasteWithFormatting,
 } from '../commands/clipboard'
 } from '../commands/clipboard'
 import { isVisual } from '../extensions/visual/visual'
 import { isVisual } from '../extensions/visual/visual'
+import { useEditorContext } from '@/shared/context/editor-context'
+import { useTrackingChangesMode } from '@/shared/hooks/use-tracking-changes-mode'
 
 
 export const useContextMenuItems = () => {
 export const useContextMenuItems = () => {
   const view = useCodeMirrorViewContext()
   const view = useCodeMirrorViewContext()
@@ -38,6 +40,9 @@ export const useContextMenuItems = () => {
   const { shortcuts } = useCommandRegistry()
   const { shortcuts } = useCommandRegistry()
   const { features } = useProjectContext()
   const { features } = useProjectContext()
   const requestedPdfSyncRef = useRef(false)
   const requestedPdfSyncRef = useRef(false)
+  const { setShowUpgradeModal } = useEditorContext()
+  const trackingChangesMode = useTrackingChangesMode()
+  const isReview = trackingChangesMode === 'review'
 
 
   const closeMenu = useCallback(() => {
   const closeMenu = useCallback(() => {
     view.dispatch({ effects: closeContextMenuEffect.of(null) })
     view.dispatch({ effects: closeContextMenuEffect.of(null) })
@@ -97,6 +102,11 @@ export const useContextMenuItems = () => {
   const handleDelete = wrapForContextMenu(() => commands.deleteSelection(view))
   const handleDelete = wrapForContextMenu(() => commands.deleteSelection(view))
 
 
   const handleToggleTrackChanges = wrapForContextMenu(() => {
   const handleToggleTrackChanges = wrapForContextMenu(() => {
+    // Matching the logic in review toggle to ensure consistency for server pro
+    if (!features.trackChanges && !isReview) {
+      setShowUpgradeModal(true)
+      return true
+    }
     window.dispatchEvent(new Event('toggle-track-changes'))
     window.dispatchEvent(new Event('toggle-track-changes'))
     return true
     return true
   })
   })
@@ -173,10 +183,9 @@ export const useContextMenuItems = () => {
       {
       {
         label: wantTrackChanges ? t('back_to_editing') : t('suggest_edits'),
         label: wantTrackChanges ? t('back_to_editing') : t('suggest_edits'),
         handler: handleToggleTrackChanges,
         handler: handleToggleTrackChanges,
-        // disable for now, future work opens upgrade modal
-        disabled: !features.trackChanges,
+        disabled: false,
         separatorAbove: true,
         separatorAbove: true,
-        show: canEdit,
+        show: canEdit && features.trackChangesVisible,
         shortcut: getShortcut('toggle-track-changes'),
         shortcut: getShortcut('toggle-track-changes'),
       },
       },
       {
       {

+ 8 - 0
services/web/frontend/js/shared/context/editor-context.tsx

@@ -39,6 +39,8 @@ export const EditorContext = createContext<
       premiumSuggestionResetDate: Date
       premiumSuggestionResetDate: Date
       writefullInstance: WritefullAPI | null
       writefullInstance: WritefullAPI | null
       setWritefullInstance: (instance: WritefullAPI) => void
       setWritefullInstance: (instance: WritefullAPI) => void
+      showUpgradeModal: boolean
+      setShowUpgradeModal: Dispatch<SetStateAction<boolean>>
     }
     }
   | undefined
   | undefined
 >(undefined)
 >(undefined)
@@ -95,6 +97,8 @@ export const EditorProvider: FC<React.PropsWithChildren> = ({ children }) => {
         : new Date()
         : new Date()
     })
     })
 
 
+  const [showUpgradeModal, setShowUpgradeModal] = useState(false)
+
   const isPendingEditor = useMemo(
   const isPendingEditor = useMemo(
     () =>
     () =>
       Boolean(
       Boolean(
@@ -186,6 +190,8 @@ export const EditorProvider: FC<React.PropsWithChildren> = ({ children }) => {
       setPremiumSuggestionResetDate,
       setPremiumSuggestionResetDate,
       writefullInstance,
       writefullInstance,
       setWritefullInstance,
       setWritefullInstance,
+      showUpgradeModal,
+      setShowUpgradeModal,
     }),
     }),
     [
     [
       cobranding,
       cobranding,
@@ -205,6 +211,8 @@ export const EditorProvider: FC<React.PropsWithChildren> = ({ children }) => {
       setPremiumSuggestionResetDate,
       setPremiumSuggestionResetDate,
       writefullInstance,
       writefullInstance,
       setWritefullInstance,
       setWritefullInstance,
+      showUpgradeModal,
+      setShowUpgradeModal,
     ]
     ]
   )
   )
 
 

+ 28 - 0
services/web/frontend/js/shared/hooks/use-tracking-changes-mode.ts

@@ -0,0 +1,28 @@
+import { useIdeReactContext } from '@/features/ide-react/context/ide-react-context'
+import { useTrackChangesStateContext } from '@/features/review-panel/context/track-changes-state-context'
+import { useUserContext } from '../context/user-context'
+
+type Mode = 'view' | 'review' | 'edit'
+
+export const useTrackingChangesMode = (): Mode => {
+  const trackChanges = useTrackChangesStateContext()
+  const user = useUserContext()
+  const { permissionsLevel } = useIdeReactContext()
+
+  if (permissionsLevel === 'readOnly') {
+    return 'view'
+  } else if (permissionsLevel === 'review') {
+    return 'review'
+  }
+
+  const trackChangesForCurrentUser =
+    trackChanges?.onForEveryone ||
+    (user?.id && trackChanges?.onForMembers[user.id]) ||
+    (!user?.id && trackChanges?.onForGuests)
+
+  if (trackChangesForCurrentUser) {
+    return 'review'
+  } else {
+    return 'edit'
+  }
+}

+ 71 - 15
services/web/test/frontend/features/source-editor/components/codemirror-editor-context-menu.spec.tsx

@@ -90,6 +90,10 @@ describe('editor context menu', { scrollBehavior: false }, function () {
     window.metaAttributesCache.set('ol-splitTestVariants', {
     window.metaAttributesCache.set('ol-splitTestVariants', {
       'editor-context-menu': 'enabled',
       'editor-context-menu': 'enabled',
     })
     })
+    cy.intercept('POST', '/project/*/track_changes', {
+      statusCode: 200,
+      body: {},
+    }).as('trackChanges')
     cy.interceptEvents()
     cy.interceptEvents()
     cy.interceptMetadata()
     cy.interceptMetadata()
   })
   })
@@ -184,7 +188,10 @@ describe('editor context menu', { scrollBehavior: false }, function () {
 
 
       cy.mount(
       cy.mount(
         <TestContainer>
         <TestContainer>
-          <EditorProviders scope={scope}>
+          <EditorProviders
+            scope={scope}
+            features={{ trackChangesVisible: true }}
+          >
             <CodeMirrorEditor />
             <CodeMirrorEditor />
           </EditorProviders>
           </EditorProviders>
         </TestContainer>
         </TestContainer>
@@ -224,7 +231,10 @@ describe('editor context menu', { scrollBehavior: false }, function () {
 
 
       cy.mount(
       cy.mount(
         <TestContainer>
         <TestContainer>
-          <EditorProviders scope={scope}>
+          <EditorProviders
+            scope={scope}
+            features={{ trackChangesVisible: true }}
+          >
             <CodeMirrorEditor />
             <CodeMirrorEditor />
           </EditorProviders>
           </EditorProviders>
         </TestContainer>
         </TestContainer>
@@ -369,7 +379,7 @@ describe('editor context menu', { scrollBehavior: false }, function () {
     })
     })
   })
   })
 
 
-  describe('track changes toggle', function () {
+  describe('when clicking the track changes buttons', function () {
     let toggleTrackChangesListener: Cypress.Agent<sinon.SinonStub>
     let toggleTrackChangesListener: Cypress.Agent<sinon.SinonStub>
 
 
     beforeEach(function () {
     beforeEach(function () {
@@ -394,11 +404,19 @@ describe('editor context menu', { scrollBehavior: false }, function () {
         <TestContainer>
         <TestContainer>
           <EditorProviders
           <EditorProviders
             scope={scope}
             scope={scope}
-            projectFeatures={{ trackChanges: true }}
             providers={{
             providers={{
               EditorPropertiesProvider: makeEditorPropertiesProvider({
               EditorPropertiesProvider: makeEditorPropertiesProvider({
                 wantTrackChanges: false,
                 wantTrackChanges: false,
               }),
               }),
+              ProjectProvider: makeProjectProvider(
+                mockProject({
+                  trackChangesState: false,
+                  projectFeatures: {
+                    trackChanges: true,
+                    trackChangesVisible: true,
+                  },
+                })
+              ),
             }}
             }}
           >
           >
             <CodeMirrorEditor />
             <CodeMirrorEditor />
@@ -432,11 +450,17 @@ describe('editor context menu', { scrollBehavior: false }, function () {
         <TestContainer>
         <TestContainer>
           <EditorProviders
           <EditorProviders
             scope={scope}
             scope={scope}
-            projectFeatures={{ trackChanges: true }}
             providers={{
             providers={{
-              EditorPropertiesProvider: makeEditorPropertiesProvider({
-                wantTrackChanges: true,
-              }),
+              ProjectProvider: makeProjectProvider(
+                mockProject({
+                  // Re-assigns `withTrackChanges` value in the `track-changes-state-context` useEffect hook
+                  trackChangesState: true,
+                  projectFeatures: {
+                    trackChanges: true,
+                    trackChangesVisible: true,
+                  },
+                })
+              ),
             }}
             }}
           >
           >
             <CodeMirrorEditor />
             <CodeMirrorEditor />
@@ -463,14 +487,40 @@ describe('editor context menu', { scrollBehavior: false }, function () {
       cy.get('@toggleTrackChanges').should('have.been.calledOnce')
       cy.get('@toggleTrackChanges').should('have.been.calledOnce')
     })
     })
 
 
-    it('should disable suggest edits when project does not support track changes', function () {
+    it('should open upgrade modal when user does not support track changes', function () {
+      const scope = mockScope()
+
+      cy.mount(
+        <TestContainer>
+          <EditorProviders
+            scope={scope}
+            features={{ trackChangesVisible: true, trackChanges: false }}
+          >
+            <CodeMirrorEditor />
+          </EditorProviders>
+        </TestContainer>
+      )
+
+      cy.get('.cm-line').eq(10).rightclick()
+
+      cy.findByRole('menu').within(() => {
+        cy.findByRole('menuitem', { name: /suggest edits/i }).click()
+      })
+
+      cy.findByRole('dialog').should('be.visible')
+      cy.findByRole('dialog').should('contain.text', 'Upgrade to Review')
+    })
+  })
+
+  describe('when trackChangesVisible feature is disabled', function () {
+    it('should hide the track changes button', function () {
       const scope = mockScope()
       const scope = mockScope()
 
 
       cy.mount(
       cy.mount(
         <TestContainer>
         <TestContainer>
           <EditorProviders
           <EditorProviders
             scope={scope}
             scope={scope}
-            projectFeatures={{ trackChanges: false }}
+            features={{ trackChangesVisible: false }}
           >
           >
             <CodeMirrorEditor />
             <CodeMirrorEditor />
           </EditorProviders>
           </EditorProviders>
@@ -481,9 +531,10 @@ describe('editor context menu', { scrollBehavior: false }, function () {
 
 
       cy.findByRole('menu').within(() => {
       cy.findByRole('menu').within(() => {
         cy.findByRole('menuitem', { name: /suggest edits/i }).should(
         cy.findByRole('menuitem', { name: /suggest edits/i }).should(
-          'have.attr',
-          'aria-disabled',
-          'true'
+          'not.exist'
+        )
+        cy.findByRole('menuitem', { name: /back to editing/i }).should(
+          'not.exist'
         )
         )
       })
       })
     })
     })
@@ -559,6 +610,9 @@ describe('editor context menu', { scrollBehavior: false }, function () {
         cy.findByRole('menuitem', { name: /suggest edits/i }).should(
         cy.findByRole('menuitem', { name: /suggest edits/i }).should(
           'not.exist'
           'not.exist'
         )
         )
+        cy.findByRole('menuitem', { name: /back to editing/i }).should(
+          'not.exist'
+        )
         cy.findByRole('menuitem', { name: /comment/i }).should('be.enabled')
         cy.findByRole('menuitem', { name: /comment/i }).should('be.enabled')
       })
       })
     })
     })
@@ -821,7 +875,6 @@ describe('editor context menu', { scrollBehavior: false }, function () {
         <TestContainer>
         <TestContainer>
           <EditorProviders
           <EditorProviders
             scope={scope}
             scope={scope}
-            projectFeatures={{ trackChangesVisible: true }}
             features={{ trackChangesVisible: true }}
             features={{ trackChangesVisible: true }}
           >
           >
             <CodeMirrorEditor />
             <CodeMirrorEditor />
@@ -964,7 +1017,10 @@ describe('editor context menu', { scrollBehavior: false }, function () {
 
 
       cy.mount(
       cy.mount(
         <TestContainer>
         <TestContainer>
-          <EditorProviders scope={scope}>
+          <EditorProviders
+            scope={scope}
+            features={{ trackChangesVisible: true }}
+          >
             <CodeMirrorEditor />
             <CodeMirrorEditor />
           </EditorProviders>
           </EditorProviders>
         </TestContainer>
         </TestContainer>

+ 2 - 1
services/web/test/frontend/features/source-editor/helpers/mock-project.ts

@@ -11,6 +11,7 @@ export const mockProject = ({
   projectOwner = undefined,
   projectOwner = undefined,
   spellCheckLanguage = 'en',
   spellCheckLanguage = 'en',
   rootFolder = null,
   rootFolder = null,
+  trackChangesState = false,
 }: any = {}) => {
 }: any = {}) => {
   return {
   return {
     _id: 'test-project',
     _id: 'test-project',
@@ -63,7 +64,7 @@ export const mockProject = ({
     },
     },
     compiler: 'pdflatex' as ProjectCompiler,
     compiler: 'pdflatex' as ProjectCompiler,
     imageName: 'texlive-full:2024.1',
     imageName: 'texlive-full:2024.1',
-    trackChangesState: false,
+    trackChangesState,
     invites: [],
     invites: [],
     members: [],
     members: [],
     owner: projectOwner || {
     owner: projectOwner || {