From 80075015741ad4c8bf25242110f47bdb16471480 Mon Sep 17 00:00:00 2001 From: Alejandro Bailo <59607668+alejandrobailo@users.noreply.github.com> Date: Thu, 3 Sep 2026 10:23:24 +0200 Subject: [PATCH] fix(ui): avoid missing selector in scan tour (#12705) --- .../view-first-scan-tour-selector.fixed.md | 1 + ui/components/scans/scans-page-shell.test.tsx | 57 +++++++++++++++++ ui/components/scans/scans-page-shell.tsx | 9 +-- .../tours/__tests__/use-driver-tour.test.tsx | 63 ++++++++++++++++++- .../__tests__/view-first-scan.tour.test.ts | 13 ++++ ui/lib/tours/tour-types.ts | 3 + ui/lib/tours/use-driver-tour.ts | 7 ++- ui/lib/tours/view-first-scan.tour.ts | 6 +- ui/scripts/check-tour-alignment.mjs | 42 ++++++++----- ui/scripts/check-tour-alignment.test.ts | 37 +++++++++++ 10 files changed, 214 insertions(+), 24 deletions(-) create mode 100644 ui/changelog.d/view-first-scan-tour-selector.fixed.md create mode 100644 ui/scripts/check-tour-alignment.test.ts diff --git a/ui/changelog.d/view-first-scan-tour-selector.fixed.md b/ui/changelog.d/view-first-scan-tour-selector.fixed.md new file mode 100644 index 0000000000..13f56ac446 --- /dev/null +++ b/ui/changelog.d/view-first-scan-tour-selector.fixed.md @@ -0,0 +1 @@ +Scan Jobs onboarding tour no longer targets an unmounted In Progress row from other tabs diff --git a/ui/components/scans/scans-page-shell.test.tsx b/ui/components/scans/scans-page-shell.test.tsx index b702f2cdf7..8d67c0d9da 100644 --- a/ui/components/scans/scans-page-shell.test.tsx +++ b/ui/components/scans/scans-page-shell.test.tsx @@ -2,6 +2,7 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { afterEach, describe, expect, it, vi } from "vitest"; +import type { OnboardingFlow } from "@/lib/onboarding"; import { useScansStore } from "@/store"; import { ProviderProps } from "@/types"; @@ -115,6 +116,18 @@ vi.mock("@/components/onboarding", () => ({ PageReady: () =>
, })); +interface OnboardingTriggerProps { + flow: OnboardingFlow; +} + +const getTriggeredTourTargets = () => { + const triggerProps = onboardingTriggerSpy.mock.calls.at(-1)?.[0] as + | OnboardingTriggerProps + | undefined; + + return triggerProps?.flow.tour.steps.map((step) => step.target); +}; + const providers: ProviderProps[] = [ { id: "provider-1", @@ -594,6 +607,50 @@ describe("ScansPageShell", () => { expect(screen.getByTestId("onboarding-trigger")).toBeInTheDocument(); }); + it("uses only mounted tour targets when an active scan exists on the completed tab", () => { + // Given + vi.stubEnv("UI_CLOUD_ENABLED", "false"); + searchParamsValue.current = "tab=completed"; + + // When + render( + +
Scans table
+
, + ); + + // Then + expect(getTriggeredTourTargets()).toEqual([undefined, "launch", "tabs"]); + }); + + it("targets the running scan when its row is mounted on the in progress tab", () => { + // Given + vi.stubEnv("UI_CLOUD_ENABLED", "false"); + searchParamsValue.current = "tab=active"; + + // When + render( + +
Scans table
+
, + ); + + // Then + expect(getTriggeredTourTargets()).toEqual([ + undefined, + "in-progress", + "launch", + ]); + }); + it("suppresses the view-first-scan tour when no provider is connected, since Launch Scan is disabled", () => { vi.stubEnv("UI_CLOUD_ENABLED", "false"); diff --git a/ui/components/scans/scans-page-shell.tsx b/ui/components/scans/scans-page-shell.tsx index 6af2c436fd..1575c026fc 100644 --- a/ui/components/scans/scans-page-shell.tsx +++ b/ui/components/scans/scans-page-shell.tsx @@ -80,9 +80,10 @@ export function ScansPageShell({ const launchDisabled = !hasManageScansPermission || !hasConnectedProviders; const launchOpen = !launchDisabled && (isLaunchScanModalOpen || urlLaunchOpen); - // When a scan is already running, the tour highlights its row (anchored in - // ScanJobsTable); otherwise it falls back to the Launch Scan button + tabs. - const hasInProgressScan = activeScanCount > 0; + // ScanJobsTable only mounts the in-progress row anchor on the active tab. + // Other tabs use the fallback tour so every target exists in the current DOM. + const hasVisibleInProgressScan = + activeScanCount > 0 && filters.activeTab === SCAN_JOBS_TAB.ACTIVE; const getTabLabel = (tab: ScanJobsTab) => { const label = SCAN_TAB_LABELS[tab]; @@ -122,7 +123,7 @@ export function ScansPageShell({ diff --git a/ui/lib/tours/__tests__/use-driver-tour.test.tsx b/ui/lib/tours/__tests__/use-driver-tour.test.tsx index f9a523d80c..dd928b2d62 100644 --- a/ui/lib/tours/__tests__/use-driver-tour.test.tsx +++ b/ui/lib/tours/__tests__/use-driver-tour.test.tsx @@ -1,5 +1,5 @@ import { render } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { addProviderTour } from "../add-provider.tour"; import type { TourCompletionRecord } from "../tour-types"; @@ -94,3 +94,64 @@ describe("adaptStep autoAdvance", () => { expect(driveStep.popover?.showButtons).toBeUndefined(); }); }); + +describe("adaptStep selector fallback", () => { + afterEach(() => { + document.body.replaceChildren(); + }); + + it("uses the fallback target when the primary target disappears", () => { + // Given + const fallbackElement = document.createElement("div"); + fallbackElement.dataset.tourId = "view-first-scan-tabs"; + document.body.append(fallbackElement); + const driveStep = adaptStep("view-first-scan", { + target: "in-progress", + fallbackTarget: "tabs", + title: "Your scan is running", + }); + + // When + const resolvedElement = (driveStep.element as () => Element)(); + + // Then + expect(resolvedElement).toBe(fallbackElement); + }); + + it("keeps the primary target when both targets exist", () => { + // Given + const primaryElement = document.createElement("div"); + primaryElement.dataset.tourId = "view-first-scan-in-progress"; + const fallbackElement = document.createElement("div"); + fallbackElement.dataset.tourId = "view-first-scan-tabs"; + document.body.append(primaryElement, fallbackElement); + const driveStep = adaptStep("view-first-scan", { + target: "in-progress", + fallbackTarget: "tabs", + title: "Your scan is running", + }); + + // When + const resolvedElement = (driveStep.element as () => Element)(); + + // Then + expect(resolvedElement).toBe(primaryElement); + }); + + it("still reports configuration drift when both targets are missing", () => { + // Given + const driveStep = adaptStep("view-first-scan", { + target: "in-progress", + fallbackTarget: "tabs", + title: "Your scan is running", + }); + + // When + const resolveElement = () => (driveStep.element as () => Element)(); + + // Then + expect(resolveElement).toThrow( + 'Tour "view-first-scan" references missing selector: [data-tour-id="view-first-scan-in-progress"]', + ); + }); +}); diff --git a/ui/lib/tours/__tests__/view-first-scan.tour.test.ts b/ui/lib/tours/__tests__/view-first-scan.tour.test.ts index 8fead9fd33..2f161d87db 100644 --- a/ui/lib/tours/__tests__/view-first-scan.tour.test.ts +++ b/ui/lib/tours/__tests__/view-first-scan.tour.test.ts @@ -72,6 +72,19 @@ describe("buildViewFirstScanTour with a running scan", () => { expect(tour.version).toBe(viewFirstScanTour.version); }); + it("falls back to the stable tabs anchor if the running row disappears", () => { + // Given + const runningScanStep = tour.steps.find( + (step) => step.target === "in-progress", + ); + + // When + const fallbackTarget = runningScanStep?.fallbackTarget; + + // Then + expect(fallbackTarget).toBe("tabs"); + }); + it("never targets an element outside the allowed anchor set", () => { for (const target of definedTargets(tour)) { expect(ALLOWED_TARGETS).toContain(target); diff --git a/ui/lib/tours/tour-types.ts b/ui/lib/tours/tour-types.ts index d443f21489..ff0d97a646 100644 --- a/ui/lib/tours/tour-types.ts +++ b/ui/lib/tours/tour-types.ts @@ -45,6 +45,9 @@ export interface TourCompletionRecord { // Modal step omits `target`; anchored step provides the `data-tour-id` value (no brackets). export interface TourStep { target?: TTarget; + // Optional stable anchor used when a volatile primary target disappears before + // driver.js resolves the step (for example, a running scan row that completes). + fallbackTarget?: TTarget; title?: string; description?: string; side?: TourStepSide; diff --git a/ui/lib/tours/use-driver-tour.ts b/ui/lib/tours/use-driver-tour.ts index a0ab78523b..d7228cd303 100644 --- a/ui/lib/tours/use-driver-tour.ts +++ b/ui/lib/tours/use-driver-tour.ts @@ -172,11 +172,16 @@ export function adaptStep( if (step.target) { const selector = toSelector(`${tourId}-${step.target}`); + const fallbackSelector = step.fallbackTarget + ? toSelector(`${tourId}-${step.fallbackTarget}`) + : undefined; driveStep.element = () => { if (typeof document === "undefined") { throw new Error("Tour element resolved without a DOM"); } - const found = document.querySelector(selector); + const found = + document.querySelector(selector) ?? + (fallbackSelector ? document.querySelector(fallbackSelector) : null); if (!found) { throw new Error( `Tour "${tourId}" references missing selector: ${selector}`, diff --git a/ui/lib/tours/view-first-scan.tour.ts b/ui/lib/tours/view-first-scan.tour.ts index c6b5a00d8d..f8ee778ac1 100644 --- a/ui/lib/tours/view-first-scan.tour.ts +++ b/ui/lib/tours/view-first-scan.tour.ts @@ -33,9 +33,8 @@ const INTRO_STEP = { /** * Builds the tour for the scans page. When a scan is already running we anchor the * In Progress row first (and mention the other tabs in copy); otherwise we fall back - * to highlighting Launch Scan and the tabs. Gating on `hasInProgressScan` keeps the - * tour from anchoring to a missing row — the same guard pattern the findings tour - * uses for an empty table. + * to highlighting Launch Scan and the tabs. The volatile row step itself falls back + * to the stable tabs anchor if the scan completes while the tour is open. */ export function buildViewFirstScanTour( hasInProgressScan: boolean, @@ -49,6 +48,7 @@ export function buildViewFirstScanTour( INTRO_STEP, { target: "in-progress", + fallbackTarget: "tabs", side: TOUR_STEP_SIDES.BOTTOM, align: TOUR_STEP_ALIGNMENTS.START, title: "Your scan is running", diff --git a/ui/scripts/check-tour-alignment.mjs b/ui/scripts/check-tour-alignment.mjs index 12863fd303..77de2bd9b1 100644 --- a/ui/scripts/check-tour-alignment.mjs +++ b/ui/scripts/check-tour-alignment.mjs @@ -1,9 +1,10 @@ #!/usr/bin/env node -// Tour alignment check (syntactic). Extracts `data-tour-id` values from every -// `ui/lib/tours/*.tour.ts` and verifies a matching attribute exists under `ui/`, -// in either the JSX form (`data-tour-id="..."`) or the object-property form -// (`"data-tour-id": "..."`) used for dynamically-spread anchors. Two directions: -// - Tour → DOM: fails on any tour `target` with no matching attribute. +// Tour alignment check (syntactic). Extracts primary and fallback `data-tour-id` +// values from every `ui/lib/tours/*.tour.ts` and verifies a matching attribute +// exists under `ui/`, in either the JSX form (`data-tour-id="..."`) or the +// object-property form (`"data-tour-id": "..."`) used for dynamically-spread +// anchors. Two directions: +// - Tour → DOM: fails on any `target` or `fallbackTarget` without an attribute. // - DOM → tour: warns on any `data-tour-id` not referenced by any tour // (does not fail — staged anchors during multi-PR rollouts are OK). // Complements the semantic `prowler-tour` skill for CI/local runs without @@ -41,7 +42,18 @@ async function findTourFiles(dir) { } const TOUR_ID_PATTERN = /\bid\s*:\s*["']([a-z0-9-]+)["']/m; -const TARGET_PATTERN = /\btarget\s*:\s*["']([a-z0-9-]+)["']/g; +const TARGET_PATTERN = + /\b(?:target|fallbackTarget)\s*:\s*["']([a-z0-9-]+)["']/g; + +/** + * Extracts every primary and fallback target declared by a tour. + * + * @param {string} source + * @returns {string[]} + */ +export function extractTourTargets(source) { + return Array.from(source.matchAll(TARGET_PATTERN), (match) => match[1]); +} async function parseTour(filePath) { const source = await readFile(filePath, "utf8"); @@ -53,10 +65,7 @@ async function parseTour(filePath) { } const tourId = idMatch[1]; - const targets = []; - for (const match of source.matchAll(TARGET_PATTERN)) { - targets.push(match[1]); - } + const targets = extractTourTargets(source); return { file: relative(UI_DIR, filePath), @@ -155,7 +164,7 @@ async function main() { if (tourOrphans.length === 0) { const referenced = tours.reduce((sum, t) => sum + t.selectors.length, 0); console.log( - `✓ Tour alignment OK — ${tours.length} tour(s), ${referenced} anchored step(s).`, + `✓ Tour alignment OK — ${tours.length} tour(s), ${referenced} anchor reference(s).`, ); return; } @@ -172,7 +181,10 @@ async function main() { process.exit(1); } -main().catch((err) => { - console.error(err.stack || err.message); - process.exit(1); -}); +const entryPoint = process.argv[1]; +if (entryPoint && resolve(entryPoint) === fileURLToPath(import.meta.url)) { + main().catch((err) => { + console.error(err.stack || err.message); + process.exit(1); + }); +} diff --git a/ui/scripts/check-tour-alignment.test.ts b/ui/scripts/check-tour-alignment.test.ts new file mode 100644 index 0000000000..680dddd5a0 --- /dev/null +++ b/ui/scripts/check-tour-alignment.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; + +import { extractTourTargets } from "./check-tour-alignment.mjs"; + +describe("tour target extraction", () => { + it("should include primary and fallback targets", () => { + // Given + const source = ` + { + target: "volatile-row", + fallbackTarget: "stable-tabs", + } + `; + + // When + const targets = extractTourTargets(source); + + // Then + expect(targets).toEqual(["volatile-row", "stable-tabs"]); + }); + + it("should include a target used only as a fallback", () => { + // Given + const source = ` + { + fallbackTarget: "fallback-only-anchor", + title: "Fallback-only step", + } + `; + + // When + const targets = extractTourTargets(source); + + // Then + expect(targets).toEqual(["fallback-only-anchor"]); + }); +});