chore: integrate playwright axe for a11y e2e tests#8372
Open
itsjavi wants to merge 2 commits into
Open
Conversation
|
Contributor
WalkthroughUpdated agent instruction files and ignore rules for local 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/playwright/survey-accessibility.spec.ts`:
- Around line 431-438: The first active card is being captured too soon after
clicking the welcome button, which can leave activeCard(page).first() pointing
at the welcome state instead of the first required question. In the survey
accessibility spec, update the flow around the welcome click and
activeCard/page.getByRole steps so the test waits for the welcome card to fully
transition before reading firstCardId; apply the same synchronization fix in the
second matching block as well.
- Around line 51-59: The accessibility allowlist in survey-accessibility.spec.ts
is too broad because the generic iframe target causes unrelated iframe
violations to be suppressed. Tighten the matching logic in the rule entries for
frame-title and the related nested-interactive cases by removing the generic
iframe matcher and keeping only the Cal.com-specific targets, so the allowlist
applies only to the intended third-party embed.
- Line 24: The failing tag set in FAIL_TAGS is missing WCAG 2.2 Level A, which
lets those violations pass the accessibility gate. Update the accessibility tag
list used by survey-accessibility.spec.ts to include wcag22a alongside the
existing wcag2a/wcag2aa/wcag21a/wcag21aa/wcag22aa entries, so the
AxeBuilder.withTags filtering in the accessibility test covers both Level A and
Level AA criteria for WCAG 2.2.
In `@apps/web/playwright/utils/accessibility.ts`:
- Around line 131-137: The seeded Cal card in the accessibility fixture
currently points at a live third-party endpoint, which makes the unattended axe
run depend on external network and markup. Update the fixture in
accessibility.ts so the card uses a deterministic test-owned mocked Cal route or
endpoint, or remove/exclude the live embed from this no-network setup. Use the
existing card seed shape around createId() and the calUserName/calHost fields to
keep the fixture stable without reaching out to Cal.com.
- Around line 253-287: The RTL fixture only patches headline/subheader/choices
today, so several label-bearing fields still stay in default language. Update
the localization pass in accessibility.ts within the block/element patching
logic to also run localize over placeholder, rating lowerLabel/upperLabel, and
matrix rows/columns label fields wherever they appear in the element shapes.
Keep the existing recursive-style cloning approach in
patchedBlocks/patchedElements so axe sees fully localized Arabic content across
all block types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 90d3d3a1-a523-4536-96ba-33fa397af528
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.gitignoreAGENTS.mdCLAUDE.mdapps/web/playwright/survey-accessibility.spec.tsapps/web/playwright/utils/accessibility.tspackage.json
|
|
||
| // Only WCAG 2.1 / 2.2 AA conformance tags fail the build. These are the levels we | ||
| // commit to for the survey surface. | ||
| const FAIL_TAGS = ["wcag2a", "wcag2aa", "wcag21a", "wcag21aa", "wcag22aa"]; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Does axe-core 4.11.3 expose WCAG 2.2 Level A rules with the tag "wcag22a", and does AxeBuilder.withTags require listing it separately from "wcag22aa"?
💡 Result:
Axe-core version 4.11.3 includes support for the tag "wcag22a", which corresponds to WCAG 2.2 Level A rules [1][2][3]. Regarding AxeBuilder.withTags, axe-core treats tags as a set of inclusion filters [1][4]. The withTags method does not automatically include rules from one level (e.g., Level A) when you specify another (e.g., Level AA) [1][5]. Because these tags are distinct, if you wish to run both WCAG 2.2 Level A and Level AA rules, you must list both "wcag22a" and "wcag22aa" explicitly in the tags array [1][5]. For example, to run both levels, your configuration would look like:.withTags(['wcag22a', 'wcag22aa']) [1][5]
Citations:
- 1: https://docs.deque.com/devtools-for-web/4/en/node-br-ref-overview/
- 2: https://docs.deque.com/devtools-for-web/4/en/wa-java-axerunonly/
- 3: https://docs.dequelabs.com/devtools-for-web/4/en/wa-java-axerunonly/
- 4: https://github.com/dequelabs/axe-core-npm/blob/develop/packages/playwright/README.md
- 5: https://qaskills.sh/blog/playwright-accessibility-testing-axe-complete-guide
Include WCAG 2.2 Level A in the failing tag set.
AA conformance requires passing both Level A and Level AA criteria. The current configuration omits wcag22a, allowing WCAG 2.2 Level A violations to bypass the gate. Verification confirms axe-core 4.11.3 treats tags as explicit inclusion filters; AxeBuilder.withTags does not implicitly include Level A rules when only Level AA is specified.
Proposed fix
-const FAIL_TAGS = ["wcag2a", "wcag2aa", "wcag21a", "wcag21aa", "wcag22aa"];
+const FAIL_TAGS = ["wcag2a", "wcag2aa", "wcag21a", "wcag21aa", "wcag22a", "wcag22aa"];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const FAIL_TAGS = ["wcag2a", "wcag2aa", "wcag21a", "wcag21aa", "wcag22aa"]; | |
| const FAIL_TAGS = ["wcag2a", "wcag2aa", "wcag21a", "wcag21aa", "wcag22a", "wcag22aa"]; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/playwright/survey-accessibility.spec.ts` at line 24, The failing tag
set in FAIL_TAGS is missing WCAG 2.2 Level A, which lets those violations pass
the accessibility gate. Update the accessibility tag list used by
survey-accessibility.spec.ts to include wcag22a alongside the existing
wcag2a/wcag2aa/wcag21a/wcag21aa/wcag22aa entries, so the AxeBuilder.withTags
filtering in the accessibility test covers both Level A and Level AA criteria
for WCAG 2.2.
Comment on lines
+51
to
+59
| ruleId: "frame-title", | ||
| targets: ["iframe", "cal-embed", "cal.com"], | ||
| justification: "Cal.com embed is a third-party iframe (wontfix — not our DOM).", | ||
| }, | ||
| { | ||
| // Same third-party Cal.com iframe: any nested-interactive / aria issues raised | ||
| // against the embed container itself are out of our control. | ||
| ruleId: "nested-interactive", | ||
| targets: ["cal-embed", "cal.com", "iframe"], |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove generic allowlist matchers that suppress unrelated iframe violations.
Because targets.some(...) is an OR match, targets: ["iframe", ...] allowlists every frame-title finding on any iframe, not just Cal.com.
Proposed tightening
- targets: ["iframe", "cal-embed", "cal.com"],
+ targets: ["cal-embed", "cal.com"],
@@
- targets: ["cal-embed", "cal.com", "iframe"],
+ targets: ["cal-embed", "cal.com"],Also applies to: 87-92
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/playwright/survey-accessibility.spec.ts` around lines 51 - 59, The
accessibility allowlist in survey-accessibility.spec.ts is too broad because the
generic iframe target causes unrelated iframe violations to be suppressed.
Tighten the matching logic in the rule entries for frame-title and the related
nested-interactive cases by removing the generic iframe matcher and keeping only
the Cal.com-specific targets, so the allowlist applies only to the intended
third-party embed.
Comment on lines
+431
to
+438
| await page.goto(seeded.surveyUrl, { waitUntil: "domcontentloaded" }); | ||
| const welcomeStart = page.getByRole("button", { name: /^(Start|Next|Begin)$/ }).first(); | ||
| await expect(welcomeStart).toBeVisible({ timeout: CARD_TIMEOUT }); | ||
| await welcomeStart.click({ timeout: ACTION_TIMEOUT }); | ||
|
|
||
| const firstCard = activeCard(page).first(); | ||
| await expect(firstCard).toBeVisible({ timeout: CARD_TIMEOUT }); | ||
| const firstCardId = await firstCard.getAttribute("id"); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wait until the welcome card actually transitions before capturing the first question.
These two tests reintroduce the race avoided in walkAndScan: immediately after clicking welcome, activeCard(page).first() can still be questionCard--1, so validation/back-nav scans may target the welcome state instead of the first required question.
Proposed fix pattern for both blocks
await page.goto(seeded.surveyUrl, { waitUntil: "domcontentloaded" });
+const welcomeCardId = await getActiveCardId(page);
const welcomeStart = page.getByRole("button", { name: /^(Start|Next|Begin)$/ }).first();
await expect(welcomeStart).toBeVisible({ timeout: CARD_TIMEOUT });
await welcomeStart.click({ timeout: ACTION_TIMEOUT });
+if (welcomeCardId) {
+ await expect
+ .poll(() => getActiveCardId(page), {
+ message: "welcome should advance to the first question card",
+ timeout: CARD_TIMEOUT,
+ })
+ .not.toBe(welcomeCardId);
+}
const firstCard = activeCard(page).first();Also applies to: 456-463
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/playwright/survey-accessibility.spec.ts` around lines 431 - 438, The
first active card is being captured too soon after clicking the welcome button,
which can leave activeCard(page).first() pointing at the welcome state instead
of the first required question. In the survey accessibility spec, update the
flow around the welcome click and activeCard/page.getByRole steps so the test
waits for the welcome card to fully transition before reading firstCardId; apply
the same synchronization fix in the second matching block as well.
Comment on lines
+131
to
+137
| id: createId(), | ||
| type: "cal", | ||
| headline: t("Book a call with us"), | ||
| required: false, | ||
| calUserName: "rick/get-rick-rolled", | ||
| calHost: "cal.com", | ||
| }, |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Avoid depending on live Cal.com in the unattended axe fixture.
This seeded card can pull third-party content during the accessibility walk, making CI depend on external network and markup. Use a deterministic mocked route/test-owned Cal endpoint, or exclude the live embed from this no-network fixture.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/playwright/utils/accessibility.ts` around lines 131 - 137, The
seeded Cal card in the accessibility fixture currently points at a live
third-party endpoint, which makes the unattended axe run depend on external
network and markup. Update the fixture in accessibility.ts so the card uses a
deterministic test-owned mocked Cal route or endpoint, or remove/exclude the
live embed from this no-network setup. Use the existing card seed shape around
createId() and the calUserName/calHost fields to keep the fixture stable without
reaching out to Cal.com.
Comment on lines
+253
to
+287
| // Patch Arabic translations into the headline/subheader/labels of every block | ||
| // element so axe scans real RTL text. Falls back to the default text for keys we | ||
| // do not translate; rendering direction is driven by the `ar` code regardless. | ||
| const survey = await prisma.survey.findUnique({ where: { id: surveyId }, select: { blocks: true } }); | ||
| const blocks = (survey?.blocks ?? []) as Prisma.JsonArray; | ||
|
|
||
| const localize = (value: unknown): unknown => { | ||
| if (value && typeof value === "object" && "default" in (value as Record<string, unknown>)) { | ||
| const i18n = value as Record<string, string>; | ||
| return { ...i18n, ar: `${i18n.default} (مرحبا)` }; | ||
| } | ||
| return value; | ||
| }; | ||
|
|
||
| const patchedBlocks = blocks.map((block) => { | ||
| if (!block || typeof block !== "object") return block; | ||
| const b = block as Record<string, unknown>; | ||
| const elements = Array.isArray(b.elements) ? b.elements : []; | ||
| const patchedElements = elements.map((element) => { | ||
| if (!element || typeof element !== "object") return element; | ||
| const e = { ...(element as Record<string, unknown>) }; | ||
| if (e.headline) e.headline = localize(e.headline); | ||
| if (e.subheader) e.subheader = localize(e.subheader); | ||
| if (Array.isArray(e.choices)) { | ||
| e.choices = e.choices.map((choice) => { | ||
| if (!choice || typeof choice !== "object") return choice; | ||
| const c = { ...(choice as Record<string, unknown>) }; | ||
| if (c.label) c.label = localize(c.label); | ||
| return c; | ||
| }); | ||
| } | ||
| return e; | ||
| }); | ||
| return { ...b, elements: patchedElements }; | ||
| }); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Patch all label-bearing fields for the RTL fixture.
The Arabic walk currently leaves fields like placeholder, rating lowerLabel / upperLabel, and matrix rows / columns labels default-only, so the RTL fixture only partially exercises localized content.
Proposed patching expansion
- if (e.headline) e.headline = localize(e.headline);
- if (e.subheader) e.subheader = localize(e.subheader);
- if (Array.isArray(e.choices)) {
- e.choices = e.choices.map((choice) => {
+ for (const key of ["headline", "subheader", "placeholder", "lowerLabel", "upperLabel", "buttonLabel"]) {
+ if (e[key]) e[key] = localize(e[key]);
+ }
+ for (const key of ["choices", "rows", "columns"]) {
+ if (!Array.isArray(e[key])) continue;
+ e[key] = e[key].map((choice) => {
if (!choice || typeof choice !== "object") return choice;
const c = { ...(choice as Record<string, unknown>) };
if (c.label) c.label = localize(c.label);
return c;
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Patch Arabic translations into the headline/subheader/labels of every block | |
| // element so axe scans real RTL text. Falls back to the default text for keys we | |
| // do not translate; rendering direction is driven by the `ar` code regardless. | |
| const survey = await prisma.survey.findUnique({ where: { id: surveyId }, select: { blocks: true } }); | |
| const blocks = (survey?.blocks ?? []) as Prisma.JsonArray; | |
| const localize = (value: unknown): unknown => { | |
| if (value && typeof value === "object" && "default" in (value as Record<string, unknown>)) { | |
| const i18n = value as Record<string, string>; | |
| return { ...i18n, ar: `${i18n.default} (مرحبا)` }; | |
| } | |
| return value; | |
| }; | |
| const patchedBlocks = blocks.map((block) => { | |
| if (!block || typeof block !== "object") return block; | |
| const b = block as Record<string, unknown>; | |
| const elements = Array.isArray(b.elements) ? b.elements : []; | |
| const patchedElements = elements.map((element) => { | |
| if (!element || typeof element !== "object") return element; | |
| const e = { ...(element as Record<string, unknown>) }; | |
| if (e.headline) e.headline = localize(e.headline); | |
| if (e.subheader) e.subheader = localize(e.subheader); | |
| if (Array.isArray(e.choices)) { | |
| e.choices = e.choices.map((choice) => { | |
| if (!choice || typeof choice !== "object") return choice; | |
| const c = { ...(choice as Record<string, unknown>) }; | |
| if (c.label) c.label = localize(c.label); | |
| return c; | |
| }); | |
| } | |
| return e; | |
| }); | |
| return { ...b, elements: patchedElements }; | |
| }); | |
| // Patch Arabic translations into the headline/subheader/labels of every block | |
| // element so axe scans real RTL text. Falls back to the default text for keys we | |
| // do not translate; rendering direction is driven by the `ar` code regardless. | |
| const survey = await prisma.survey.findUnique({ where: { id: surveyId }, select: { blocks: true } }); | |
| const blocks = (survey?.blocks ?? []) as Prisma.JsonArray; | |
| const localize = (value: unknown): unknown => { | |
| if (value && typeof value === "object" && "default" in (value as Record<string, unknown>)) { | |
| const i18n = value as Record<string, string>; | |
| return { ...i18n, ar: `${i18n.default} (مرحبا)` }; | |
| } | |
| return value; | |
| }; | |
| const patchedBlocks = blocks.map((block) => { | |
| if (!block || typeof block !== "object") return block; | |
| const b = block as Record<string, unknown>; | |
| const elements = Array.isArray(b.elements) ? b.elements : []; | |
| const patchedElements = elements.map((element) => { | |
| if (!element || typeof element !== "object") return element; | |
| const e = { ...(element as Record<string, unknown>) }; | |
| for (const key of ["headline", "subheader", "placeholder", "lowerLabel", "upperLabel", "buttonLabel"]) { | |
| if (e[key]) e[key] = localize(e[key]); | |
| } | |
| for (const key of ["choices", "rows", "columns"]) { | |
| if (!Array.isArray(e[key])) continue; | |
| e[key] = e[key].map((choice) => { | |
| if (!choice || typeof choice !== "object") return choice; | |
| const c = { ...(choice as Record<string, unknown>) }; | |
| if (c.label) c.label = localize(c.label); | |
| return c; | |
| }); | |
| } | |
| return e; | |
| }); | |
| return { ...b, elements: patchedElements }; | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/playwright/utils/accessibility.ts` around lines 253 - 287, The RTL
fixture only patches headline/subheader/choices today, so several label-bearing
fields still stay in default language. Update the localization pass in
accessibility.ts within the block/element patching logic to also run localize
over placeholder, rating lowerLabel/upperLabel, and matrix rows/columns label
fields wherever they appear in the element shapes. Keep the existing
recursive-style cloning approach in patchedBlocks/patchedElements so axe sees
fully localized Arabic content across all block types.
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.
This PR adds a basic integration of the AXE library, for accessibility tests.
It also enables agents to look up in additional (local) context dirs.
Closes ENG-1588