Răsfoiți Sursa

[WEB] Move Review Toggle into the toolbar (#34066)

* move Review Toggle into the toolbar

* cleaning up and adding a comment

* adding the cursor styling

* adding isolation on writefull toolbar to adjust z-index of writefull toolbar

* fixing the dark mode colours for review dropdown trigger

* Fix review mode switcher dark mode styles

GitOrigin-RevId: 36847e0debdc4dce5f96492261d25e7cc46b2e96
Davinder Singh 2 luni în urmă
părinte
comite
e0f542a241

+ 71 - 69
services/web/frontend/js/features/review-panel/components/review-mode-switcher.tsx

@@ -33,86 +33,88 @@ function ReviewModeSwitcher() {
   const view = useCodeMirrorViewContext()
 
   return (
-    <div className="review-mode-switcher-container">
-      <Dropdown className="review-mode-switcher" align="end">
-        <DropdownToggle
-          as={ModeSwitcherToggleButton}
-          id="review-mode-switcher"
-        />
-        <DropdownMenu flip={false}>
-          <OLDropdownMenuItem
-            disabled={!write}
-            onClick={() => {
-              if (mode === 'edit') {
-                view.focus()
-                return
-              }
+    <Dropdown className="review-mode-switcher" align="end">
+      <DropdownToggle as={ModeSwitcherToggleButton} id="review-mode-switcher" />
+      <DropdownMenu
+        flip={false}
+        popperConfig={{ strategy: 'fixed' }}
+        // renderOnMount ensures the menu is in the DOM on mount so Popper.js
+        // can calculate its position correctly when using strategy: 'fixed'
+        // inside the toolbar's stacking context
+        renderOnMount
+      >
+        <OLDropdownMenuItem
+          disabled={!write}
+          onClick={() => {
+            if (mode === 'edit') {
+              view.focus()
+              return
+            }
+            sendMB('editing-mode-change', {
+              role: permissionsLevel,
+              previousMode: mode,
+              newMode: 'edit',
+            })
+            if (user?.id) {
+              saveTrackChangesForCurrentUser(false)
+            } else {
+              saveTrackChanges({ on_for_guests: false })
+            }
+            view.focus()
+          }}
+          description={t('edit_content_directly')}
+          leadingIcon="edit"
+          active={write && mode === 'edit'}
+        >
+          {t('editing')}
+        </OLDropdownMenuItem>
+        <OLDropdownMenuItem
+          disabled={permissionsLevel === 'readOnly'}
+          onClick={() => {
+            if (mode === 'review') {
+              view.focus()
+              return
+            }
+            if (!features.trackChanges) {
+              setUpgradeTrackChangesModal({
+                show: true,
+                location: 'review-switcher',
+              })
+            } else {
               sendMB('editing-mode-change', {
                 role: permissionsLevel,
                 previousMode: mode,
-                newMode: 'edit',
+                newMode: 'review',
               })
               if (user?.id) {
-                saveTrackChangesForCurrentUser(false)
+                saveTrackChangesForCurrentUser(true)
               } else {
-                saveTrackChanges({ on_for_guests: false })
+                saveTrackChanges({ on_for_guests: true })
               }
               view.focus()
-            }}
-            description={t('edit_content_directly')}
-            leadingIcon="edit"
-            active={write && mode === 'edit'}
-          >
-            {t('editing')}
-          </OLDropdownMenuItem>
-          <OLDropdownMenuItem
-            disabled={permissionsLevel === 'readOnly'}
-            onClick={() => {
-              if (mode === 'review') {
-                view.focus()
-                return
-              }
-              if (!features.trackChanges) {
-                setUpgradeTrackChangesModal({
-                  show: true,
-                  location: 'review-switcher',
-                })
-              } else {
-                sendMB('editing-mode-change', {
-                  role: permissionsLevel,
-                  previousMode: mode,
-                  newMode: 'review',
-                })
-                if (user?.id) {
-                  saveTrackChangesForCurrentUser(true)
-                } else {
-                  saveTrackChanges({ on_for_guests: true })
-                }
-                view.focus()
-              }
-            }}
-            description={
-              permissionsLevel === 'review' && !trackedWrite
-                ? t('comment_only')
-                : t('edits_become_suggestions')
             }
-            leadingIcon="rate_review"
-            active={trackedWrite && mode === 'review'}
+          }}
+          description={
+            permissionsLevel === 'review' && !trackedWrite
+              ? t('comment_only')
+              : t('edits_become_suggestions')
+          }
+          leadingIcon="rate_review"
+          active={trackedWrite && mode === 'review'}
+        >
+          {t('reviewing')}
+        </OLDropdownMenuItem>
+        {showViewOption && (
+          <OLDropdownMenuItem
+            description={t('can_view_content')}
+            leadingIcon="visibility"
+            active={mode === 'view'}
           >
-            {t('reviewing')}
+            {t('viewing')}
           </OLDropdownMenuItem>
-          {showViewOption && (
-            <OLDropdownMenuItem
-              description={t('can_view_content')}
-              leadingIcon="visibility"
-              active={mode === 'view'}
-            >
-              {t('viewing')}
-            </OLDropdownMenuItem>
-          )}
-        </DropdownMenu>
-      </Dropdown>
-    </div>
+        )}
+      </DropdownMenu>
+    </Dropdown>
   )
 }
 

+ 1 - 5
services/web/frontend/js/features/review-panel/components/review-panel-container.tsx

@@ -2,7 +2,6 @@ import ReactDOM from 'react-dom'
 import { useCodeMirrorViewContext } from '../../source-editor/components/codemirror-context'
 import { memo } from 'react'
 import ReviewPanel from './review-panel'
-import ReviewModeSwitcher from './review-mode-switcher'
 import useReviewPanelLayout from '../hooks/use-review-panel-layout'
 
 function ReviewPanelContainer() {
@@ -14,10 +13,7 @@ function ReviewPanelContainer() {
   }
 
   return ReactDOM.createPortal(
-    <>
-      <ReviewModeSwitcher />
-      {showPanel && <ReviewPanel mini={mini} />}
-    </>,
+    <>{showPanel && <ReviewPanel mini={mini} />}</>,
     view.scrollDOM
   )
 }

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

@@ -18,6 +18,7 @@ import useDropdown from '../../../shared/hooks/use-dropdown'
 import { getPanel } from '@codemirror/view'
 import { createToolbarPanel } from '../extensions/toolbar/toolbar-panel'
 import EditorSwitch from './editor-switch'
+import ReviewModeSwitcher from '@/features/review-panel/components/review-mode-switcher'
 import SwitchToPDFButton from './switch-to-pdf-button'
 import { DetacherSynctexControl } from '../../pdf-preview/components/detach-synctex-control'
 import DetachCompileButtonWrapper from '../../pdf-preview/components/detach-compile-button-wrapper'
@@ -33,6 +34,7 @@ import Breadcrumbs from '@/features/source-editor/extensions/breadcrumbs'
 import classNames from 'classnames'
 import { useUserSettingsContext } from '@/shared/context/user-settings-context'
 import { useFeatureFlag } from '@/shared/context/split-test-context'
+import { useProjectContext } from '@/shared/context/project-context'
 import importOverleafModules from '../../../../macros/import-overleaf-module.macro'
 
 const sourceEditorToolbarComponents = importOverleafModules(
@@ -62,6 +64,7 @@ const Toolbar = memo(function Toolbar() {
     userSettings: { breadcrumbs },
   } = useUserSettingsContext()
   const visualPreviewEnabled = useFeatureFlag('visual-preview')
+  const { features } = useProjectContext()
 
   const [overflowed, setOverflowed] = useState(false)
 
@@ -206,6 +209,7 @@ const Toolbar = memo(function Toolbar() {
             ref={handleButtons}
           >
             {!visualPreviewEnabled && <EditorSwitch />}
+            {features.trackChangesVisible && <ReviewModeSwitcher />}
             {sourceEditorToolbarEndButtons.map(
               ({ import: { default: Component }, path }) => (
                 <Component key={path} />

+ 21 - 23
services/web/frontend/stylesheets/pages/editor/review-panel.scss

@@ -20,6 +20,9 @@ $rp-type-blue: #6b7797;
   --review-panel-width: 230px;
   --review-panel-width-mini: 24px;
   --review-panel-color-scheme: light;
+  --review-mode-switcher-reviewing-bg: var(--yellow-10);
+  --review-mode-switcher-reviewing-hover-bg: var(--yellow-20);
+  --review-mode-switcher-reviewing-color: var(--yellow-60);
 
   @include theme('default') {
     .ide-redesign-main {
@@ -33,6 +36,9 @@ $rp-type-blue: #6b7797;
       --review-panel-button-hover-bg-color: var(--bg-dark-tertiary);
       --review-panel-border-color: var(--border-divider-dark);
       --review-panel-color-scheme: dark;
+      --review-mode-switcher-reviewing-bg: var(--yellow-70);
+      --review-mode-switcher-reviewing-hover-bg: var(--yellow-60);
+      --review-mode-switcher-reviewing-color: var(--yellow-10);
     }
   }
 }
@@ -712,35 +718,22 @@ del.review-panel-content-highlight {
   pointer-events: none; // this is to prevent mouseLeave event from firing when hovering over the tooltip
 }
 
-.review-mode-switcher-container {
-  position: sticky;
-  top: 0;
-  right: 0;
-}
-
 .review-mode-switcher {
-  position: absolute;
-  top: var(--spacing-03);
-  right: var(--spacing-03);
   font-family: var(--font-sans);
 
   &:hover,
   &:focus {
     .review-mode-switcher-toggle-button.editing {
-      background-color: var(--bg-light-tertiary);
+      background-color: var(--review-panel-button-hover-bg-color);
     }
 
     .review-mode-switcher-toggle-button.reviewing {
-      background-color: var(--yellow-20);
+      background-color: var(--review-mode-switcher-reviewing-hover-bg);
     }
 
     .review-mode-switcher-toggle-button.viewing {
       background-color: var(--blue-20);
     }
-
-    .review-mode-switcher-toggle-label {
-      display: block;
-    }
   }
 
   // prevent gap between button and menu
@@ -755,11 +748,16 @@ del.review-panel-content-highlight {
   font-family: $font-family-base;
   display: flex;
   align-items: center;
-  border-radius: 14px;
+  border-radius: var(--border-radius-base);
   font-size: var(--font-size-02);
-  padding: var(--spacing-02) var(--spacing-03);
-  gap: var(--spacing-02);
-  height: 20px;
+  padding: var(--spacing-01) var(--spacing-02);
+  gap: var(--spacing-01);
+  cursor: pointer;
+
+  &:focus-visible {
+    outline: 2px solid var(--content-primary);
+    outline-offset: 2px;
+  }
 
   .material-symbols {
     font-size: 16px;
@@ -771,13 +769,13 @@ del.review-panel-content-highlight {
   }
 
   &.editing {
-    background-color: var(--bg-light-secondary);
-    color: var(--content-primary);
+    background-color: var(--toolbar-alt-bg-color);
+    color: var(--toolbar-btn-color);
   }
 
   &.reviewing {
-    background-color: var(--yellow-10);
-    color: var(--yellow-60);
+    background-color: var(--review-mode-switcher-reviewing-bg);
+    color: var(--review-mode-switcher-reviewing-color);
   }
 
   &.viewing {

+ 6 - 0
services/web/frontend/stylesheets/pages/editor/toolbar.scss

@@ -295,6 +295,12 @@
   z-index: 10; // Prevent track changes showing over toolbar
 }
 
+// Isolate secondary toolbar rows so they paint below the main toolbar (z-index: 10)
+// and its dropdowns, which would otherwise be obscured due to DOM order.
+.ol-cm-toolbar-wrapper > .writefull {
+  isolation: isolate;
+}
+
 /**************************************
 	Toggle Switch
 ***************************************/