Przeglądaj źródła

Merge pull request #31372 from overleaf/mj-tutorial-cleanup

[web] Stop manually checking inactiveTutorials

GitOrigin-RevId: c2d14d8633ff58c1dc0b0544c890090d8beb64e2
Mathias Jakobsen 6 miesięcy temu
rodzic
commit
08f2597948

+ 3 - 8
services/web/frontend/js/features/history/components/change-list/all-history-list.tsx

@@ -13,7 +13,6 @@ import Close from '@/shared/components/close'
 import { Trans, useTranslation } from 'react-i18next'
 import MaterialIcon from '@/shared/components/material-icon'
 import useTutorial from '@/shared/hooks/promotions/use-tutorial'
-import { useTutorialContext } from '@/shared/context/tutorial-context'
 
 function AllHistoryList() {
   const { id: currentUserId } = useUserContext()
@@ -91,12 +90,12 @@ function AllHistoryList() {
     }
   }, [updatesLoadingState])
 
-  const { inactiveTutorials } = useTutorialContext()
   const {
     showPopup: showHistoryTutorial,
     tryShowingPopup: tryShowingHistoryTutorial,
     hideUntilReload: hideHistoryTutorialUntilReload,
     completeTutorial: completeHistoryTutorial,
+    checkCompletion: checkHistoryTutorialCompletion,
   } = useTutorial('react-history-buttons-tutorial', {
     name: 'react-history-buttons-tutorial',
   })
@@ -111,27 +110,23 @@ function AllHistoryList() {
     visibleUpdates.length === 2 && updatesInfo.freeHistoryLimitHit
 
   useEffect(() => {
-    const hasCompletedHistoryTutorial = inactiveTutorials.includes(
-      'react-history-buttons-tutorial'
-    )
-
     // wait for the layout to settle before showing popover, to avoid a flash/ instant move
     if (!layoutSettled) {
       return
     }
     if (
-      !hasCompletedHistoryTutorial &&
+      !checkHistoryTutorialCompletion() &&
       isMoreThanOneVersion &&
       !isPaywallAndNonComparable
     ) {
       tryShowingHistoryTutorial()
     }
   }, [
-    inactiveTutorials,
     isMoreThanOneVersion,
     isPaywallAndNonComparable,
     layoutSettled,
     tryShowingHistoryTutorial,
+    checkHistoryTutorialCompletion,
   ])
 
   const { t } = useTranslation()

+ 3 - 5
services/web/frontend/js/features/ide-react/components/editor-survey.tsx

@@ -10,7 +10,6 @@ import { memo, useCallback, useEffect, useMemo, useState } from 'react'
 import { sendMB } from '@/infrastructure/event-tracking'
 import { useIsNewEditorEnabled } from '@/features/ide-redesign/utils/new-editor-utils'
 import { useTranslation } from 'react-i18next'
-import { useTutorialContext } from '@/shared/context/tutorial-context'
 
 type EditorSurveyPage = 'ease-of-use' | 'meets-my-needs' | 'thank-you'
 
@@ -28,8 +27,6 @@ const EditorSurveyContent = () => {
   const [easeOfUse, setEaseOfUse] = useState<number | null>(null)
   const [meetsMyNeeds, setMeetsMyNeeds] = useState<number | null>(null)
   const [page, setPage] = useState<EditorSurveyPage>('ease-of-use')
-  const { inactiveTutorials } = useTutorialContext()
-  const hasCompletedSurvey = inactiveTutorials.includes(TUTORIAL_KEY)
   const newEditor = useIsNewEditorEnabled()
 
   const { t } = useTranslation()
@@ -39,15 +36,16 @@ const EditorSurveyContent = () => {
     showPopup: showSurvey,
     dismissTutorial: dismissSurvey,
     completeTutorial: completeSurvey,
+    checkCompletion: checkSurveyCompletion,
   } = useTutorial(TUTORIAL_KEY, {
     name: TUTORIAL_KEY,
   })
 
   useEffect(() => {
-    if (!hasCompletedSurvey) {
+    if (!checkSurveyCompletion()) {
       tryShowingSurvey()
     }
-  }, [hasCompletedSurvey, tryShowingSurvey])
+  }, [checkSurveyCompletion, tryShowingSurvey])
 
   const onSubmit = useCallback(() => {
     sendMB('editor-survey-submit', {

+ 3 - 8
services/web/frontend/js/features/ide-redesign/components/new-editor-opt-out-intro-modal.tsx

@@ -12,18 +12,17 @@ import { useTranslation } from 'react-i18next'
 import { useIsNewToNewEditor } from '../utils/new-editor-utils'
 import { useNewEditorTourContext } from '../contexts/new-editor-tour-context'
 import promoVideo from './new-editor-promo-video.mp4'
-import { useTutorialContext } from '@/shared/context/tutorial-context'
 
 const TUTORIAL_KEY = 'new-editor-intro-2'
 
 export default function NewEditorOptOutIntroModal() {
-  const { inactiveTutorials } = useTutorialContext()
   const {
     tryShowingPopup,
     showPopup: showModal,
     dismissTutorial,
     completeTutorial,
     clearPopup,
+    checkCompletion,
   } = useTutorial(TUTORIAL_KEY, {
     name: TUTORIAL_KEY,
   })
@@ -35,15 +34,11 @@ export default function NewEditorOptOutIntroModal() {
   const isNewToNewEditor = useIsNewToNewEditor()
 
   useEffect(() => {
-    if (
-      isNewToNewEditor &&
-      !hasShown &&
-      !inactiveTutorials.includes(TUTORIAL_KEY)
-    ) {
+    if (isNewToNewEditor && !hasShown && !checkCompletion()) {
       tryShowingPopup('notification-prompt')
       setHasShown(true)
     }
-  }, [tryShowingPopup, inactiveTutorials, isNewToNewEditor, hasShown])
+  }, [tryShowingPopup, checkCompletion, isNewToNewEditor, hasShown])
 
   const startProductTour = useCallback(() => {
     completeTutorial({ event: 'notification-click', action: 'complete' })

+ 3 - 4
services/web/frontend/js/features/ide-redesign/components/old-editor-warning-tooltip.tsx

@@ -6,7 +6,6 @@ import useTutorial from '@/shared/hooks/promotions/use-tutorial'
 import { useCallback, useEffect, useState } from 'react'
 import { useSwitchEnableNewEditorState } from '../hooks/use-switch-enable-new-editor-state'
 import { canUseNewEditor } from '../utils/new-editor-utils'
-import { useTutorialContext } from '@/shared/context/tutorial-context'
 
 const TUTORIAL_KEY = 'old-editor-warning-tooltip-2'
 
@@ -15,7 +14,6 @@ export default function OldEditorWarningTooltip({
 }: {
   target: HTMLElement | null
 }) {
-  const { inactiveTutorials } = useTutorialContext()
   const { t } = useTranslation()
   const { loading, setEditorRedesignStatus } = useSwitchEnableNewEditorState()
 
@@ -25,6 +23,7 @@ export default function OldEditorWarningTooltip({
     dismissTutorial,
     completeTutorial,
     clearPopup,
+    checkCompletion,
   } = useTutorial(TUTORIAL_KEY, {
     name: TUTORIAL_KEY,
   })
@@ -32,11 +31,11 @@ export default function OldEditorWarningTooltip({
   const canShow = canUseNewEditor()
 
   useEffect(() => {
-    if (canShow && !hasShown && !inactiveTutorials.includes(TUTORIAL_KEY)) {
+    if (canShow && !hasShown && !checkCompletion()) {
       tryShowingPopup('notification-prompt')
       setHasShown(true)
     }
-  }, [tryShowingPopup, inactiveTutorials, hasShown, canShow])
+  }, [tryShowingPopup, checkCompletion, hasShown, canShow])
 
   const onSwitch = useCallback(() => {
     completeTutorial({ event: 'notification-click', action: 'complete' })

+ 9 - 6
services/web/frontend/js/features/ide-redesign/components/tooltip-promo.tsx

@@ -1,5 +1,4 @@
 import Close from '@/shared/components/close'
-import { useTutorialContext } from '@/shared/context/tutorial-context'
 import useTutorial from '@/shared/hooks/promotions/use-tutorial'
 import { isSplitTestEnabled } from '@/utils/splitTestUtils'
 import classNames from 'classnames'
@@ -26,15 +25,19 @@ export default function TooltipPromotion({
   placement?: OverlayProps['placement']
   splitTestName?: string
 }) {
-  const { inactiveTutorials } = useTutorialContext()
-  const { showPopup, tryShowingPopup, hideUntilReload, dismissTutorial } =
-    useTutorial(tutorialKey, eventData)
+  const {
+    showPopup,
+    tryShowingPopup,
+    hideUntilReload,
+    dismissTutorial,
+    checkCompletion,
+  } = useTutorial(tutorialKey, eventData)
 
   useEffect(() => {
-    if (!inactiveTutorials.includes(tutorialKey)) {
+    if (!checkCompletion()) {
       tryShowingPopup()
     }
-  }, [tryShowingPopup, inactiveTutorials, tutorialKey])
+  }, [tryShowingPopup, checkCompletion, tutorialKey])
 
   const isInSplitTestIfNeeded = splitTestName
     ? isSplitTestEnabled(splitTestName)

+ 3 - 7
services/web/frontend/js/features/monthly-texlive/rolling-compile-image-changed-alert.tsx

@@ -4,14 +4,13 @@ import OLNotification from '@/shared/components/ol/ol-notification'
 import { useTranslation, Trans } from 'react-i18next'
 import { useCallback } from 'react'
 import { onRollingBuild } from '@/shared/utils/rolling-build'
-import { useTutorialContext } from '@/shared/context/tutorial-context'
 
 export const TUTORIAL_KEY = 'rolling-compile-image-changed'
 
 const RollingCompileImageChangedAlert = () => {
-  const { completeTutorial } = useTutorial(TUTORIAL_KEY)
+  const { completeTutorial, checkCompletion: hasCompleted } =
+    useTutorial(TUTORIAL_KEY)
   const { project } = useProjectContext()
-  const { inactiveTutorials } = useTutorialContext()
 
   const { t } = useTranslation()
 
@@ -19,10 +18,7 @@ const RollingCompileImageChangedAlert = () => {
     completeTutorial({ event: 'promo-click', action: 'complete' })
   }, [completeTutorial])
 
-  if (
-    inactiveTutorials.includes(TUTORIAL_KEY) ||
-    !onRollingBuild(project?.imageName)
-  ) {
+  if (hasCompleted() || !onRollingBuild(project?.imageName)) {
     return null
   }
 

+ 7 - 7
services/web/frontend/js/features/project-list/components/sidebar/use-themed-dashboard-intro.ts

@@ -1,5 +1,4 @@
 import { useFeatureFlag } from '@/shared/context/split-test-context'
-import { useTutorialContext } from '@/shared/context/tutorial-context'
 import useTutorial from '@/shared/hooks/promotions/use-tutorial'
 import { useCallback, useEffect, useRef } from 'react'
 
@@ -8,12 +7,12 @@ const THEMED_DASHBOARD_TUTORIAL_KEY = 'themed-dashboard-intro'
 export const useThemedDashboardIntro = () => {
   const themedDsNav = useFeatureFlag('themed-project-dashboard')
   const targetRef = useRef<HTMLDivElement | null>(null)
-  const { inactiveTutorials } = useTutorialContext()
   const {
     tryShowingPopup: tryShowingPopupThemedDashboardIntro,
     showPopup: showingThemedDashboardIntro,
     completeTutorial: completeThemedDashboardIntro,
     dismissTutorial,
+    checkCompletion: checkThemedDashboardIntroCompletion,
   } = useTutorial(THEMED_DASHBOARD_TUTORIAL_KEY, {
     name: THEMED_DASHBOARD_TUTORIAL_KEY,
   })
@@ -21,13 +20,14 @@ export const useThemedDashboardIntro = () => {
     dismissTutorial()
   }, [dismissTutorial])
   useEffect(() => {
-    if (
-      themedDsNav &&
-      !inactiveTutorials.includes(THEMED_DASHBOARD_TUTORIAL_KEY)
-    ) {
+    if (themedDsNav && !checkThemedDashboardIntroCompletion()) {
       tryShowingPopupThemedDashboardIntro()
     }
-  }, [inactiveTutorials, tryShowingPopupThemedDashboardIntro, themedDsNav])
+  }, [
+    checkThemedDashboardIntroCompletion,
+    tryShowingPopupThemedDashboardIntro,
+    themedDsNav,
+  ])
 
   return {
     targetRef,

+ 12 - 2
services/web/frontend/js/shared/hooks/promotions/use-tutorial.tsx

@@ -21,8 +21,17 @@ const useTutorial = (
 ) => {
   const [showPopup, setShowPopup] = useState(false)
 
-  const { deactivateTutorial, currentPopup, setCurrentPopup } =
-    useTutorialContext()
+  const {
+    deactivateTutorial,
+    currentPopup,
+    setCurrentPopup,
+    inactiveTutorials,
+  } = useTutorialContext()
+
+  const checkCompletion = useCallback(
+    () => inactiveTutorials.includes(tutorialKey),
+    [inactiveTutorials, tutorialKey]
+  )
 
   const completeTutorial = useCallback(
     async (
@@ -113,6 +122,7 @@ const useTutorial = (
     clearAndShow,
     showPopup,
     hideUntilReload,
+    checkCompletion,
   }
 }
 

+ 0 - 1
services/web/test/frontend/helpers/editor-providers.tsx

@@ -249,7 +249,6 @@ export function EditorProviders({
     LayoutProvider: makeLayoutProvider(layoutContext),
     ProjectProvider: makeProjectProvider(project),
     ReferencesProvider: makeReferencesProvider(),
-    TutorialProvider: makeTutorialProvider(),
     ...providers,
   }
 

+ 163 - 0
services/web/test/frontend/shared/hooks/use-tutorial.spec.tsx

@@ -0,0 +1,163 @@
+import useTutorial from '@/shared/hooks/promotions/use-tutorial'
+import { useEffect, useState } from 'react'
+import {
+  EditorProviders,
+  makeTutorialProvider,
+} from '../../helpers/editor-providers'
+
+const TutorialTester = ({
+  tutorial,
+  failSilently,
+}: {
+  tutorial: string
+  failSilently?: boolean
+}) => {
+  const {
+    tryShowingPopup,
+    dismissTutorial,
+    checkCompletion,
+    showPopup,
+    completeTutorial,
+  } = useTutorial(tutorial)
+  const [error, setError] = useState(false)
+
+  useEffect(() => {
+    if (!checkCompletion()) {
+      tryShowingPopup()
+    }
+  }, [checkCompletion, tryShowingPopup])
+
+  if (error) {
+    return <div>{tutorial} error</div>
+  }
+
+  if (!showPopup) {
+    return null
+  }
+
+  return (
+    <div>
+      <p>{tutorial} active</p>
+      <button onClick={() => dismissTutorial('promo-dismiss')}>Dismiss</button>
+      <button
+        onClick={() =>
+          completeTutorial(
+            { action: 'complete', event: 'promo-click' },
+            { failSilently }
+          ).catch(_ => {
+            setError(true)
+          })
+        }
+      >
+        Complete
+      </button>
+    </div>
+  )
+}
+
+describe('useTutorial', function () {
+  beforeEach(function () {
+    cy.intercept('POST', '/tutorial/test-tutorial/complete', {
+      statusCode: 200,
+    }).as('completeTutorial')
+  })
+
+  describe('with a tutorial that is not completed', function () {
+    it('shows the popup', function () {
+      cy.mount(
+        <EditorProviders>
+          <TutorialTester tutorial="test-tutorial" />
+        </EditorProviders>
+      )
+
+      cy.findByText('test-tutorial active').should('be.visible')
+    })
+
+    it('dismisses the popup', function () {
+      cy.mount(
+        <EditorProviders>
+          <TutorialTester tutorial="test-tutorial" />
+        </EditorProviders>
+      )
+      cy.findByRole('button', { name: 'Dismiss' }).click()
+      cy.findByText('test-tutorial active').should('not.exist')
+      cy.wait('@completeTutorial')
+    })
+
+    it('completes the tutorial', function () {
+      cy.mount(
+        <EditorProviders>
+          <TutorialTester tutorial="test-tutorial" />
+        </EditorProviders>
+      )
+      cy.findByRole('button', { name: 'Complete' }).click()
+      cy.findByText('test-tutorial active').should('not.exist')
+      cy.wait('@completeTutorial')
+    })
+  })
+
+  describe('with a tutorial that is already completed', function () {
+    it('does not show the popup', function () {
+      cy.mount(
+        <EditorProviders
+          providers={{
+            TutorialProvider: makeTutorialProvider({
+              inactiveTutorials: ['test-tutorial'],
+            }),
+          }}
+        >
+          <TutorialTester tutorial="test-tutorial" />
+        </EditorProviders>
+      )
+      cy.findByText('test-tutorial active').should('not.exist')
+    })
+  })
+
+  describe('with a tutorial that fails to complete', function () {
+    it('fails silently by default', function () {
+      cy.intercept('POST', '/tutorial/test-tutorial/complete', {
+        statusCode: 500,
+      }).as('completeTutorialFailure')
+
+      cy.mount(
+        <EditorProviders>
+          <TutorialTester tutorial="test-tutorial" />
+        </EditorProviders>
+      )
+      cy.findByRole('button', { name: 'Complete' }).click()
+      cy.findByText('test-tutorial active').should('not.exist')
+      cy.wait('@completeTutorialFailure')
+    })
+
+    it('throws an error if failSilently is set to false', function () {
+      cy.intercept('POST', '/tutorial/test-tutorial/complete', {
+        statusCode: 500,
+      }).as('completeTutorialFailure')
+
+      cy.mount(
+        <EditorProviders>
+          <TutorialTester tutorial="test-tutorial" failSilently={false} />
+        </EditorProviders>
+      )
+      cy.findByRole('button', { name: 'Complete' }).click()
+      cy.wait('@completeTutorialFailure')
+      cy.findByText('test-tutorial error').should('be.visible')
+      cy.findByText('test-tutorial active').should('not.exist')
+    })
+  })
+
+  describe('for two tutorials at the same time', function () {
+    // FIXME: This should work, but doesn't.
+    // eslint-disable-next-line mocha/no-skipped-tests
+    it.skip('only shows one popup at a time', function () {
+      cy.mount(
+        <EditorProviders>
+          <TutorialTester tutorial="test-tutorial-1" />
+          <TutorialTester tutorial="test-tutorial-2" />
+        </EditorProviders>
+      )
+
+      cy.findAllByText(/active/).should('have.length', 1)
+    })
+  })
+})