Skip to content

fix: rewrite responsive e2e tests to match current toolbar UI (#1347) #1349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ggfevans merged 4 commits into from
Feb 27, 2026
Merged

fix: rewrite responsive e2e tests to match current toolbar UI (#1347) #1349

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
892efde
fix: rewrite responsive e2e tests to match current toolbar UI
ggfevans Feb 27, 2026
b824cca
fix: address CodeRabbit feedback on responsive e2e tests
ggfevans Feb 27, 2026
e405135
fix: replace waitForTimeout with auto-retrying poll assertion
ggfevans Feb 27, 2026
0fe5faf
fix: address CodeRabbit round 2 feedback
ggfevans Feb 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
251 changes: 57 additions & 194 deletions e2e/responsive.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,20 @@ test.describe("Responsive Layout", () => {
await gotoWithRack(page);
});

test("toolbar buttons show text labels", async ({ page }) => {
const saveButton = page.getByRole("button", { name: /save/i });
await expect(saveButton).toBeVisible();

const buttonText = await saveButton.textContent();
expect(buttonText).toContain("Save");
test("toolbar action cluster is visible", async ({ page }) => {
const toolbarCenter = page.locator(".toolbar-center");
await expect(toolbarCenter).toBeVisible();
});

test("brand name visible", async ({ page }) => {
const brandName = page.locator(".brand-name");
await expect(brandName).toBeVisible();
await expect(brandName).toHaveText("Rackula");
const brandTitle = page.locator(".toolbar-brand").getByRole("img", {
name: /Rackula/,
});
await expect(brandTitle).toBeVisible();
});

test("sidebar is visible", async ({ page }) => {
const sidebar = page.locator("aside.sidebar");
test("sidebar pane is visible", async ({ page }) => {
const sidebar = page.locator(".sidebar-pane");
await expect(sidebar).toBeVisible();
});
Comment on lines +23 to 26
Copy link

@coderabbitai coderabbitai bot Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Acceptable class-based selector for sidebar.

If the sidebar uses <aside>, consider using page.getByRole('complementary') for a more semantic selector. Otherwise, the class selector is fine.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@e2e/responsive.spec.ts` around lines 23 - 26, The test "sidebar pane is
visible" currently uses a class selector via page.locator(".sidebar-pane"); if
the sidebar is implemented with an <aside> element prefer using the semantic
Playwright query page.getByRole('complementary') instead to improve
accessibility-based selection; update the test to use
page.getByRole('complementary') when the DOM uses <aside>, otherwise keep the
existing ".sidebar-pane" selector (adjust the locator in the test function named
"sidebar pane is visible").


Expand All @@ -37,17 +35,9 @@ test.describe("Responsive Layout", () => {
expect(hasHorizontalScroll).toBe(false);
});

test("brand click does NOT open drawer in full mode", async ({ page }) => {
const brand = page.locator(".toolbar-brand");
await brand.click();

const drawer = page.locator(".toolbar-drawer.open");
await expect(drawer).not.toBeVisible();
});

test("hamburger icon is NOT visible in full mode", async ({ page }) => {
const hamburgerIcon = page.locator(".hamburger-icon");
await expect(hamburgerIcon).not.toBeVisible();
test("file menu is accessible", async ({ page }) => {
const fileMenu = page.getByRole("button", { name: /file menu/i });
await expect(fileMenu).toBeVisible();
});
});

Expand All @@ -57,26 +47,33 @@ test.describe("Responsive Layout", () => {
await gotoWithRack(page);
});

test("hamburger mode is active", async ({ page }) => {
const hamburgerIcon = page.locator(".hamburger-icon");
await expect(hamburgerIcon).toBeVisible();

test("mobile mode is active — action cluster hidden", async ({ page }) => {
const toolbarCenter = page.locator(".toolbar-center");
await expect(toolbarCenter).not.toBeVisible();
});

test("brand name is still visible", async ({ page }) => {
const brandName = page.locator(".brand-name");
await expect(brandName).toBeVisible();
const brandTitle = page.locator(".toolbar-brand").getByRole("img", {
name: /Rackula/,
});
await expect(brandTitle).toBeVisible();
});

test("sidebar is narrower", async ({ page }) => {
const sidebar = page.locator("aside.sidebar");
await expect(sidebar).toBeVisible();
test("mobile action buttons are shown", async ({ page }) => {
const saveBtn = page.getByRole("button", { name: /save layout/i });
const loadBtn = page.getByRole("button", { name: /load layout/i });
const exportBtn = page.getByRole("button", { name: /export layout/i });

const box = await sidebar.boundingBox();
expect(box?.width).toBeLessThanOrEqual(210);
expect(box?.width).toBeGreaterThanOrEqual(190);
await expect(saveBtn).toBeVisible();
await expect(loadBtn).toBeVisible();
await expect(exportBtn).toBeVisible();
});

test("mobile bottom navigation is visible", async ({ page }) => {
const bottomNav = page.getByRole("navigation", {
name: /mobile navigation/i,
});
await expect(bottomNav).toBeVisible();
});

test("no horizontal scroll", async ({ page }) => {
Expand All @@ -88,15 +85,6 @@ test.describe("Responsive Layout", () => {
});
expect(hasHorizontalScroll).toBe(false);
});

test("drawer menu opens on hamburger click", async ({ page }) => {
await page.locator(".toolbar-brand").click();

const drawer = page.locator(".toolbar-drawer");
await expect(drawer).toBeVisible();

await expect(page.locator('.drawer-item:has-text("Save")')).toBeVisible();
});
});

test.describe("Small viewport (600px)", () => {
Expand All @@ -105,11 +93,15 @@ test.describe("Responsive Layout", () => {
await gotoWithRack(page);
});

test("brand name is hidden, logo visible", async ({ page }) => {
const brandName = page.locator(".brand-name");
await expect(brandName).toBeHidden();
test("brand name remains visible in toolbar", async ({ page }) => {
const brandTitle = page.locator(".toolbar-brand").getByRole("img", {
name: /Rackula/,
});
await expect(brandTitle).toBeVisible();
});

const logo = page.locator(".toolbar-brand .logo-icon");
test("logo mark is visible", async ({ page }) => {
const logo = page.locator(".toolbar-brand .logo-mark");
await expect(logo).toBeVisible();
});

Expand All @@ -124,145 +116,6 @@ test.describe("Responsive Layout", () => {
});
});

test.describe("Phone viewport dialogs (375px)", () => {
test.beforeEach(async ({ page }) => {
await page.setViewportSize({ width: 375, height: 667 });
await gotoWithRack(page);

// Dismiss mobile warning if present
await page.evaluate(() => {
sessionStorage.setItem("rackula-mobile-warning-dismissed", "true");
});

// Open the New Rack dialog
const newRackTitle = page.locator(".dialog-title").filter({
hasText: "New Rack",
});
if (!(await newRackTitle.first().isVisible())) {
await page.locator(".toolbar-brand").click();
const newRackDrawerItem = page.locator(
'.drawer-item:has-text("New Rack")',
);
await expect(newRackDrawerItem).toBeVisible();
await newRackDrawerItem.click();
}
});

test("new rack dialog is fully visible and touch-friendly", async ({
page,
}) => {
const dialog = page.locator('.dialog[role="dialog"]').first();
await test.step("Dialog is visible and positioned correctly", async () => {
await expect(page.locator(".dialog-title")).toHaveText("New Rack");
await expect(dialog).toBeVisible();
await expect
.poll(async () => {
const viewportHeight = await page.evaluate(
() => window.innerHeight,
);
const box = await dialog.boundingBox();
if (!box) {
return Number.POSITIVE_INFINITY;
}
return box.y + box.height - (viewportHeight + 1);
})
.toBeLessThanOrEqual(0);

const box = await dialog.boundingBox();
expect(box).toBeTruthy();
if (box) {
const viewportHeight = await page.evaluate(() => window.innerHeight);
expect(box.x).toBeGreaterThanOrEqual(-1);
expect(box.width).toBeGreaterThanOrEqual(374);
expect(box.y).toBeGreaterThanOrEqual(-1);
expect(box.y + box.height).toBeLessThanOrEqual(viewportHeight + 1);
}

const style = await dialog.evaluate((element) => {
const computed = window.getComputedStyle(element);
return {
position: computed.position,
left: computed.left,
right: computed.right,
bottom: computed.bottom,
};
});

expect(style.position).toBe("fixed");
expect(style.left).toBe("0px");
expect(style.right).toBe("0px");
expect(style.bottom).toBe("0px");
});

await test.step("Action buttons are full-width and touch-friendly", async () => {
const actions = dialog.locator(".form-actions");
await expect(actions).toBeVisible();
const actionButtons = actions.locator("button");
const actionCount = await actionButtons.count();
expect(actionCount).toBeGreaterThanOrEqual(2);

const actionsBox = await actions.boundingBox();
expect(actionsBox).toBeTruthy();
if (actionsBox) {
for (let i = 0; i < actionCount; i++) {
const buttonBox = await actionButtons.nth(i).boundingBox();
expect(buttonBox).toBeTruthy();
if (buttonBox) {
expect(buttonBox.width).toBeGreaterThanOrEqual(
actionsBox.width * 0.95,
);
}
}
}

const cancelButton = actions.getByRole("button", { name: "Cancel" });
const cancelBox = await cancelButton.boundingBox();
expect(cancelBox).toBeTruthy();
if (cancelBox) {
expect(cancelBox.height).toBeGreaterThanOrEqual(44);
}
});

await test.step("Dialog content is scrollable", async () => {
const overflowY = await dialog
.locator(".dialog-content")
.evaluate((element) => window.getComputedStyle(element).overflowY);
expect(["auto", "scroll"]).toContain(overflowY);
});

await test.step("Dialog survives virtual keyboard resize", async () => {
await page.setViewportSize({ width: 375, height: 420 });
await expect(dialog).toBeVisible();
await expect
.poll(async () => {
const compactViewportHeight = await page.evaluate(
() => window.innerHeight,
);
const compactBox = await dialog.boundingBox();
if (!compactBox) {
return Number.POSITIVE_INFINITY;
}
return (
compactBox.y + compactBox.height - (compactViewportHeight + 1)
);
})
.toBeLessThanOrEqual(0);

const compactBox = await dialog.boundingBox();
expect(compactBox).toBeTruthy();
if (compactBox) {
const compactViewportHeight = await page.evaluate(
() => window.innerHeight,
);
expect(compactBox.y).toBeGreaterThanOrEqual(-1);
expect(compactBox.y + compactBox.height).toBeLessThanOrEqual(
compactViewportHeight + 1,
);
}
});
});
});

test.describe("Panzoom at narrow viewport", () => {
test.beforeEach(async ({ page }) => {
await page.setViewportSize({ width: 800, height: 600 });
Expand Down Expand Up @@ -300,16 +153,26 @@ test.describe("Responsive Layout", () => {
expect(transform).toContain("matrix");
});

test("reset view button works", async ({ page }) => {
// At 800px viewport, need to use hamburger menu
await page.locator(".toolbar-brand").click();
test("reset view via keyboard shortcut", async ({ page }) => {
const panzoomContainer = page.locator(".panzoom-container");

// Set a non-default transform (matches pattern in view-reset.spec.ts)
await page.evaluate(() => {
const container = document.querySelector(".panzoom-container");
if (container) {
(container as HTMLElement).style.transform =
"matrix(0.5, 0, 0, 0.5, -300, -300)";
}
});

const resetButton = page.locator('.drawer-item:has-text("Reset View")');
await resetButton.click();
const transformBefore = await panzoomContainer.getAttribute("style");
expect(transformBefore).toContain("-300");

await expect(
page.locator('.toolbar-drawer:not([aria-hidden="true"])'),
).not.toBeVisible();
// Press "f" to reset view — auto-retry until transform changes
await page.keyboard.press("f");
await expect
.poll(() => panzoomContainer.getAttribute("style"), { timeout: 2000 })
.not.toContain("-300");
});
Comment on lines 156 to 176
Copy link

@coderabbitai coderabbitai bot Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using actual pan gesture instead of direct DOM manipulation.

The expect.poll() pattern is a good improvement over waitForTimeout. However, directly setting the transform via page.evaluate() bypasses the panzoom library's internal state. If the reset shortcut reads from the library's state rather than the DOM, this test could pass/fail inconsistently.

Since the "can pan the canvas" test (lines 128-152) already demonstrates real panning via mouse gestures, consider reusing that approach for a more realistic test flow.

♻️ Suggested refactor using actual pan gesture 
     test("reset view via keyboard shortcut", async ({ page }) => {
       const panzoomContainer = page.locator(".panzoom-container");
+      const canvas = page.locator(".canvas");

-      // Pan the view to a non-default position
-      await page.evaluate(() => {
-        const container = document.querySelector(".panzoom-container");
-        if (container) {
-          (container as HTMLElement).style.transform =
-            "matrix(0.5, 0, 0, 0.5, -300, -300)";
-        }
-      });
+      // Pan the view using actual mouse gesture
+      const canvasBox = await canvas.boundingBox();
+      if (canvasBox) {
+        const startX = canvasBox.x + canvasBox.width / 2;
+        const startY = canvasBox.y + canvasBox.height / 2;
+        await page.mouse.move(startX, startY);
+        await page.mouse.down();
+        await page.mouse.move(startX - 100, startY - 100, { steps: 5 });
+        await page.mouse.up();
+      }

-      const transformBefore = await panzoomContainer.getAttribute("style");
-      expect(transformBefore).toContain("-300");
+      const transformBefore = await panzoomContainer.getAttribute("style");
+      expect(transformBefore).toContain("matrix");

       // Press "f" to reset view — auto-retry until transform changes
       await page.keyboard.press("f");
       await expect
-        .poll(() => panzoomContainer.getAttribute("style"), { timeout: 2000 })
-        .not.toContain("-300");
+        .poll(async () => {
+          const style = await panzoomContainer.getAttribute("style");
+          // Check for identity matrix (reset state)
+          return style?.includes("matrix(1, 0, 0, 1, 0, 0)") ||
+                 style?.includes("matrix(1,0,0,1,0,0)");
+        }, { timeout: 2000 })
+        .toBe(true);
     });
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@e2e/responsive.spec.ts` around lines 154 - 174, The test "reset view via
keyboard shortcut" currently forces the transform by DOM mutation which bypasses
the panzoom library state; replace the page.evaluate DOM set with an actual pan
gesture (use the same approach as the "can pan the canvas" test: locate
.panzoom-container, use
page.mouse.move/page.mouse.down/page.mouse.move/page.mouse.up to perform a pan)
so the library's internal state updates, then assert the style on
panzoomContainer and press "f" and poll as before; update any references to
transformBefore and the panzoomContainer locator accordingly.

});
});