diff --git a/packages/driver/src/context.ts b/packages/driver/src/context.ts index 0a6f70ea..37391d54 100644 --- a/packages/driver/src/context.ts +++ b/packages/driver/src/context.ts @@ -143,6 +143,9 @@ export type State = { __pendingWaitCancel?: () => void; __activeStagePosition?: StageDefinition; __overlaySvg?: SVGSVGElement; + // Marks the element this instance highlighted, so a stale destroy can + // unmark only its own target. + __instanceId?: string; __events?: { onKeyup: (e: KeyboardEvent) => void; diff --git a/packages/driver/src/driver.ts b/packages/driver/src/driver.ts index 90b01bec..b139834d 100644 --- a/packages/driver/src/driver.ts +++ b/packages/driver/src/driver.ts @@ -1,12 +1,17 @@ import { destroyPopover, Popover } from "./popover"; import { destroyOverlay } from "./overlay"; import { destroyEvents, initEvents, requireRefresh } from "./events"; -import { Config, createContext, DriverHook } from "./context"; -import { destroyHighlight, highlight } from "./highlight"; +import { Config, Context, createContext, DriverHook } from "./context"; +import { destroyHighlight, highlight, releaseHighlight } from "./highlight"; import { findReachableIndex, resolveNextHook, resolvePrevHook, resolveTourStep, shouldSkipStep } from "./step"; import { resolveElement } from "./utils"; import "./driver.css"; +// The instance that last mounted the shared overlay. A destroy on any other +// instance must not strip that overlay. +let mountedContext: Context | null = null; +let nextInstanceId = 0; + // Re-export the public types so they remain part of the package's type surface. export type { Config, DriverHook, State } from "./context"; export type { StageDefinition } from "./stage"; @@ -52,6 +57,7 @@ export interface Driver { export function driver(options: Config = {}): Driver { const ctx = createContext(options); + const instanceId = `driver-${++nextInstanceId}`; function handleClose() { if (!ctx.getConfig("allowClose")) { @@ -210,7 +216,19 @@ export function driver(options: Config = {}): Driver { return; } + // Stop a previous instance's animation from painting over this one. + if (mountedContext && mountedContext !== ctx) { + const pendingFrame = mountedContext.getState("__resizeTimeout"); + if (pendingFrame) { + window.cancelAnimationFrame(pendingFrame); + } + mountedContext.setState("__transitionCallback", undefined); + mountedContext.setState("__resizeTimeout", undefined); + } + ctx.setState("isInitialized", true); + ctx.setState("__instanceId", instanceId); + mountedContext = ctx; document.body.classList.add("driver-active", ctx.getConfig("animate") ? "driver-fade" : "driver-simple"); if (!ctx.getConfig("allowScroll")) { document.body.classList.add("driver-no-scroll"); @@ -328,16 +346,23 @@ export function driver(options: Config = {}): Driver { } function destroy(withOnDestroyStartedHook = true) { + // Already torn down, or never started. Touching the document here would + // remove the overlay that belongs to a different instance (#504). + if (!ctx.getState("isInitialized")) { + return; + } + const activeElement = ctx.getState("__activeElement"); const activeStep = ctx.getState("__activeStep"); - const activeOnDestroyed = ctx.getState("__activeOnDestroyed"); + const ownsPage = mountedContext === ctx; const onDestroyStarted = ctx.getConfig("onDestroyStarted"); // `onDestroyStarted` is used to confirm the exit of tour. If we trigger // the hook for when user calls `destroy`, driver will get into infinite loop - // not causing tour to be destroyed. - if (withOnDestroyStartedHook && onDestroyStarted) { + // not causing tour to be destroyed. A superseded instance skips the hook: + // the visible tour is no longer this one. + if (ownsPage && withOnDestroyStartedHook && onDestroyStarted) { const isActiveDummyElement = !activeElement || activeElement?.id === "driver-dummy-element"; onDestroyStarted(isActiveDummyElement ? undefined : activeElement, activeStep!, ctx.getHookOpts()); return; @@ -346,13 +371,20 @@ export function driver(options: Config = {}): Driver { const onDeselected = activeStep?.onDeselected || ctx.getConfig("onDeselected"); const onDestroyed = ctx.getConfig("onDestroyed"); - document.body.classList.remove("driver-active", "driver-fade", "driver-simple", "driver-no-scroll"); - document.body.style.removeProperty("--driver-animation-duration"); + if (ownsPage) { + document.body.classList.remove("driver-active", "driver-fade", "driver-simple", "driver-no-scroll"); + document.body.style.removeProperty("--driver-animation-duration"); + destroyHighlight(); + if (mountedContext === ctx) { + mountedContext = null; + } + } else { + releaseHighlight(activeElement, ctx.getState("__instanceId")); + } cancelElementWait(); destroyEvents(ctx); destroyPopover(ctx.getState("popover")); - destroyHighlight(); destroyOverlay(ctx); ctx.resetEmitter(); @@ -371,7 +403,7 @@ export function driver(options: Config = {}): Driver { } } - if (activeOnDestroyed) { + if (ownsPage && activeOnDestroyed) { (activeOnDestroyed as HTMLElement).focus(); } } diff --git a/packages/driver/src/highlight.ts b/packages/driver/src/highlight.ts index d6437632..ea7a72cd 100644 --- a/packages/driver/src/highlight.ts +++ b/packages/driver/src/highlight.ts @@ -167,19 +167,46 @@ function transferHighlight(ctx: Context, toElement: Element, toStep: DriveStep) toElement.setAttribute("aria-haspopup", "dialog"); toElement.setAttribute("aria-expanded", "true"); toElement.setAttribute("aria-controls", "driver-popover-content"); + const instanceId = ctx.getState("__instanceId"); + if (instanceId) { + toElement.setAttribute("data-driver-owner", instanceId); + } +} + +function unmarkHighlightedElement(element: Element) { + const parent = element.parentElement; + if (parent && parent !== document.body) { + parent.classList.remove("driver-active-element-parent", "driver-active-element-parent-no-scroll"); + } + + element.classList.remove("driver-active-element", "driver-no-interaction"); + element.removeAttribute("aria-haspopup"); + element.removeAttribute("aria-expanded"); + element.removeAttribute("aria-controls"); + element.removeAttribute("data-driver-owner"); +} + +// Drops the highlight class from this instance's element only. A later tour +// may have taken the same node; its owner id then no longer matches. +export function releaseHighlight(element: Element | undefined, instanceId: string | undefined) { + if (!element || !instanceId) { + return; + } + if (element.getAttribute("data-driver-owner") !== instanceId) { + return; + } + + if (element.id === "driver-dummy-element") { + element.remove(); + return; + } + + unmarkHighlightedElement(element); } export function destroyHighlight() { document.getElementById("driver-dummy-element")?.remove(); document.querySelectorAll(".driver-active-element").forEach(element => { - const parent = element.parentElement; - if (parent && parent !== document.body) { - parent.classList.remove("driver-active-element-parent", "driver-active-element-parent-no-scroll"); - } - - element.classList.remove("driver-active-element", "driver-no-interaction"); - element.removeAttribute("aria-haspopup"); - element.removeAttribute("aria-expanded"); - element.removeAttribute("aria-controls"); + unmarkHighlightedElement(element); }); } diff --git a/packages/driver/tests/destroy-isolation.test.ts b/packages/driver/tests/destroy-isolation.test.ts new file mode 100644 index 00000000..53e2d0a7 --- /dev/null +++ b/packages/driver/tests/destroy-isolation.test.ts @@ -0,0 +1,136 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { driver, type Driver } from "../src/driver"; +import { nextFrame, popoverEl, popoverTitle, useDriverHarness } from "./utils"; + +// #504: destroy() on a stale instance must not tear down the tour that is +// actually on the page. + +useDriverHarness(); + +const extra: Driver[] = []; +function track(d: Driver): Driver { + extra.push(d); + return d; +} +afterEach(() => { + while (extra.length) { + extra.pop()?.destroy(); + } +}); + +function liveTour(): Driver { + return track( + driver({ + animate: false, + steps: [{ element: "#intro", popover: { title: "Live tour" } }], + }) + ); +} + +function expectTourIntact(tour: Driver) { + expect(tour.isActive()).toBe(true); + expect(popoverTitle()).toBe("Live tour"); + expect(popoverEl()).not.toBeNull(); + expect(document.body.classList.contains("driver-active")).toBe(true); + expect(document.querySelector("#intro")?.classList.contains("driver-active-element")).toBe(true); +} + +describe("destroy isolation (#504)", () => { + it("ignores a second destroy after the instance already tore down", async () => { + const onDestroyed = vi.fn(); + const demo = track(driver({ animate: false, onDestroyed })); + demo.highlight({ element: "#card-1", popover: { title: "Highlight" } }); + await nextFrame(); + demo.destroy(); + + const tour = liveTour(); + tour.drive(); + await nextFrame(); + + demo.destroy(); + + expect(demo.isActive()).toBe(false); + expectTourIntact(tour); + expect(onDestroyed).toHaveBeenCalledTimes(1); + }); + + it("does not let a still-active stale instance wipe the tour that replaced it", async () => { + const demoDestroyed = vi.fn(); + const tourDestroyed = vi.fn(); + const demo = track( + driver({ + animate: false, + onDestroyed: demoDestroyed, + }) + ); + demo.highlight({ element: "#card-1", popover: { title: "Highlight" } }); + await nextFrame(); + + const tour = track( + driver({ + animate: false, + onDestroyed: tourDestroyed, + steps: [{ element: "#intro", popover: { title: "Live tour" } }], + }) + ); + tour.drive(); + await nextFrame(); + + // The highlight object is still active. That is the timeout in #504: + // isActive() is true, so the caller does call destroy(). + expect(demo.isActive()).toBe(true); + const popovers = document.querySelectorAll(".driver-popover"); + const overlays = document.querySelectorAll(".driver-overlay"); + const livePopover = popovers[popovers.length - 1]; + const liveOverlay = overlays[overlays.length - 1]; + demo.destroy(); + + expect(demo.isActive()).toBe(false); + expect(demoDestroyed).toHaveBeenCalledTimes(1); + expect(tourDestroyed).not.toHaveBeenCalled(); + const remainingPopovers = document.querySelectorAll(".driver-popover"); + const remainingOverlays = document.querySelectorAll(".driver-overlay"); + expect(remainingPopovers[remainingPopovers.length - 1]).toBe(livePopover); + expect(remainingOverlays[remainingOverlays.length - 1]).toBe(liveOverlay); + expect(document.querySelector("#card-1")?.classList.contains("driver-active-element")).toBe(false); + expectTourIntact(tour); + }); + + it("leaves the live tour alone when destroy() runs on an instance that never started", () => { + const idle = track(driver({ animate: false })); + const tour = liveTour(); + tour.drive(); + + idle.destroy(); + + expect(idle.isActive()).toBe(false); + expectTourIntact(tour); + }); + + it("can highlight again after destroy", () => { + const demo = track(driver({ animate: false })); + demo.highlight({ element: "#intro", popover: { title: "First" } }); + demo.destroy(); + demo.highlight({ element: "#card-1", popover: { title: "Second" } }); + + expect(demo.isActive()).toBe(true); + expect(popoverTitle()).toBe("Second"); + expect(document.querySelector("#card-1")?.classList.contains("driver-active-element")).toBe(true); + expect(document.querySelector("#intro")?.classList.contains("driver-active-element")).toBe(false); + }); + + it("keeps the replacement tour after an animated highlight is destroyed", async () => { + const demo = track(driver({ animate: true, duration: 50 })); + demo.highlight({ element: "#card-1", popover: { title: "Highlight" } }); + await nextFrame(); + demo.destroy(); + + const tour = liveTour(); + tour.drive(); + await nextFrame(); + demo.destroy(); + await nextFrame(); + + expectTourIntact(tour); + }); +});