fix: rewrite responsive e2e tests to match current toolbar UI (#1347)#1349
Merged
fix: rewrite responsive e2e tests to match current toolbar UI (#1347)#1349
Conversation
The responsive E2E spec was written for a previous toolbar design that had hamburger menus and drawer navigation. The current toolbar uses a simpler mobile layout with direct Save/Load/Export buttons and bottom navigation. Updated all selectors to use stable, user-visible signals (aria roles/labels) instead of brittle class-based selectors. Key changes: - Replace .brand-name with role-based selector (SVG aria-label) - Replace aside.sidebar with .sidebar-pane - Remove nonexistent hamburger/drawer tests - Add mobile action button and bottom nav assertions - Remove phone dialog test (relied on defunct drawer UI) Fixes #1347 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRewrites the responsive E2E test to match updated UI structure: replaces legacy selectors with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
bot
added
the
size:M
Contributor
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/responsive.spec.ts`:
- Around line 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").
- Around line 62-75: Update the Playwright selectors to use case-insensitive
regex name matching instead of exact strings: replace page.getByRole("button", {
name: "Save layout" }) / "Load layout" / "Export layout" with
page.getByRole("button", { name: /save layout/i }) / /load layout/i / /export
layout/i for the test "mobile action buttons are shown" (variables saveBtn,
loadBtn, exportBtn), and replace page.getByRole("button", { name: "Devices" })
with page.getByRole("button", { name: /devices/i }) for the "bottom navigation
is visible" test (variable bottomNav) so the tests are resilient to casing/
minor wording changes.
- Around line 154-159: The test "reset view via keyboard shortcut" currently
only presses "f" and checks visibility, which doesn't verify reset behavior;
update the test to first change the view (e.g., use page.mouse.drag or
page.keyboard to pan and page.mouse.wheel or a zoom control to change scale) so
the panzoom container has a non-default transform, then press "f", and finally
assert that the panzoom container's transform or style (the ".panzoom-container"
locator) has returned to the identity/default (e.g., transform matrix equals
identity or top/left/scale equal defaults) to confirm the reset action worked.
Comment on lines
+23
to
26
| test("sidebar pane is visible", async ({ page }) => { | ||
| const sidebar = page.locator(".sidebar-pane"); | ||
| await expect(sidebar).toBeVisible(); | ||
| }); |
There was a problem hiding this comment.
🧹 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").
- Use case-insensitive regex for mobile button aria-label matching - Enhance reset view test to verify actual transform reset behaviour (set non-default transform, press "f", assert transform changed back) - Sidebar selector kept as .sidebar-pane (not <aside> at desktop viewport) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/responsive.spec.ts`:
- Around line 154-176: Replace the brittle sleep after triggering the reset
shortcut with a Playwright auto-retrying assertion: after calling
page.keyboard.press("f") remove page.waitForTimeout(300) and instead assert on
the panzoomContainer locator that its style attribute matches the default
transform (e.g. use expect(panzoomContainer).toHaveAttribute("style",
/matrix\(1,\s*0,\s*0,\s*1/, { timeout: 2000 }) or another regex for your
default), so the test retries until the transform resets rather than relying on
a fixed timeout.
Use expect.poll() instead of waitForTimeout(300) in the reset view test so Playwright retries until the transform changes rather than relying on a fixed sleep. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/responsive.spec.ts`:
- Around line 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.
Comment on lines
154
to
174
| test("reset view via keyboard shortcut", async ({ page }) => { | ||
| const panzoomContainer = page.locator(".panzoom-container"); | ||
|
|
||
| // 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)"; | ||
| } | ||
| }); | ||
|
|
||
| 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"); | ||
| }); |
There was a problem hiding this comment.
🧹 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.
- Rename bottom nav test and use semantic navigation role locator - Keep DOM-based transform setup for reset view test (consistent with view-reset.spec.ts pattern; real mouse panning doesn't reliably change panzoom transform in this viewport) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
e2e/responsive.spec.tsto match the current toolbar implementation (no hamburger menu/drawer — mobile uses direct Save/Load/Export buttons + bottom navigation).brand-name,aside.sidebar,.hamburger-icon,.toolbar-drawer,.drawer-item) with stable, user-visible signals (aria-label,role, existing classes)Test plan
Closes #1347
🤖 Generated with Claude Code
CodeAnt-AI Description
Update responsive end-to-end tests to match the current toolbar UI
What Changed
Impact
✅ Fewer flaky responsive tests✅ Tests reflect current mobile toolbar behavior (Save/Load/Export and bottom nav visible)✅ Clearer test failures for branding and sidebar visibility💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.