Merge pull request #14577 from overleaf/as-fix-popover

Fix history UI tutorial so that it is correctly dismissed

GitOrigin-RevId: f5792db1531759815fd22c07d253649bedcf3dbd

services/web/frontend/extracted-translations.json

@@ -799,7 +799,6 @@
   "raw_logs": "",
   "raw_logs_description": "",
   "react_history_tutorial_content": "",
-  "react_history_tutorial_learn_more": "",
   "react_history_tutorial_title": "",
   "reactivate_subscription": "",
   "read_only": "",

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

@@ -1,4 +1,4 @@
-import { useEffect, useRef, useState } from 'react'
+import { useCallback, useEffect, useRef, useState } from 'react'
 import HistoryVersion from './history-version'
 import LoadingSpinner from '../../../../shared/components/loading-spinner'
 import { OwnerPaywallPrompt } from './owner-paywall-prompt'
@@ -10,11 +10,15 @@ import {
 import { useUserContext } from '../../../../shared/context/user-context'
 import useDropdownActiveItem from '../../hooks/use-dropdown-active-item'
 import { useHistoryContext } from '../../context/history-context'
-import getMeta from '../../../../utils/meta'
+import { useEditorContext } from '../../../../shared/context/editor-context'
 
 type CompletedTutorials = {
   'react-history-buttons-tutorial': Date
 }
+type EditorTutorials = {
+  completedTutorials: CompletedTutorials
+  setCompletedTutorial: (key: string) => void
+}
 
 const unselectedStates: ItemSelectionState[] = [
   'aboveSelected',
@@ -97,14 +101,15 @@ function AllHistoryList() {
     }
   }, [updatesLoadingState])
 
-  const completedTutorials: CompletedTutorials = getMeta(
-    'ol-completedTutorials'
-  )
+  const { completedTutorials, setCompletedTutorial }: EditorTutorials =
+    useEditorContext()
 
   // only show tutorial popover if they havent dismissed ("completed") it yet
-  const hasCompletedHistTutorial = Boolean(
-    completedTutorials?.['react-history-buttons-tutorial']
-  )
+  const showTutorial = !completedTutorials?.['react-history-buttons-tutorial']
+
+  const completeTutorial = useCallback(() => {
+    setCompletedTutorial('react-history-buttons-tutorial')
+  }, [setCompletedTutorial])
 
   // only show tutorial popover on the first icon
   const firstUnselectedIndex = visibleUpdates.findIndex(update => {
@@ -116,8 +121,6 @@ function AllHistoryList() {
     return unselectedStates.includes(selectionState)
   })
 
-  const [showTutorial, setShowTutorial] = useState(!hasCompletedHistTutorial)
-
   return (
     <div ref={scrollerRef} className="history-all-versions-scroller">
       <div className="history-all-versions-container">
@@ -167,7 +170,7 @@ function AllHistoryList() {
               }
               dropdownActive={dropdownActive}
               hasTutorialOverlay={hasTutorialOverlay}
-              setShowTutorial={setShowTutorial}
+              completeTutorial={completeTutorial}
             />
           )
         })}

services/web/frontend/js/features/history/components/change-list/history-version.tsx

@@ -49,7 +49,7 @@ type HistoryVersionProps = {
   setActiveDropdownItem: ActiveDropdown['setActiveDropdownItem']
   closeDropdownForItem: ActiveDropdown['closeDropdownForItem']
   hasTutorialOverlay?: boolean
-  setShowTutorial: (show: boolean) => void
+  completeTutorial: () => void
 }
 
 function HistoryVersion({
@@ -68,7 +68,7 @@ function HistoryVersion({
   setActiveDropdownItem,
   closeDropdownForItem,
   hasTutorialOverlay = false,
-  setShowTutorial,
+  completeTutorial,
 }: HistoryVersionProps) {
   const orderedLabels = orderBy(update.labels, ['created_at'], ['desc'])
   const iconRef = useRef<HTMLDivElement>(null)
@@ -81,10 +81,16 @@ function HistoryVersion({
   // wait for the layout to settle before showing popover, to avoid a flash/ instant move
   const [layoutSettled, setLayoutSettled] = useState(false)
 
+  const [resizing, setResizing] = useState(false)
+
+  // Determine whether the tutorial popover should be shown or not.
+  // This is a slightly unusual pattern, as in theory we could control this via
+  // the `show` prop. However we were concerned about the perf impact of every
+  // history version having a (hidden) popover that won't ever be triggered.
   useEffect(() => {
-    if (iconRef.current && hasTutorialOverlay && layoutSettled) {
+    if (iconRef.current && hasTutorialOverlay && layoutSettled && !resizing) {
       const dismissModal = () => {
-        setShowTutorial(false)
+        completeTutorial()
         runAsync(completeHistoryTutorial()).catch(console.error)
       }
 
@@ -98,7 +104,7 @@ function HistoryVersion({
       setPopover(
         <Overlay
           placement="left"
-          show={hasTutorialOverlay}
+          show
           target={iconRef.current ?? undefined}
           shouldUpdatePosition
         >
@@ -114,17 +120,25 @@ function HistoryVersion({
           >
             <Trans
               i18nKey="react_history_tutorial_content"
-              components={[compareIcon]} // eslint-disable-line react/jsx-key
+              components={[
+                compareIcon,
+                <a href="https://www.overleaf.com/learn/latex/Using_the_History_feature" />, // eslint-disable-line jsx-a11y/anchor-has-content, react/jsx-key
+              ]}
             />
-            <a href="https://www.overleaf.com/learn/latex/Using_the_History_feature">
-              {' '}
-              {t('react_history_tutorial_learn_more')}
-            </a>
           </Popover>
         </Overlay>
       )
+    } else {
+      setPopover(null)
     }
-  }, [hasTutorialOverlay, runAsync, setShowTutorial, t, layoutSettled])
+  }, [
+    hasTutorialOverlay,
+    runAsync,
+    t,
+    layoutSettled,
+    completeTutorial,
+    resizing,
+  ])
 
   // give the components time to position before showing popover so we dont get a instant position change
   useEffect(() => {
@@ -144,11 +158,11 @@ function HistoryVersion({
       if (timer) {
         window.clearTimeout(timer)
       } else {
-        setShowTutorial(false)
+        setResizing(true)
       }
       timer = window.setTimeout(() => {
         timer = null
-        setShowTutorial(true)
+        setResizing(false)
       }, 500)
     }
 
@@ -157,7 +171,7 @@ function HistoryVersion({
       window.addEventListener('resize', handleResize)
       return () => window.removeEventListener('resize', handleResize)
     }
-  }, [hasTutorialOverlay, setShowTutorial])
+  }, [hasTutorialOverlay])
 
   const closeDropdown = useCallback(() => {
     closeDropdownForItem(update, 'moreOptions')
@@ -167,7 +181,7 @@ function HistoryVersion({
 
   return (
     <>
-      {hasTutorialOverlay && popover}
+      {popover}
       {showDivider ? (
         <div
           className={classNames({

services/web/frontend/js/shared/context/editor-context.js

@@ -4,6 +4,7 @@ import {
   useContext,
   useEffect,
   useMemo,
+  useState,
 } from 'react'
 import PropTypes from 'prop-types'
 import useScopeValue from '../hooks/use-scope-value'
@@ -79,6 +80,20 @@ export function EditorProvider({ children, settings }) {
   const [showSymbolPalette] = useScopeValue('editor.showSymbolPalette')
   const [toggleSymbolPalette] = useScopeValue('editor.toggleSymbolPalette')
 
+  const [completedTutorials, setCompletedTutorials] = useState(() =>
+    getMeta('ol-completedTutorials')
+  )
+
+  const setCompletedTutorial = useCallback(
+    tutorialKey => {
+      setCompletedTutorials({
+        ...completedTutorials,
+        [tutorialKey]: new Date(),
+      })
+    },
+    [completedTutorials]
+  )
+
   useEffect(() => {
     if (ide?.socket) {
       ide.socket.on('projectNameUpdated', setProjectName)
@@ -151,6 +166,8 @@ export function EditorProvider({ children, settings }) {
       showSymbolPalette,
       toggleSymbolPalette,
       insertSymbol,
+      completedTutorials,
+      setCompletedTutorial,
     }),
     [
       cobranding,
@@ -163,6 +180,8 @@ export function EditorProvider({ children, settings }) {
       showSymbolPalette,
       toggleSymbolPalette,
       insertSymbol,
+      completedTutorials,
+      setCompletedTutorial,
     ]
   )
 

services/web/locales/en.json

@@ -1299,8 +1299,7 @@
   "quoted_text_in": "Quoted text in",
   "raw_logs": "Raw logs",
   "raw_logs_description": "Raw logs from the LaTeX compiler",
-  "react_history_tutorial_content": "To compare a range of versions, use the  <0></0> on the versions you want at the start and end of the range. To add a label or to download a version use the options in the three-dot menu. Learn more about using Overleaf History.",
-  "react_history_tutorial_learn_more": "Learn more about using Overleaf History.",
+  "react_history_tutorial_content": "To compare a range of versions, use the  <0></0> on the versions you want at the start and end of the range. To add a label or to download a version use the options in the three-dot menu. <1>Learn more about using Overleaf History.</1>",
   "react_history_tutorial_title": "History actions have a new home",
   "reactivate_subscription": "Reactivate your subscription",
   "read_only": "Read Only",