diff --git a/services/web/frontend/js/shared/context/tutorial-context.tsx b/services/web/frontend/js/shared/context/tutorial-context.tsx index 8f6b6d0890..c09e0857a3 100644 --- a/services/web/frontend/js/shared/context/tutorial-context.tsx +++ b/services/web/frontend/js/shared/context/tutorial-context.tsx @@ -1,12 +1,12 @@ import getMeta from '@/utils/meta' import { createContext, - Dispatch, FC, - SetStateAction, + MutableRefObject, useCallback, useContext, useMemo, + useRef, useState, } from 'react' @@ -15,7 +15,8 @@ export const TutorialContext = createContext< deactivateTutorial: (tutorial: string) => void inactiveTutorials: string[] currentPopup: string | null - setCurrentPopup: Dispatch> + currentPopupRef: MutableRefObject + setCurrentPopup: (value: string | null) => void } | undefined >(undefined) @@ -25,7 +26,13 @@ export const TutorialProvider: FC = ({ children }) => { () => getMeta('ol-inactiveTutorials') || [] ) - const [currentPopup, setCurrentPopup] = useState(null) + const [currentPopup, setCurrentPopupState] = useState(null) + const currentPopupRef = useRef(null) + + const setCurrentPopup = useCallback((value: string | null) => { + currentPopupRef.current = value + setCurrentPopupState(value) + }, []) const deactivateTutorial = useCallback( (tutorialKey: string) => { @@ -39,6 +46,7 @@ export const TutorialProvider: FC = ({ children }) => { deactivateTutorial, inactiveTutorials, currentPopup, + currentPopupRef, setCurrentPopup, }), [deactivateTutorial, inactiveTutorials, currentPopup, setCurrentPopup] diff --git a/services/web/frontend/js/shared/hooks/promotions/use-tutorial.tsx b/services/web/frontend/js/shared/hooks/promotions/use-tutorial.tsx index 40016cff9f..f371367680 100644 --- a/services/web/frontend/js/shared/hooks/promotions/use-tutorial.tsx +++ b/services/web/frontend/js/shared/hooks/promotions/use-tutorial.tsx @@ -24,15 +24,28 @@ const useTutorial = ( const { deactivateTutorial, currentPopup, + currentPopupRef, setCurrentPopup, inactiveTutorials, } = useTutorialContext() const checkCompletion = useCallback( () => inactiveTutorials.includes(tutorialKey), - [inactiveTutorials, tutorialKey] + // currentPopup is a dependency so consumers re-run their effect when the + // popup slot frees up and can retry tryShowingPopup. + // eslint-disable-next-line react-hooks/exhaustive-deps + [inactiveTutorials, tutorialKey, currentPopup] ) + const clearPopup = useCallback(() => { + // popups should only clear themselves, in cases they need to cleanup or shouldnt show anymore + // allow forcing the clear if needed, eg: higher prio alert needs to show + if (currentPopupRef.current === tutorialKey) { + setCurrentPopup(null) + setShowPopup(false) + } + }, [currentPopupRef, setCurrentPopup, setShowPopup, tutorialKey]) + const completeTutorial = useCallback( async ( { @@ -52,10 +65,10 @@ const useTutorial = ( } debugConsole.error(err) } - setShowPopup(false) deactivateTutorial(tutorialKey) + clearPopup() }, - [deactivateTutorial, eventData, tutorialKey] + [deactivateTutorial, eventData, tutorialKey, clearPopup] ) const dismissTutorial = useCallback( @@ -79,26 +92,19 @@ const useTutorial = ( // try to show the popup if we don't already have one showing, returns true if it can show, false if it can't const tryShowingPopup = useCallback( (eventName: string = 'promo-prompt') => { - if (currentPopup === null) { - setCurrentPopup(tutorialKey) - setShowPopup(true) - eventTracking.sendMB(eventName, eventData) - return true + // Check the lock via ref so concurrent callers in the same render see + // each other's claim; useState would batch and both would read stale null. + if (currentPopupRef.current !== null) { + return false } - return false + setCurrentPopup(tutorialKey) + setShowPopup(true) + eventTracking.sendMB(eventName, eventData) + return true }, - [currentPopup, setCurrentPopup, tutorialKey, eventData] + [currentPopupRef, setCurrentPopup, tutorialKey, eventData] ) - const clearPopup = useCallback(() => { - // popups should only clear themselves, in cases they need to cleanup or shouldnt show anymore - // allow forcing the clear if needed, eg: higher prio alert needs to show - if (currentPopup === tutorialKey) { - setCurrentPopup(null) - setShowPopup(false) - } - }, [setCurrentPopup, setShowPopup, currentPopup, tutorialKey]) - const clearAndShow = useCallback( (eventName: string = 'promo-prompt') => { setCurrentPopup(tutorialKey) diff --git a/services/web/test/frontend/helpers/make-tutorial-provider.tsx b/services/web/test/frontend/helpers/make-tutorial-provider.tsx index 01dff51702..0ab9a3a041 100644 --- a/services/web/test/frontend/helpers/make-tutorial-provider.tsx +++ b/services/web/test/frontend/helpers/make-tutorial-provider.tsx @@ -2,6 +2,7 @@ import React, { type FC, type PropsWithChildren, useCallback, + useRef, useState, } from 'react' import { TutorialContext } from '@/shared/context/tutorial-context' @@ -13,6 +14,7 @@ export const makeTutorialProvider = (opts?: { const [inactiveTutorials, setInactiveTutorials] = useState( opts?.inactiveTutorials ?? [] ) + const currentPopupRef = useRef(null) const deactivateTutorial = useCallback((key: string) => { setInactiveTutorials(prev => (prev.includes(key) ? prev : [...prev, key])) }, []) @@ -20,6 +22,7 @@ export const makeTutorialProvider = (opts?: { deactivateTutorial, inactiveTutorials, currentPopup: null, + currentPopupRef, setCurrentPopup: () => {}, } return ( diff --git a/services/web/test/frontend/shared/hooks/use-tutorial.spec.tsx b/services/web/test/frontend/shared/hooks/use-tutorial.spec.tsx index 21329bef06..d57db06e62 100644 --- a/services/web/test/frontend/shared/hooks/use-tutorial.spec.tsx +++ b/services/web/test/frontend/shared/hooks/use-tutorial.spec.tsx @@ -145,9 +145,16 @@ describe('useTutorial', function () { }) 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 () { + beforeEach(function () { + cy.intercept('POST', '/tutorial/test-tutorial-1/complete', { + statusCode: 200, + }).as('completeTutorial1') + cy.intercept('POST', '/tutorial/test-tutorial-2/complete', { + statusCode: 200, + }).as('completeTutorial2') + }) + + it('only shows one popup at a time', function () { cy.mount( @@ -157,5 +164,23 @@ describe('useTutorial', function () { cy.findAllByText(/active/).should('have.length', 1) }) + + it('shows the second popup after the first is completed', function () { + cy.mount( + + + + + ) + + cy.findByText('test-tutorial-1 active').should('be.visible') + cy.findByText('test-tutorial-2 active').should('not.exist') + + cy.findByRole('button', { name: 'Complete' }).click() + cy.wait('@completeTutorial1') + + cy.findByText('test-tutorial-1 active').should('not.exist') + cy.findByText('test-tutorial-2 active').should('be.visible') + }) }) })