From b276990f2ff3b3ca52766aa2f114ba8e24d3580d Mon Sep 17 00:00:00 2001 From: ethan Date: Fri, 12 Dec 2025 12:27:49 +1100 Subject: [PATCH 1/6] =?UTF-8?q?=F0=9F=A4=96=20fix:=20use=20aria-label=20to?= =?UTF-8?q?=20target=20experiment=20toggles=20in=20stories?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Select switches by name instead of DOM order to prevent flaky tests when new switches are added to the Settings modal. Also simplify ExperimentsToggleOff to show default state directly rather than clicking twice (on then off). --- src/browser/stories/App.settings.stories.tsx | 25 ++++---------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/src/browser/stories/App.settings.stories.tsx b/src/browser/stories/App.settings.stories.tsx index 2eb11ce889..2db67944d3 100644 --- a/src/browser/stories/App.settings.stories.tsx +++ b/src/browser/stories/App.settings.stories.tsx @@ -186,32 +186,17 @@ export const ExperimentsToggleOn: AppStory = { const modal = body.getByRole("dialog"); const modalCanvas = within(modal); - // Find the switch by its role - experiments use role="switch" - const switches = await modalCanvas.findAllByRole("switch"); - if (switches.length > 0) { - // Toggle the first experiment on - await userEvent.click(switches[0]); - } + // Find the experiment toggle by its aria-label (set in ExperimentsSection.tsx) + const toggle = await modalCanvas.findByRole("switch", { name: /Post-Compaction Context/i }); + await userEvent.click(toggle); }, }; -/** Experiments section - toggle experiment off (starts enabled, then toggles off) */ +/** Experiments section - shows experiment in OFF state (default) */ export const ExperimentsToggleOff: AppStory = { render: () => setupSettingsStory({})} />, play: async ({ canvasElement }: { canvasElement: HTMLElement }) => { await openSettingsToSection(canvasElement, "experiments"); - - const body = within(document.body); - const modal = body.getByRole("dialog"); - const modalCanvas = within(modal); - - // Find the switch - const switches = await modalCanvas.findAllByRole("switch"); - if (switches.length > 0) { - // Toggle on first - await userEvent.click(switches[0]); - // Then toggle off - await userEvent.click(switches[0]); - } + // Default state is OFF - no clicks needed }, }; From c79cf78036b0e08dcb235d85595e5c1f330f87b0 Mon Sep 17 00:00:00 2001 From: ethan Date: Fri, 12 Dec 2025 12:56:06 +1100 Subject: [PATCH 2/6] fix: wait for toggle checked state before Chromatic snapshot --- src/browser/stories/App.settings.stories.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/browser/stories/App.settings.stories.tsx b/src/browser/stories/App.settings.stories.tsx index 2db67944d3..24b97c0cbc 100644 --- a/src/browser/stories/App.settings.stories.tsx +++ b/src/browser/stories/App.settings.stories.tsx @@ -189,6 +189,9 @@ export const ExperimentsToggleOn: AppStory = { // Find the experiment toggle by its aria-label (set in ExperimentsSection.tsx) const toggle = await modalCanvas.findByRole("switch", { name: /Post-Compaction Context/i }); await userEvent.click(toggle); + + // Wait for toggle to be checked before Chromatic snapshot + await modalCanvas.findByRole("switch", { name: /Post-Compaction Context/i, checked: true }); }, }; From 74d4132d55df552c46483a78c98f7582a32f44d4 Mon Sep 17 00:00:00 2001 From: ethan Date: Fri, 12 Dec 2025 12:59:07 +1100 Subject: [PATCH 3/6] fix: use includes() for section button matching The exact text match fails because button textContent includes icon SVG content. Using includes() is more robust. --- src/browser/stories/App.settings.stories.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/browser/stories/App.settings.stories.tsx b/src/browser/stories/App.settings.stories.tsx index 24b97c0cbc..ebafa72983 100644 --- a/src/browser/stories/App.settings.stories.tsx +++ b/src/browser/stories/App.settings.stories.tsx @@ -66,10 +66,10 @@ async function openSettingsToSection(canvasElement: HTMLElement, section?: strin if (section && section !== "general") { const modal = body.getByRole("dialog"); const modalCanvas = within(modal); - // Find the nav section button (exact text match) + // Find the nav section button by matching the section label within the button text const navButtons = await modalCanvas.findAllByRole("button"); - const sectionButton = navButtons.find( - (btn) => btn.textContent?.toLowerCase().trim() === section.toLowerCase() + const sectionButton = navButtons.find((btn) => + btn.textContent?.toLowerCase().includes(section.toLowerCase()) ); if (!sectionButton) throw new Error(`Section button "${section}" not found`); await userEvent.click(sectionButton); From e7e97aea989987eef1c0071859a8426152b4f6e8 Mon Sep 17 00:00:00 2001 From: ethan Date: Fri, 12 Dec 2025 13:02:00 +1100 Subject: [PATCH 4/6] fix: use findByRole with name pattern and wait for section switch --- src/browser/stories/App.settings.stories.tsx | 25 ++++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/browser/stories/App.settings.stories.tsx b/src/browser/stories/App.settings.stories.tsx index ebafa72983..b7cb6ad683 100644 --- a/src/browser/stories/App.settings.stories.tsx +++ b/src/browser/stories/App.settings.stories.tsx @@ -62,17 +62,28 @@ async function openSettingsToSection(canvasElement: HTMLElement, section?: strin ); // Navigate to specific section if requested - // The sidebar nav has buttons with exact section names if (section && section !== "general") { const modal = body.getByRole("dialog"); const modalCanvas = within(modal); - // Find the nav section button by matching the section label within the button text - const navButtons = await modalCanvas.findAllByRole("button"); - const sectionButton = navButtons.find((btn) => - btn.textContent?.toLowerCase().includes(section.toLowerCase()) - ); - if (!sectionButton) throw new Error(`Section button "${section}" not found`); + + // Use findByRole with name to get the section button - this has built-in waiting + // Capitalize first letter to match the button text (e.g., "experiments" -> "Experiments") + const sectionLabel = section.charAt(0).toUpperCase() + section.slice(1); + const sectionButton = await modalCanvas.findByRole("button", { + name: new RegExp(sectionLabel, "i"), + }); await userEvent.click(sectionButton); + + // Wait for section content to be visible (the section header shows current section name) + await waitFor( + () => { + const sectionHeader = modal.querySelector(".text-foreground.text-sm.font-medium"); + if (!sectionHeader?.textContent?.toLowerCase().includes(section.toLowerCase())) { + throw new Error(`Section "${section}" not yet active`); + } + }, + { timeout: 3000 } + ); } } From a040b949f2a7b48aea76050d962c018144657473 Mon Sep 17 00:00:00 2001 From: ethan Date: Fri, 12 Dec 2025 13:04:28 +1100 Subject: [PATCH 5/6] fix: use screen from @storybook/test for portal content Following ~/coder patterns - use screen for dialog/portal elements instead of within(document.body). Simplifies the helper function. --- src/browser/stories/App.settings.stories.tsx | 42 ++++---------------- 1 file changed, 7 insertions(+), 35 deletions(-) diff --git a/src/browser/stories/App.settings.stories.tsx b/src/browser/stories/App.settings.stories.tsx index b7cb6ad683..fc8324cf30 100644 --- a/src/browser/stories/App.settings.stories.tsx +++ b/src/browser/stories/App.settings.stories.tsx @@ -14,7 +14,7 @@ import { appMeta, AppWithMocks, type AppStory } from "./meta.js"; import { createWorkspace, groupWorkspacesByProject } from "./mockFactory"; import { selectWorkspace } from "./storyHelpers"; import { createMockORPCClient } from "../../../.storybook/mocks/orpc"; -import { within, waitFor, userEvent } from "@storybook/test"; +import { within, userEvent, screen } from "@storybook/test"; export default { ...appMeta, @@ -47,43 +47,20 @@ async function openSettingsToSection(canvasElement: HTMLElement, section?: strin const canvas = within(canvasElement); // Wait for app to fully load (sidebar with settings button should appear) - // Use longer timeout since app initialization can take time const settingsButton = await canvas.findByTestId("settings-button", {}, { timeout: 10000 }); await userEvent.click(settingsButton); - // Wait for modal to appear - Radix Dialog uses a portal so we need to search the entire document - const body = within(document.body); - await waitFor( - () => { - const modal = body.getByRole("dialog"); - if (!modal) throw new Error("Settings modal not found"); - }, - { timeout: 5000 } - ); + // Wait for dialog to appear (portal renders outside canvasElement) + await screen.findByRole("dialog"); // Navigate to specific section if requested if (section && section !== "general") { - const modal = body.getByRole("dialog"); - const modalCanvas = within(modal); - - // Use findByRole with name to get the section button - this has built-in waiting // Capitalize first letter to match the button text (e.g., "experiments" -> "Experiments") const sectionLabel = section.charAt(0).toUpperCase() + section.slice(1); - const sectionButton = await modalCanvas.findByRole("button", { + const sectionButton = await screen.findByRole("button", { name: new RegExp(sectionLabel, "i"), }); await userEvent.click(sectionButton); - - // Wait for section content to be visible (the section header shows current section name) - await waitFor( - () => { - const sectionHeader = modal.querySelector(".text-foreground.text-sm.font-medium"); - if (!sectionHeader?.textContent?.toLowerCase().includes(section.toLowerCase())) { - throw new Error(`Section "${section}" not yet active`); - } - }, - { timeout: 3000 } - ); } } @@ -192,17 +169,12 @@ export const ExperimentsToggleOn: AppStory = { play: async ({ canvasElement }: { canvasElement: HTMLElement }) => { await openSettingsToSection(canvasElement, "experiments"); - // Find and click the switch to toggle it on - const body = within(document.body); - const modal = body.getByRole("dialog"); - const modalCanvas = within(modal); - - // Find the experiment toggle by its aria-label (set in ExperimentsSection.tsx) - const toggle = await modalCanvas.findByRole("switch", { name: /Post-Compaction Context/i }); + // Find and click the switch to toggle it on (dialog is a portal, use screen) + const toggle = await screen.findByRole("switch", { name: /Post-Compaction Context/i }); await userEvent.click(toggle); // Wait for toggle to be checked before Chromatic snapshot - await modalCanvas.findByRole("switch", { name: /Post-Compaction Context/i, checked: true }); + await screen.findByRole("switch", { name: /Post-Compaction Context/i, checked: true }); }, }; From 7f3f630dbf279c0bae09acf934bd8e6640c1c056 Mon Sep 17 00:00:00 2001 From: ethan Date: Fri, 12 Dec 2025 13:13:30 +1100 Subject: [PATCH 6/6] fix: use canvasElement.ownerDocument.body for portal queries The 'screen' from @storybook/test queries the entire page including Storybook UI, causing false matches on buttons like 'Experiments' in the sidebar. Using canvasElement.ownerDocument.body scopes queries to just the iframe content. --- src/browser/stories/App.settings.stories.tsx | 40 +++++++++++++------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/src/browser/stories/App.settings.stories.tsx b/src/browser/stories/App.settings.stories.tsx index fc8324cf30..aa773478b1 100644 --- a/src/browser/stories/App.settings.stories.tsx +++ b/src/browser/stories/App.settings.stories.tsx @@ -14,7 +14,8 @@ import { appMeta, AppWithMocks, type AppStory } from "./meta.js"; import { createWorkspace, groupWorkspacesByProject } from "./mockFactory"; import { selectWorkspace } from "./storyHelpers"; import { createMockORPCClient } from "../../../.storybook/mocks/orpc"; -import { within, userEvent, screen } from "@storybook/test"; +import { within, userEvent } from "@storybook/test"; +import { getExperimentKey, EXPERIMENT_IDS } from "@/common/constants/experiments"; export default { ...appMeta, @@ -29,11 +30,21 @@ export default { function setupSettingsStory(options: { providersConfig?: Record; providersList?: string[]; + /** Pre-set experiment states in localStorage before render */ + experiments?: Partial>; }): APIClient { const workspaces = [createWorkspace({ id: "ws-1", name: "main", projectName: "my-app" })]; selectWorkspace(workspaces[0]); + // Pre-set experiment states if provided + if (options.experiments) { + for (const [experimentId, enabled] of Object.entries(options.experiments)) { + const key = getExperimentKey(experimentId as typeof EXPERIMENT_IDS.POST_COMPACTION_CONTEXT); + window.localStorage.setItem(key, JSON.stringify(enabled)); + } + } + return createMockORPCClient({ projects: groupWorkspacesByProject(workspaces), workspaces, @@ -45,19 +56,21 @@ function setupSettingsStory(options: { /** Open settings modal and optionally navigate to a section */ async function openSettingsToSection(canvasElement: HTMLElement, section?: string): Promise { const canvas = within(canvasElement); + // Use ownerDocument.body to scope to iframe, not parent Storybook UI + const body = within(canvasElement.ownerDocument.body); // Wait for app to fully load (sidebar with settings button should appear) const settingsButton = await canvas.findByTestId("settings-button", {}, { timeout: 10000 }); await userEvent.click(settingsButton); - // Wait for dialog to appear (portal renders outside canvasElement) - await screen.findByRole("dialog"); + // Wait for dialog to appear (portal renders outside canvasElement but inside iframe body) + await body.findByRole("dialog"); // Navigate to specific section if requested if (section && section !== "general") { // Capitalize first letter to match the button text (e.g., "experiments" -> "Experiments") const sectionLabel = section.charAt(0).toUpperCase() + section.slice(1); - const sectionButton = await screen.findByRole("button", { + const sectionButton = await body.findByRole("button", { name: new RegExp(sectionLabel, "i"), }); await userEvent.click(sectionButton); @@ -163,18 +176,19 @@ export const Experiments: AppStory = { }, }; -/** Experiments section - toggle experiment on */ +/** Experiments section - shows experiment in ON state (pre-enabled via localStorage) */ export const ExperimentsToggleOn: AppStory = { - render: () => setupSettingsStory({})} />, + render: () => ( + + setupSettingsStory({ + experiments: { [EXPERIMENT_IDS.POST_COMPACTION_CONTEXT]: true }, + }) + } + /> + ), play: async ({ canvasElement }: { canvasElement: HTMLElement }) => { await openSettingsToSection(canvasElement, "experiments"); - - // Find and click the switch to toggle it on (dialog is a portal, use screen) - const toggle = await screen.findByRole("switch", { name: /Post-Compaction Context/i }); - await userEvent.click(toggle); - - // Wait for toggle to be checked before Chromatic snapshot - await screen.findByRole("switch", { name: /Post-Compaction Context/i, checked: true }); }, };