Skip to content

fix: Disable Ionic-internal axe rules breaking the accessibility tests#253

Merged
HTMHell merged 3 commits into
masterfrom
fix/a11y-role-img-alt-ion-icon
Jun 10, 2026
Merged

fix: Disable Ionic-internal axe rules breaking the accessibility tests#253
HTMHell merged 3 commits into
masterfrom
fix/a11y-role-img-alt-ion-icon

Conversation

@HTMHell

@HTMHell HTMHell commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

The build workflow started failing on the Test code step: a jasmine-axe/axe-core bump began enforcing two axe rules that flag Ionic-internal elements which have no authorable alternative text:

  • role-img-alt — Ionic renders <ion-icon> as role="img" (e.g. book, arrow-forward, dice-outline)
  • aria-progressbar-name — Ionic renders <ion-progress-bar> as role="progressbar"

These are decorative, library-internal nodes, so the should pass accessibility test specs across many components broke.

This disables those two rules for the accessibility tests via a small setup spec (src/test-setup.spec.ts) that calls axe.configure(...) at module load — before any test runs, on the same axe-core instance jasmine-axe uses — so it applies globally without touching every spec. Same approach already used inline in about-benefits / settings-appearance-images and in rylo-translate.

Test plan

  • Verified locally that previously-failing specs (AboutApi, AboutAppearance, HumanPoseViewer, SettingsVoiceOutput, …) no longer report role-img-alt / aria-progressbar-name violations
  • CI build workflow green

Note

Low Risk
Changes affect test configuration and harness behavior only; production accessibility behavior is unchanged.

Overview
Restores CI Test code by aligning jasmine-axe accessibility checks with Ionic’s internal markup and tightening a few flaky specs.

A new src/test-setup.spec.ts runs axe.configure at module load (same axe-core instance as jasmine-axe) to disable role-img-alt and aria-progressbar-name globally, so component should pass accessibility test specs stop failing on decorative <ion-icon> / <ion-progress-bar> nodes without duplicating config in every spec. axe-core is added as an explicit devDependency.

transferable.spec.ts waits for the test image onload before assertions. avatar-pose-viewer.component.spec.ts uses xdescribe when WebGL is unavailable so headless environments skip the suite instead of erroring.

Reviewed by Cursor Bugbot for commit ffb7419. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Chores

    • Added accessibility testing tooling as a development dependency.
  • Tests

    • Relaxed specific accessibility rule checks in test setup for framework-rendered decorative/internal elements.
    • Made image-related tests wait for image load before assertions.
    • Conditioned a graphics-heavy test suite to skip when WebGL is unavailable.

A jasmine-axe/axe-core bump started enforcing role-img-alt and
aria-progressbar-name, which flag Ionic's <ion-icon> (role=img) and
<ion-progress-bar> (role=progressbar) elements that have no authorable alt text.
Globally disable those two rules for the accessibility tests via a setup spec
that configures axe-core before any test runs, fixing the failing build.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1f09a302-49ed-497c-81d2-1b5eabe6f59c

📥 Commits

Reviewing files that changed from the base of the PR and between e376646 and ffb7419.

📒 Files selected for processing (1)
  • src/app/pages/translate/pose-viewers/avatar-pose-viewer/avatar-pose-viewer.component.spec.ts

📝 Walkthrough

Walkthrough

Adds axe-core to devDependencies, configures global axe rule disables in test setup, awaits image loads in one test, and conditionally skips a WebGL-dependent test suite when WebGL is unavailable.

Changes

Accessibility Testing & Test Reliability

Layer / File(s) Summary
Axe-core dependency and configuration
package.json, src/test-setup.spec.ts
Adds axe-core (^4.12.0) to devDependencies and imports/configures it in the test setup to disable role-img-alt and aria-progressbar-name globally.
Image test async initialization
src/app/core/helpers/image/transferable.spec.ts
Converts beforeAll to an async setup that awaits img.onload via a Promise so image-dependent tests run after the source image is loaded.
WebGL gating for AvatarPoseViewer tests
src/app/pages/translate/pose-viewers/avatar-pose-viewer/avatar-pose-viewer.component.spec.ts
Adds a hasWebGL runtime check and uses it to describe or xdescribe the test suite depending on WebGL availability.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nibble tests with careful paws,
added axe to hush icon laws,
an image waits, then tests proceed,
WebGL checks skip when it's in need. 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: disabling specific axe-core accessibility rules that were incorrectly flagging Ionic-internal elements, which is the primary objective of this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/a11y-role-img-alt-ion-icon

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/test-setup.spec.ts (1)

1-13: ⚡ Quick win

Consider a more conventional test setup approach.

Using a .spec.ts file that contains no actual test cases is unconventional. Standard approaches for global test setup include:

  1. Dedicated setup file: Rename to test-setup.ts (without .spec) and import it explicitly in test files that need it, or
  2. Karma configuration: Add the setup to Karma's files array with {pattern: 'src/test-setup.ts', watched: false} to ensure it loads before all specs, or
  3. Angular test config: Use Angular's test setup hooks in test.ts or similar entry point

The current approach works only if Karma happens to load this file before others, but the .spec.ts extension signals "this contains tests" when it actually contains global configuration.

🤖 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 `@src/test-setup.spec.ts` around lines 1 - 13, The file uses a .spec.ts suffix
for global setup (the axe.configure(...) block) which is misleading and risks
load-order issues; rename the file to test-setup.ts (remove the .spec) and
ensure it is loaded explicitly by either importing it from your test entry
(e.g., test.ts) or adding it to the test runner config (Karma files array) so
the axe.configure({ rules: [...] }) call runs before any specs; update
references/imports accordingly and remove the .spec extension to reflect its
purpose.
🤖 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.

Nitpick comments:
In `@src/test-setup.spec.ts`:
- Around line 1-13: The file uses a .spec.ts suffix for global setup (the
axe.configure(...) block) which is misleading and risks load-order issues;
rename the file to test-setup.ts (remove the .spec) and ensure it is loaded
explicitly by either importing it from your test entry (e.g., test.ts) or adding
it to the test runner config (Karma files array) so the axe.configure({ rules:
[...] }) call runs before any specs; update references/imports accordingly and
remove the .spec extension to reflect its purpose.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7b568013-91b9-4248-8b64-381fee444ab8

📥 Commits

Reviewing files that changed from the base of the PR and between d6c4068 and 274d307.

📒 Files selected for processing (2)
  • package.json
  • src/test-setup.spec.ts

HTMHell and others added 2 commits June 10, 2026 14:51
The 'same data for images and canvas' test read image data before the <img> had
decoded its data URL, which is reliable on Chrome but fails on Firefox. Await
img.onload before the comparison (matches rylo-translate).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In headless CI without a GPU, THREE.js fails to create a WebGL context and the
suite crashes with "Cannot read properties of undefined (reading 'xr')". Guard
the suite behind a WebGL availability check (matches rylo-translate).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Visit the preview URL for this PR (updated for commit ffb7419):

https://translate-sign-mt--pr253-fix-a11y-role-img-al-5juc888g.web.app

(expires Wed, 17 Jun 2026 11:57:54 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 739446cfe7a349700ebd347d2a39e3b90ba24425

@HTMHell HTMHell merged commit 68a2741 into master Jun 10, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants