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

Remove PDF detach split test (#9275)

GitOrigin-RevId: 9e227fe45d874e01c4282ebbe489d7bd6d2250ac
Alf Eaton 4 лет назад
Родитель
Сommit
e741eb0cb8

+ 1 - 30
services/web/app/src/Features/Project/ProjectController.js

@@ -859,22 +859,6 @@ const ProjectController = {
             }
           )
         },
-        pdfDetachAssignment(cb) {
-          SplitTestHandler.getAssignment(
-            req,
-            res,
-            'pdf-detach',
-            {},
-            (error, assignment) => {
-              // do not fail editor load if assignment fails
-              if (error) {
-                cb(null)
-              } else {
-                cb(null, assignment)
-              }
-            }
-          )
-        },
         pdfjsAssignment(cb) {
           SplitTestHandler.getAssignment(
             req,
@@ -1044,7 +1028,6 @@ const ProjectController = {
           isTokenMember,
           brandVariation,
           newSourceEditorAssignment,
-          pdfDetachAssignment,
           pdfjsAssignment,
           dictionaryEditorAssignment,
           stopOnFirstErrorAssignment,
@@ -1120,20 +1103,9 @@ const ProjectController = {
               }
             }
 
-            const showPdfDetach =
-              !Features.hasFeature('saas') ||
-              shouldDisplayFeature(
-                'pdf_detach',
-                pdfDetachAssignment.variant === 'enabled'
-              )
-
             const debugPdfDetach = shouldDisplayFeature('debug_pdf_detach')
 
-            let detachRole = null
-
-            if (showPdfDetach) {
-              detachRole = req.params.detachRole
-            }
+            const detachRole = req.params.detachRole
 
             const showNewSourceEditorOption =
               newSourceEditorAssignment?.variant === 'codemirror' ||
@@ -1222,7 +1194,6 @@ const ProjectController = {
               showSupport: Features.hasFeature('support'),
               pdfjsVariant: pdfjsAssignment.variant,
               dictionaryEditorEnabled,
-              showPdfDetach,
               debugPdfDetach,
               showNewSourceEditorOption,
               showSymbolPalette,

+ 0 - 1
services/web/app/views/project/editor/meta.pug

@@ -19,7 +19,6 @@ meta(name="ol-aceBasePath" content="/js/" + lib('ace'))
 meta(name="ol-useShareJsHash" data-type="boolean" content=true)
 meta(name="ol-wsRetryHandshake" data-type="json" content=settings.wsRetryHandshake)
 meta(name="ol-pdfjsVariant" content=pdfjsVariant)
-meta(name="ol-showPdfDetach" data-type="boolean" content=showPdfDetach)
 meta(name="ol-debugPdfDetach" data-type="boolean" content=debugPdfDetach)
 meta(name="ol-showNewSourceEditorOption" data-type="boolean" content=showNewSourceEditorOption)
 meta(name="ol-showSymbolPalette" data-type="boolean" content=showSymbolPalette)

+ 0 - 8
services/web/frontend/js/features/editor-navigation-toolbar/components/editor-navigation-toolbar-root.js

@@ -65,7 +65,6 @@ const EditorNavigationToolbarRoot = React.memo(
       view,
       setView,
       setLeftMenuShown,
-      pdfLayout,
     } = useLayoutContext(layoutContextPropTypes)
 
     const { markMessagesAsRead, unreadMessageCount } =
@@ -87,10 +86,6 @@ const EditorNavigationToolbarRoot = React.memo(
       setView(view === 'history' ? 'editor' : 'history')
     }, [view, setView])
 
-    const togglePdfView = useCallback(() => {
-      setView(view === 'pdf' ? 'editor' : 'pdf')
-    }, [view, setView])
-
     const openShareModal = useCallback(() => {
       openShareProjectModal()
     }, [openShareProjectModal])
@@ -133,9 +128,6 @@ const EditorNavigationToolbarRoot = React.memo(
         renameProject={renameProject}
         hasRenamePermissions={permissionsLevel === 'owner'}
         openShareModal={openShareModal}
-        pdfViewIsOpen={view === 'pdf'}
-        pdfButtonIsVisible={pdfLayout === 'flat'}
-        togglePdfView={togglePdfView}
         trackChangesVisible={trackChangesVisible}
       />
     )

+ 0 - 34
services/web/frontend/js/features/editor-navigation-toolbar/components/pdf-toggle-button.tsx

@@ -1,34 +0,0 @@
-import classNames from 'classnames'
-import Tooltip from '../../../shared/components/tooltip'
-import Icon from '../../../shared/components/icon'
-
-type PdfToggleButtonProps = {
-  onClick: () => void
-  pdfViewIsOpen?: boolean
-}
-
-function PdfToggleButton({ onClick, pdfViewIsOpen }: PdfToggleButtonProps) {
-  const classes = classNames(
-    'btn',
-    'btn-full-height',
-    'btn-full-height-no-border',
-    {
-      active: pdfViewIsOpen,
-    }
-  )
-
-  return (
-    <Tooltip
-      id="online-user"
-      description="PDF"
-      overlayProps={{ placement: 'bottom', trigger: ['hover', 'focus'] }}
-    >
-      {/* eslint-disable-next-line jsx-a11y/anchor-is-valid,jsx-a11y/click-events-have-key-events,jsx-a11y/interactive-supports-focus */}
-      <a role="button" className={classes} onClick={onClick}>
-        <Icon type="file-pdf-o" fw accessibilityLabel="PDF" />
-      </a>
-    </Tooltip>
-  )
-}
-
-export default PdfToggleButton

+ 1 - 14
services/web/frontend/js/features/editor-navigation-toolbar/components/toolbar-header.js

@@ -12,7 +12,6 @@ import ProjectNameEditableLabel from './project-name-editable-label'
 import TrackChangesToggleButton from './track-changes-toggle-button'
 import HistoryToggleButton from './history-toggle-button'
 import ShareProjectButton from './share-project-button'
-import PdfToggleButton from './pdf-toggle-button'
 import importOverleafModules from '../../../../macros/import-overleaf-module.macro'
 
 const [publishModalModules] = importOverleafModules('publishModal')
@@ -36,9 +35,6 @@ const ToolbarHeader = React.memo(function ToolbarHeader({
   renameProject,
   hasRenamePermissions,
   openShareModal,
-  pdfViewIsOpen,
-  pdfButtonIsVisible,
-  togglePdfView,
   trackChangesVisible,
 }) {
   const { t } = useTranslation()
@@ -59,12 +55,6 @@ const ToolbarHeader = React.memo(function ToolbarHeader({
         )}
         <BackToProjectsButton />
       </div>
-      {!window.showPdfDetach && pdfButtonIsVisible && (
-        <PdfToggleButton
-          onClick={togglePdfView}
-          pdfViewIsOpen={pdfViewIsOpen}
-        />
-      )}
       {window.showUpgradePrompt && <UpgradePrompt />}
       <ProjectNameEditableLabel
         className="toolbar-center"
@@ -96,7 +86,7 @@ const ToolbarHeader = React.memo(function ToolbarHeader({
           />
         )}
 
-        {window.showPdfDetach && <LayoutDropdownButton />}
+        <LayoutDropdownButton />
 
         {!isRestrictedTokenMember && (
           <ChatToggleButton
@@ -128,9 +118,6 @@ ToolbarHeader.propTypes = {
   renameProject: PropTypes.func.isRequired,
   hasRenamePermissions: PropTypes.bool,
   openShareModal: PropTypes.func.isRequired,
-  pdfViewIsOpen: PropTypes.bool,
-  pdfButtonIsVisible: PropTypes.bool,
-  togglePdfView: PropTypes.func.isRequired,
   trackChangesVisible: PropTypes.bool,
 }
 

+ 4 - 9
services/web/frontend/js/features/pdf-preview/hooks/use-compile-triggers.js

@@ -1,11 +1,8 @@
 import { useCallback } from 'react'
-import getMeta from '../../../utils/meta'
 import { useDetachCompileContext as useCompileContext } from '../../../shared/context/detach-compile-context'
 import useEventListener from '../../../shared/hooks/use-event-listener'
 import useDetachAction from '../../../shared/hooks/use-detach-action'
 
-const showPdfDetach = getMeta('ol-showPdfDetach')
-
 export default function useCompileTriggers() {
   const { startCompile, setChangedAt } = useCompileContext()
 
@@ -18,11 +15,9 @@ export default function useCompileTriggers() {
   )
   const compileHandler = useCallback(
     event => {
-      showPdfDetach
-        ? startOrTriggerCompile(event.detail)
-        : startCompile(event.detail)
+      startOrTriggerCompile(event.detail)
     },
-    [startOrTriggerCompile, startCompile]
+    [startOrTriggerCompile]
   )
   useEventListener('pdf:recompile', compileHandler)
 
@@ -34,8 +29,8 @@ export default function useCompileTriggers() {
     'detached'
   )
   const setChangedAtHandler = useCallback(() => {
-    showPdfDetach ? setOrTriggerChangedAt(Date.now()) : setChangedAt(Date.now())
-  }, [setOrTriggerChangedAt, setChangedAt])
+    setOrTriggerChangedAt(Date.now())
+  }, [setOrTriggerChangedAt])
   useEventListener('doc:changed', setChangedAtHandler)
   useEventListener('doc:saved', setChangedAtHandler) // TODO: store this separately?
 }

+ 2 - 1
services/web/frontend/stories/editor-navigation-toolbar.stories.js

@@ -1,4 +1,5 @@
 import ToolbarHeader from '../js/features/editor-navigation-toolbar/components/toolbar-header'
+import { ScopeDecorator } from './decorators/scope'
 
 export const UpToThreeConnectedUsers = args => {
   return <ToolbarHeader {...args} />
@@ -29,7 +30,6 @@ export default {
     toggleHistoryOpen: { action: 'toggleHistoryOpen' },
     toggleReviewPanelOpen: { action: 'toggleReviewPanelOpen' },
     toggleChatOpen: { action: 'toggleChatOpen' },
-    togglePdfView: { action: 'togglePdfView' },
     openShareModal: { action: 'openShareModal' },
     onShowLeftMenuClick: { action: 'onShowLeftMenuClick' },
   },
@@ -38,4 +38,5 @@ export default {
     onlineUsers: [{ user_id: 'abc', name: 'overleaf' }],
     unreadMessageCount: 0,
   },
+  decorators: [ScopeDecorator],
 }

+ 13 - 29
services/web/test/frontend/features/editor-navigation-toolbar/components/toolbar-header.test.js

@@ -1,7 +1,8 @@
 import { expect } from 'chai'
-import { render, screen } from '@testing-library/react'
+import { screen } from '@testing-library/react'
 
 import ToolbarHeader from '../../../../../frontend/js/features/editor-navigation-toolbar/components/toolbar-header'
+import { renderWithEditorContext } from '../../../helpers/render-with-context'
 
 describe('<ToolbarHeader />', function () {
   const defaultProps = {
@@ -15,7 +16,6 @@ describe('<ToolbarHeader />', function () {
     projectName: 'test project',
     renameProject: () => {},
     openShareModal: () => {},
-    togglePdfView: () => {},
     hasPublishPermissions: true,
     trackChangesVisible: true,
     handleChangeLayout: () => {},
@@ -27,7 +27,7 @@ describe('<ToolbarHeader />', function () {
 
   describe('cobranding logo', function () {
     it('is not displayed by default', function () {
-      render(<ToolbarHeader {...defaultProps} />)
+      renderWithEditorContext(<ToolbarHeader {...defaultProps} />)
       expect(screen.queryByRole('link', { name: 'variation' })).to.not.exist
     })
 
@@ -40,30 +40,14 @@ describe('<ToolbarHeader />', function () {
           logoImgUrl: 'http://cobranding/logo',
         },
       }
-      render(<ToolbarHeader {...props} />)
+      renderWithEditorContext(<ToolbarHeader {...props} />)
       screen.getByRole('link', { name: 'variation' })
     })
   })
 
-  describe('pdf toggle button', function () {
-    it('is not displayed by default', function () {
-      render(<ToolbarHeader {...defaultProps} />)
-      expect(screen.queryByText('PDF')).to.not.exist
-    })
-
-    it('is displayed when "pdfButtonIsVisible" prop is set to true', function () {
-      const props = {
-        ...defaultProps,
-        pdfButtonIsVisible: true,
-      }
-      render(<ToolbarHeader {...props} />)
-      screen.getByText('PDF')
-    })
-  })
-
   describe('track changes toggle button', function () {
     it('is displayed by default', function () {
-      render(<ToolbarHeader {...defaultProps} />)
+      renderWithEditorContext(<ToolbarHeader {...defaultProps} />)
       screen.getByText('Review')
     })
 
@@ -72,7 +56,7 @@ describe('<ToolbarHeader />', function () {
         ...defaultProps,
         isRestrictedTokenMember: true,
       }
-      render(<ToolbarHeader {...props} />)
+      renderWithEditorContext(<ToolbarHeader {...props} />)
       expect(screen.queryByText('Review')).to.not.exist
     })
 
@@ -81,14 +65,14 @@ describe('<ToolbarHeader />', function () {
         ...defaultProps,
         trackChangesVisible: false,
       }
-      render(<ToolbarHeader {...props} />)
+      renderWithEditorContext(<ToolbarHeader {...props} />)
       expect(screen.queryByText('Review')).to.not.exist
     })
   })
 
   describe('History toggle button', function () {
     it('is displayed by default', function () {
-      render(<ToolbarHeader {...defaultProps} />)
+      renderWithEditorContext(<ToolbarHeader {...defaultProps} />)
       screen.getByText('History')
     })
 
@@ -97,14 +81,14 @@ describe('<ToolbarHeader />', function () {
         ...defaultProps,
         isRestrictedTokenMember: true,
       }
-      render(<ToolbarHeader {...props} />)
+      renderWithEditorContext(<ToolbarHeader {...props} />)
       expect(screen.queryByText('History')).to.not.exist
     })
   })
 
   describe('Chat toggle button', function () {
     it('is displayed by default', function () {
-      render(<ToolbarHeader {...defaultProps} />)
+      renderWithEditorContext(<ToolbarHeader {...defaultProps} />)
       screen.getByText('Chat')
     })
 
@@ -113,14 +97,14 @@ describe('<ToolbarHeader />', function () {
         ...defaultProps,
         isRestrictedTokenMember: true,
       }
-      render(<ToolbarHeader {...props} />)
+      renderWithEditorContext(<ToolbarHeader {...props} />)
       expect(screen.queryByText('Chat')).to.not.exist
     })
   })
 
   describe('Publish button', function () {
     it('is displayed by default', function () {
-      render(<ToolbarHeader {...defaultProps} />)
+      renderWithEditorContext(<ToolbarHeader {...defaultProps} />)
       screen.getByText('Submit')
     })
 
@@ -129,7 +113,7 @@ describe('<ToolbarHeader />', function () {
         ...defaultProps,
         hasPublishPermissions: false,
       }
-      render(<ToolbarHeader {...props} />)
+      renderWithEditorContext(<ToolbarHeader {...props} />)
       expect(screen.queryByText('Submit')).to.not.exist
     })
   })

+ 0 - 61
services/web/test/unit/src/Project/ProjectControllerTests.js

@@ -1364,67 +1364,6 @@ describe('ProjectController', function () {
       })
     })
 
-    describe('feature flags', function () {
-      describe('showPdfDetach', function () {
-        describe('showPdfDetach=false', function () {
-          beforeEach(function () {
-            this.Features.hasFeature.withArgs('saas').returns(true)
-          })
-
-          it('should be false by default in SaaS', function (done) {
-            this.res.render = (pageName, opts) => {
-              expect(opts.showPdfDetach).to.be.false
-              done()
-            }
-            this.ProjectController.loadEditor(this.req, this.res)
-          })
-
-          it('should be true by default in Server Pro', function (done) {
-            this.Features.hasFeature.withArgs('saas').returns(false)
-            this.res.render = (pageName, opts) => {
-              expect(opts.showPdfDetach).to.be.true
-              done()
-            }
-            this.ProjectController.loadEditor(this.req, this.res)
-          })
-
-          it('should be false when the split test is enabled and ?pdf_detach=false', function (done) {
-            this.res.render = (pageName, opts) => {
-              expect(opts.showPdfDetach).to.be.false
-              done()
-            }
-            this.SplitTestHandler.getAssignment
-              .withArgs(this.req, this.res, 'pdf-detach')
-              .yields(null, { variant: 'enabled' })
-            this.req.query.pdf_detach = 'false'
-            this.ProjectController.loadEditor(this.req, this.res)
-          })
-        })
-
-        describe('showPdfDetach=true', function () {
-          it('should be true when ?pdf_detach=true', function (done) {
-            this.res.render = (pageName, opts) => {
-              expect(opts.showPdfDetach).to.be.true
-              done()
-            }
-            this.req.query.pdf_detach = 'true'
-            this.ProjectController.loadEditor(this.req, this.res)
-          })
-
-          it('should be true for alpha group', function (done) {
-            this.res.render = (pageName, opts) => {
-              expect(opts.showPdfDetach).to.be.true
-              done()
-            }
-            this.SplitTestHandler.getAssignment
-              .withArgs(this.req, this.res, 'pdf-detach')
-              .yields(null, { variant: 'enabled' })
-            this.ProjectController.loadEditor(this.req, this.res)
-          })
-        })
-      })
-    })
-
     describe('upgrade prompt (on header and share project modal)', function () {
       beforeEach(function () {
         // default to saas enabled