From 7c8865d0e3acaaabe7e4582ba6fdd04ebc66b13e Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Sat, 10 Oct 2026 01:21:13 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20fix:=20one=20main=20landmark=20i?= =?UTF-8?q?n=20immersive=20review=20and=20on=20pages=20without=20a=20works?= =?UTF-8?q?pace?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #5969 --- src/browser/App.tsx | 5 +- .../components/ProjectPage/ProjectPage.tsx | 5 +- .../components/ScratchPage/ScratchPage.tsx | 5 +- .../CodeReview/ImmersiveReviewView.tsx | 8 +- .../App.seededWorkspaceA11y.stories.tsx | 162 +++++++++++++++++- 5 files changed, 172 insertions(+), 13 deletions(-) diff --git a/src/browser/App.tsx b/src/browser/App.tsx index 1a745cc49bf..58553509ee4 100644 --- a/src/browser/App.tsx +++ b/src/browser/App.tsx @@ -138,7 +138,8 @@ function RootRouteShell(props: { onToggleLeftSidebarCollapsed: () => void; }) { return ( -
+ // The page has no chat pane here, so the shell is its one `main` landmark (#5969). +
{props.leftSidebarCollapsed ? (
Select or add a project to get started.

-
+ ); } diff --git a/src/browser/components/ProjectPage/ProjectPage.tsx b/src/browser/components/ProjectPage/ProjectPage.tsx index 64f06db4aae..9b43d7b12de 100644 --- a/src/browser/components/ProjectPage/ProjectPage.tsx +++ b/src/browser/components/ProjectPage/ProjectPage.tsx @@ -316,7 +316,8 @@ export const ProjectPage: React.FC = ({ {/* Flex container to fill parent space */} -
+ {/* No workspace is open, so this page is the one `main` landmark (#5969). */} +
{/* Draggable header bar - matches WorkspaceMenuBar for consistency */}
= ({ projectName={projectName} />
-
+ ); diff --git a/src/browser/components/ScratchPage/ScratchPage.tsx b/src/browser/components/ScratchPage/ScratchPage.tsx index dc763b2c41e..e497c2165fd 100644 --- a/src/browser/components/ScratchPage/ScratchPage.tsx +++ b/src/browser/components/ScratchPage/ScratchPage.tsx @@ -79,7 +79,8 @@ export function ScratchPage(props: ScratchPageProps) { return ( -
+ {/* No workspace is open, so this page is the one `main` landmark (#5969). */} +
- + ); diff --git a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx index 84c29557a0d..afa530eb9d7 100644 --- a/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx +++ b/src/browser/features/RightSidebar/CodeReview/ImmersiveReviewView.tsx @@ -319,7 +319,7 @@ function findReviewHunkId(review: Review, fileHunks: DiffHunk[]): string | null } export const ImmersiveReviewView: React.FC = (props) => { - const containerRef = useRef(null); + const containerRef = useRef(null); const scrollContainerRef = useRef(null); const notesSidebarRef = useRef(null); const hunkJumpScrollBlockRef = useRef("center"); @@ -2072,7 +2072,9 @@ export const ImmersiveReviewView: React.FC = (props) = : "empty"; return ( - )} - + ); }; diff --git a/src/browser/stories/App.seededWorkspaceA11y.stories.tsx b/src/browser/stories/App.seededWorkspaceA11y.stories.tsx index 63235a306c6..c7e00d1aafc 100644 --- a/src/browser/stories/App.seededWorkspaceA11y.stories.tsx +++ b/src/browser/stories/App.seededWorkspaceA11y.stories.tsx @@ -2,19 +2,23 @@ * Seeded workspace page (#5951): the ARIA and target-size audits that Lighthouse failed on a * workspace with a short chat. The play checks the same facts with role queries, in a real * browser so the toggle's hit area is measured. Contrast is a separate decision (#5950). The - * phone stories cover the main landmark at 390 px (#5956). + * phone stories cover the main landmark at 390 px (#5956). The landmark stories at the end cover + * the layouts without a chat pane: immersive review and the pages with no workspace (#5969). */ -import { expect, waitFor, within } from "@storybook/test"; +import { expect, userEvent, waitFor, within } from "@storybook/test"; import { updatePersistedState } from "@/browser/hooks/usePersistedState"; -import { RIGHT_SIDEBAR_WIDTH_KEY } from "@/common/constants/storage"; +import { CUSTOM_EVENTS, createCustomEvent } from "@/common/constants/events"; +import { getReviewImmersiveKey, RIGHT_SIDEBAR_WIDTH_KEY } from "@/common/constants/storage"; +import type { ProjectConfig } from "@/node/config"; import type { ThemeMode } from "@/browser/contexts/ThemeContext"; import { appMeta, AppWithMocks, PIXEL_DISABLED, type AppStory } from "./meta.js"; import { setupSimpleChatStory } from "./helpers/chatSetup"; -import { expandLeftSidebar, expandRightSidebar } from "./helpers/uiState"; +import { expandLeftSidebar, expandProjects, expandRightSidebar } from "./helpers/uiState"; +import { createMockORPCClient } from "./mocks/orpc"; import { createAssistantMessage, createUserMessage } from "./mocks/messages"; import { STABLE_TIMESTAMP } from "./mocks/workspaces"; @@ -226,3 +230,153 @@ const phoneStory = (theme: ThemeMode): AppStory => ({ export const PhoneLight = phoneStory("light"); export const PhoneDark = phoneStory("dark"); + +/** + * Layouts without a chat pane (#5969): immersive review hides the chat, and the root, project and + * scratch pages have no workspace. Each still needs exactly one `main` landmark, holding its + * primary content and none of the sidebars. Where a footer exists it stays outside `main`. + */ +async function expectOneMainHolding(canvasElement: HTMLElement, content: HTMLElement) { + const canvas = within(canvasElement); + const mains = canvas.getAllByRole("main"); + await expect(mains).toHaveLength(1); + await expect(mains[0]).toContainElement(content); + const main = within(mains[0]); + await expect(main.queryByRole("navigation", { name: "Projects" })).toBeNull(); + await expect(main.queryByRole("complementary", { name: "Workspace insights" })).toBeNull(); + await expect(main.queryByRole("contentinfo")).toBeNull(); +} + +const REVIEW_WORKSPACE_ID = "ws-a11y-review"; +const REVIEW_DIFF = `diff --git a/src/a11y/landmarks.ts b/src/a11y/landmarks.ts +index 1111111..2222222 100644 +--- a/src/a11y/landmarks.ts ++++ b/src/a11y/landmarks.ts +@@ -1,3 +1,4 @@ + export const LANDMARKS = ["main"]; ++export const ONE_MAIN = true; + export const VERSION = 1; +`; +const REVIEW_NUMSTAT = "1\t0\tsrc/a11y/landmarks.ts"; + +const PROJECT_PATH = "/home/user/projects/xum"; + +type LayoutPlay = (canvasElement: HTMLElement, layout: "desktop" | "phone") => Promise; + +interface LandmarkLayout { + setup: () => ReturnType; + /** Opens the layout and returns an element of its primary content. */ + open: LayoutPlay; +} + +const LANDMARK_LAYOUTS: Record< + "ImmersiveReview" | "RootPage" | "ProjectPage" | "ScratchPage", + LandmarkLayout +> = { + ImmersiveReview: { + setup: () => { + const client = setupSimpleChatStory({ + workspaceId: REVIEW_WORKSPACE_ID, + workspaceName: "a11y-review", + projectName: "xum", + projectPath: PROJECT_PATH, + messages: [ + createUserMessage("msg-1", "Review the change", { + historySequence: 1, + timestamp: STABLE_TIMESTAMP - 60_000, + }), + ], + gitDiff: { diffOutput: REVIEW_DIFF, numstatOutput: REVIEW_NUMSTAT }, + }); + expandRightSidebar(); + // Immersive mode is persisted per workspace, so start each story with the chat showing. + updatePersistedState(getReviewImmersiveKey(REVIEW_WORKSPACE_ID), false); + return client; + }, + open: async (canvasElement, layout) => { + const canvas = within(canvasElement); + await canvas.findByRole("textbox", { name: "Message" }, { timeout: 15_000 }); + window.dispatchEvent( + createCustomEvent( + layout === "phone" + ? CUSTOM_EVENTS.OPEN_TOUCH_REVIEW_IMMERSIVE + : CUSTOM_EVENTS.OPEN_REVIEW_IMMERSIVE, + { workspaceId: REVIEW_WORKSPACE_ID } + ) + ); + return canvas.findByTestId("immersive-review-view", {}, { timeout: 10_000 }); + }, + }, + RootPage: { + setup: () => createMockORPCClient({}), + open: (canvasElement) => + within(canvasElement).findByText( + "Select or add a project to get started.", + {}, + { timeout: 15_000 } + ), + }, + ProjectPage: { + setup: () => { + expandLeftSidebar(); + expandProjects([PROJECT_PATH]); + return createMockORPCClient({ + projects: new Map([[PROJECT_PATH, { workspaces: [] }]]), + workspaces: [], + }); + }, + open: async (canvasElement) => { + const body = within(canvasElement.ownerDocument.body); + await userEvent.click( + await body.findByRole("button", { name: "Create workspace in xum" }, { timeout: 15_000 }) + ); + return body.findByRole("textbox", { name: "Message" }); + }, + }, + ScratchPage: { + setup: () => { + expandLeftSidebar(); + return createMockORPCClient({}); + }, + open: async (canvasElement) => { + const body = within(canvasElement.ownerDocument.body); + await userEvent.click( + await body.findByRole("button", { name: "New scratch chat" }, { timeout: 15_000 }) + ); + return body.findByRole("textbox", { name: "Message" }); + }, + }, +}; + +const landmarkStory = ( + name: keyof typeof LANDMARK_LAYOUTS, + layout: "desktop" | "phone" +): AppStory => ({ + ...(layout === "phone" + ? { + globals: { viewport: { value: "mobile2", isRotated: false } }, + decorators: [ + (Story) => ( +
+ +
+ ), + ], + } + : {}), + parameters: { ...appMeta.parameters, pixel: PIXEL_DISABLED }, + render: () => , + play: async ({ canvasElement }) => { + const content = await LANDMARK_LAYOUTS[name].open(canvasElement, layout); + await waitFor(() => expectOneMainHolding(canvasElement, content)); + }, +}); + +export const ImmersiveReviewLandmark = landmarkStory("ImmersiveReview", "desktop"); +export const ImmersiveReviewLandmarkPhone = landmarkStory("ImmersiveReview", "phone"); +export const RootPageLandmark = landmarkStory("RootPage", "desktop"); +export const RootPageLandmarkPhone = landmarkStory("RootPage", "phone"); +export const ProjectPageLandmark = landmarkStory("ProjectPage", "desktop"); +export const ProjectPageLandmarkPhone = landmarkStory("ProjectPage", "phone"); +export const ScratchPageLandmark = landmarkStory("ScratchPage", "desktop"); +export const ScratchPageLandmarkPhone = landmarkStory("ScratchPage", "phone");