diff --git a/packages/dashboard/app/components/__tests__/agent-modals-mobile.test.tsx b/packages/dashboard/app/components/__tests__/agent-modals-mobile.test.tsx index 32175c1ba3..fb4e38cb8a 100644 --- a/packages/dashboard/app/components/__tests__/agent-modals-mobile.test.tsx +++ b/packages/dashboard/app/components/__tests__/agent-modals-mobile.test.tsx @@ -1,8 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import fs from "node:fs"; -import path from "node:path"; import { AgentDetailView } from "../AgentDetailView"; import { AgentGenerationModal } from "../AgentGenerationModal"; import { AgentImportModal } from "../AgentImportModal"; @@ -203,13 +201,30 @@ describe("agent modal mobile CSS structure", () => { }); describe("AgentDetailView", () => { - it("overlay and modal have mobile-targetable classes", async () => { + /* + FNXC:ModalTouchGeometry 2026-07-30-18:20: + THE SCRIM IS FLOATINGWINDOW'S NOW — this asserts the panel, not a local overlay. + + This case demanded `.agent-detail-overlay`, which no component renders: FN-8619 migrated Agent + Detail onto `FloatingWindow`, whose `modal` host owns the scrim + (`.floating-window-overlay--modal`). `AgentDetailView.core.test.tsx:97` already asserts the + class is ABSENT, so the suite was asserting both sides of the same fact and one of them had to + be red. + + The panel class is what this file is actually for: `.agent-detail-modal` is live and is what the + mobile `@media` block in AgentDetailView.css targets. The overlay half is dropped rather than + re-pointed at `.floating-window-overlay` — that would only re-assert FloatingWindow's own + contract, which FloatingWindow.test.tsx already covers, and would say nothing about Agent + Detail being mobile-targetable. + */ + it("modal panel keeps its mobile-targetable class", async () => { render(); await waitFor(() => { - expect(document.querySelector(".agent-detail-overlay")).toBeTruthy(); expect(document.querySelector(".agent-detail-modal")).toBeTruthy(); }); + // The retired local scrim must stay retired; its CSS is dead and tracked in #2915. + expect(document.querySelector(".agent-detail-overlay")).toBeNull(); }); it("tabs have scrollable container class", async () => { @@ -293,10 +308,20 @@ describe("agent modal mobile CSS structure", () => { }); describe("AgentGenerationModal", () => { - it("overlay and dialog classes exist", () => { + /* + FNXC:ModalTouchGeometry 2026-07-30-18:20: + `.agent-dialog-overlay` belongs to NewAgentDialog, NOT to this modal. + + That class is still live — `NewAgentDialog.tsx:415` renders it — which is why the stale + expectation here looked plausible. But AgentGenerationModal was migrated onto `FloatingWindow` + with `modal` (AgentGenerationModal.tsx:162), so its scrim is + `.floating-window-overlay--modal` and it never emits the local overlay. Asserted explicitly + because "modal blocks the app beneath it" is a real contract worth pinning per FN-8619. + */ + it("dialog renders inside the shared modal scrim", () => { render(); - expect(document.querySelector(".agent-dialog-overlay")).toBeTruthy(); + expect(document.querySelector(".floating-window-overlay--modal")).toBeTruthy(); expect(document.querySelector(".agent-dialog")).toBeTruthy(); }); diff --git a/packages/dashboard/app/components/__tests__/core-modals-mobile.test.tsx b/packages/dashboard/app/components/__tests__/core-modals-mobile.test.tsx index 0e19e0fc94..08fad182fb 100644 --- a/packages/dashboard/app/components/__tests__/core-modals-mobile.test.tsx +++ b/packages/dashboard/app/components/__tests__/core-modals-mobile.test.tsx @@ -1,6 +1,4 @@ -import fs from "node:fs"; import { loadAllAppCss } from "../../test/cssFixture"; -import path from "node:path"; import { describe, expect, it } from "vitest"; @@ -34,6 +32,21 @@ function getMainMobileBlock(css: string): string { return block; } +/* +FNXC:ModalTouchGeometry 2026-07-30-19:20: +FloatingWindow's phone breakpoint is NOT the 768px one `getMainMobileBlock` aggregates. + +It is `(max-width: 767.98px), (max-height: 480px)` — the project's documented mobile query, whose +`max-height` clause catches landscape phones that exceed 768px wide. Reusing the 768px helper here +silently returns styles.css's block, which contains none of these selectors; the anti-vacuity +assertion in the caller is what caught that during authoring. +*/ +function getFloatingWindowMobileBlock(css: string): string { + const block = getMediaBlocks(css, /@media[^{]*\(max-width:\s*767\.98px\)[^{]*\{/g); + expect(block).toContain(".floating-window"); + return block; +} + function getEmbeddedGitManagerBlock(css: string): string { const block = getMediaBlocks(css, /@container\s+gm-embedded\s+\(max-width:\s*560px\)\s*\{/g); expect(block).toContain(".gm-modal--embedded .gm-sidebar"); @@ -67,35 +80,57 @@ function getLastRuleBlock(css: string, selector: string): string { return block!; } -function extractVhHeight(rule: string): number { - const heightMatch = rule.match(/height:\s*(\d+)vh;/); - expect(heightMatch).toBeTruthy(); - return Number(heightMatch![1]); -} - describe("core modals mobile css coverage", () => { - it("TaskDetailModal: keeps desktop, tablet, mobile, and embedded height invariants", () => { + /* + FNXC:ModalTouchGeometry 2026-07-30-19:05: + TASK DETAIL NO LONGER SIZES ITSELF — the invariant moved layers, it did not disappear. + + This case pinned `.modal.task-detail-modal` at `height: 85vh` (desktop), `92vh` (tablet) and + `100dvh` (mobile), with `resize: both` / `resize: none`. FN-8619 migrated Task Detail onto + `FloatingWindow`, so the panel is now `width: 100%; height: 100%` and fills a host sized by + geometry. The desktop and tablet `vh` heights are `defaultSize` in TSX, not CSS, so asserting them + against a stylesheet can only ever fail. + + The MOBILE invariant is the one that still matters and is still CSS, so it is asserted at its new + home: `.floating-window--task-detail` inside FloatingWindow.css's mobile block takes over the + viewport and hides the resize handle. That is the same guarantee the old `100dvh` / `resize: none` + assertions made — a phone gets a full-screen sheet, not a draggable window. + + Deliberately NOT re-pinning the desktop/tablet numbers via `defaultSize`: those are ordinary + layout defaults a designer may retune, and a test that fails on a 640→680 width change is noise. + The full-screen-on-mobile rule is a real contract; 85vh on a desktop is a preference. + + Note the media query is `(max-width: 767.98px), (max-height: 480px)` — landscape phones exceed + 768px wide, so the height clause is load-bearing and asserted with it. + */ + it("TaskDetailModal: takes over the viewport on mobile instead of sizing itself", () => { const css = loadAllAppCss(); const tabletBlock = getTabletBlock(css); - const mobileBlock = getMainMobileBlock(css); + /* The panel defers sizing to its FloatingWindow host. */ const baseRule = getFirstRuleBlock(css, ".modal.task-detail-modal"); - expect(baseRule).toContain("height: 85vh;"); - expect(baseRule).toContain("max-height: calc(100dvh - var(--overlay-padding-top, 10vh) - 16px);"); - expect(baseRule).toContain("resize: both;"); + expect(baseRule).toContain("width: 100%;"); + expect(baseRule).toContain("height: 100%;"); - const tabletRule = getLastRuleBlock(tabletBlock, ".modal.task-detail-modal"); - expect(tabletRule).toContain("height: 92vh;"); - expect(extractVhHeight(tabletRule)).toBeGreaterThan(extractVhHeight(baseRule)); - expect(tabletRule).toContain("width: 98vw;"); - expect(tabletRule).toContain("max-width: 98vw;"); - expect(tabletBlock).toContain("--overlay-padding-top: 6vh;"); - expect(tabletRule).toContain("max-height: calc(100dvh - var(--overlay-padding-top, 6vh) - var(--space-md));"); + /* ANTI-VACUITY: prove the mobile block was found and really is the phone breakpoint, + so a renamed/removed query fails loudly instead of matching an empty string. */ + const floatingMobileBlock = getFloatingWindowMobileBlock(css); + expect(floatingMobileBlock).toContain("max-height: 480px"); + expect(floatingMobileBlock).toContain(".floating-window--task-detail"); - const mobileRule = getLastRuleBlock(mobileBlock, ".modal.task-detail-modal"); - expect(mobileRule).toContain("height: 100dvh;"); - expect(mobileRule).toContain("max-height: 100dvh;"); - expect(mobileRule).toContain("resize: none;"); + const mobileRule = getLastRuleBlock(floatingMobileBlock, ".floating-window--task-detail"); + /* + MATCHED AT DECLARATION BOUNDARIES, not by substring. `toContain("height: 100dvh")` is satisfied + by the `max-height: 100dvh` line, so it stays green even if `height` itself is changed — caught + by mutation: rewriting `height` to `90dvh` left the old assertions passing. Anchoring on `{`/`;` + forces each declaration to be checked on its own. + */ + expect(mobileRule).toMatch(/[{;]\s*height:\s*100dvh/); + expect(mobileRule).toMatch(/[{;]\s*max-height:\s*100dvh/); + expect(mobileRule).toMatch(/[{;]\s*width:\s*100vw/); + expect(mobileRule).toMatch(/[{;]\s*max-width:\s*100vw/); + /* The resize affordance must be gone on touch, not merely inert. */ + expect(floatingMobileBlock).toContain(".floating-window--task-detail .floating-window__resize-handle"); const embeddedRule = getRuleBlocks(css, ".task-detail-content--embedded") .find((rule) => rule.includes("height: 100%;")); diff --git a/packages/dashboard/app/components/__tests__/list-view-windowing.test.tsx b/packages/dashboard/app/components/__tests__/list-view-windowing.test.tsx index 1720f140a0..1e7d3067d0 100644 --- a/packages/dashboard/app/components/__tests__/list-view-windowing.test.tsx +++ b/packages/dashboard/app/components/__tests__/list-view-windowing.test.tsx @@ -20,7 +20,39 @@ vi.mock("../../api", () => ({ fetchTaskDetail: vi.fn(), batchUpdateTaskModels: vi.fn(), fetchNodes: vi.fn(() => new Promise(() => {})), - fetchBoardWorkflows: vi.fn(() => new Promise(() => {})), + /* + FNXC:ListViewWindowing 2026-07-30-20:10: + A RESOLVED LANE IS A PRECONDITION FOR RENDERING ANY ROW — this mock used to hang forever. + + It was `vi.fn(() => new Promise(() => {}))`, which was harmless when written: List view still had + `LEGACY_LIST_COLUMNS` to fall back on. U12/R9 DELETED that fallback, so `ListView` now returns its + skeleton unless `useBoardWorkflows` resolves a lane — and a never-settling promise means it never + does. All eight cases then asserted against an empty document ("expected [] to have a length of + 50"), which reads as a broken render window rather than an unmet precondition. + + Resolved with a real column set, mirroring the payload other board tests use, so the windowing + invariants below are exercised against actual rows. `flagEnabled: true` matters: the workflow arm + is the only one left. + */ + fetchBoardWorkflows: vi.fn().mockResolvedValue({ + flagEnabled: true, + defaultWorkflowId: "builtin:coding", + workflows: [ + { + id: "builtin:coding", + name: "Coding", + columns: [ + { id: "todo", name: "Todo", flags: { intake: true, hold: true } }, + { id: "in-progress", name: "In Progress", flags: { countsTowardWip: true } }, + { id: "in-review", name: "In Review", flags: { mergeBlocker: true, humanReview: true } }, + { id: "done", name: "Done", flags: { complete: true } }, + { id: "archived", name: "Archived", flags: { archived: true } }, + ], + }, + ], + taskWorkflowIds: {}, + }), + fetchWorkflowSteps: vi.fn().mockResolvedValue([]), rebuildTaskSpec: vi.fn().mockResolvedValue({}), refreshPrStatus: vi.fn().mockResolvedValue({}), updateTask: vi.fn(), @@ -79,8 +111,14 @@ function makeTask(index: number): Task { const TASKS: Task[] = Array.from({ length: TOTAL_TASKS }, (_, i) => makeTask(i + 1)); const FAR_TASK_ID = "FN-190"; -function renderList(props: Partial> = {}) { - return render( +/* +FNXC:ListViewWindowing 2026-07-30-20:15: +ASYNC because the lane now resolves through a promise. `useBoardWorkflows` settles a microtask after +mount, and `ListView` renders its skeleton until it does — so a synchronous `render()` observes zero +rows no matter what the window logic does. Flushed here, once, rather than in each case. +*/ +async function renderList(props: Partial> = {}) { + const result = render( TASKS[0])} @@ -93,6 +131,8 @@ function renderList(props: Partial> = {}) {...props} />, ); + await act(async () => { await Promise.resolve(); }); + return result; } function renderedTaskIds(): string[] { @@ -110,8 +150,8 @@ beforeEach(() => { }); describe("ListView render windowing", () => { - it("renders only the initial window of a large section, not every task", () => { - renderList(); + it("renders only the initial window of a large section, not every task", async () => { + await renderList(); expect(renderedTaskIds()).toHaveLength(INITIAL_WINDOW); // The section header still reports the FULL group size — grouping is preserved. @@ -119,8 +159,8 @@ describe("ListView render windowing", () => { expect(screen.getByRole("button", { name: /Load 25 more/i })).toBeTruthy(); }); - it("reveals the next increment when Load more is clicked", () => { - renderList(); + it("reveals the next increment when Load more is clicked", async () => { + await renderList(); act(() => { fireEvent.click(screen.getByRole("button", { name: /Load 25 more/i })); @@ -133,8 +173,8 @@ describe("ListView render windowing", () => { expect(renderedTaskIds()).toHaveLength(INITIAL_WINDOW + INCREMENT * 2); }); - it("filters against the full set, so a match beyond the window is still found", () => { - renderList({ searchQuery: "Needle" }); + it("filters against the full set, so a match beyond the window is still found", async () => { + await renderList({ searchQuery: "Needle" }); const ids = renderedTaskIds(); expect(ids).toEqual([FAR_TASK_ID]); @@ -142,14 +182,14 @@ describe("ListView render windowing", () => { expect(screen.queryByRole("button", { name: /Load \d+ more/i })).toBeNull(); }); - it("keeps a selected task outside the window selected and visible", () => { + it("keeps a selected task outside the window selected and visible", async () => { localStorage.setItem(scopedKey("kb-dashboard-list-selected-task", PROJECT_ID), FAR_TASK_ID); localStorage.setItem( scopedKey("kb-dashboard-selected-tasks", PROJECT_ID), JSON.stringify([FAR_TASK_ID]), ); - renderList(); + await renderList(); // Selection state is id-based and untouched by the window. expect( @@ -182,8 +222,8 @@ describe("ListView select-all under render windowing", () => { }); } - it("selects only the rendered window, not the whole filtered set", () => { - renderList(); + it("selects only the rendered window, not the whole filtered set", async () => { + await renderList(); enterBulkEdit(); const rendered = renderedTaskIds(); @@ -200,7 +240,7 @@ describe("ListView select-all under render windowing", () => { }); it("confirms a bulk delete against the rendered rows only", async () => { - renderList(); + await renderList(); enterBulkEdit(); selectAll(); @@ -214,8 +254,8 @@ describe("ListView select-all under render windowing", () => { expect(message).not.toContain(String(TOTAL_TASKS)); }); - it("grows the select-all target as the window is expanded", () => { - renderList(); + it("grows the select-all target as the window is expanded", async () => { + await renderList(); enterBulkEdit(); act(() => { @@ -230,8 +270,8 @@ describe("ListView select-all under render windowing", () => { expect(persisted.sort()).toEqual([...renderedTaskIds()].sort()); }); - it("reports checked, not indeterminate, once the rendered window is fully selected", () => { - renderList(); + it("reports checked, not indeterminate, once the rendered window is fully selected", async () => { + await renderList(); enterBulkEdit(); selectAll();