fix(a11y): WCAG 2.1 accessibility improvements across Icon component and demo - #6
Merged
Merged
Conversation
- Rename _id to id so symbol/use references resolve correctly (bug fix) - Rename _normalizedBody to normalizedBody to match template usage (bug fix) - Add focusable='false' to prevent SVGs from being keyboard-focusable in IE/Edge and some Chromium builds (WCAG 2.1.1) - Decorative icons (no title prop) receive aria-hidden='true' so assistive technologies skip them (WCAG 1.1.1) - Meaningful icons (with title prop) receive role='img' so screen readers expose the title as the accessible name (WCAG 4.1.2) - Associate desc element with a stable id for future aria-describedby wiring (WCAG 1.1.1)
…se layout - Wrap slot in <main id='main-content'> to provide a navigation landmark that keyboard and AT users can target (WCAG 1.3.6, 2.4.1) - Add skip link that becomes visible on focus so keyboard users can bypass repeated header content (WCAG 2.4.1 - Level A) - Add meta description for better AT context and SEO - Skip link uses tabindex='-1' on <main> to allow programmatic focus without disrupting natural tab order
…cs in demo pages - Add explicit imports for Base layout and Icon component to replace implicit globals; makes component contracts clear and auditable - Fix variable name: _icon -> icon so the runtime reference resolves - Add title prop to standalone icons so they are exposed as role='img' with an accessible name to screen readers (WCAG 1.1.1) - Wrap star icons in <figure aria-label='Rating: 4 out of 5 stars'> with a visually-hidden <figcaption> so the group meaning is communicated to AT users (WCAG 1.1.1, 1.3.1) - Add .visually-hidden utility class following the standard SR-only clip pattern
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR enhances icon accessibility across the component library and demo. The core Icon component now conditionally applies ARIA attributes based on whether a title is provided, generates stable IDs for description linking, and enforces focusable="false". The demo layout adds a skip-to-main-content navigation link and document metadata, while demo pages illustrate proper icon accessibility patterns using title props and CSS utility classes for visually-hidden content. ChangesIcon Accessibility Enhancement
Sequence DiagramsequenceDiagram
participant DemoPage as Demo Page
participant IconComponent as Icon Component
participant DOMOutput as DOM Output
DemoPage->>IconComponent: Props (icon, title?)
IconComponent->>IconComponent: Generate ID from collection/name
alt title provided
IconComponent->>IconComponent: Set role="img"
IconComponent->>DOMOutput: Render with aria-label/title
else no title
IconComponent->>IconComponent: Set aria-hidden="true"
IconComponent->>DOMOutput: Render as decorative
end
IconComponent->>DOMOutput: Add desc with matching ID
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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 |
actions/checkout on pull_request events checks out the merge commit,
leaving the runner in detached HEAD state. A plain 'git push' then
fails with exit 128 because there is no active branch to push to.
Fix: use 'git push origin HEAD:${HEAD_REF}' when github.head_ref is
set (PR context), falling back to plain 'git push' on direct pushes
to main where HEAD is always attached.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@demo/src/pages/map.astro`:
- Around line 15-19: The per-star title prop on the Icon instances inside the
Array.from(...) in map.astro makes each star announced separately; remove the
per-star title and mark the icons decorative instead—either pass
aria-hidden="true" to each <Icon> or add a boolean decorative prop (e.g.,
decorative={true}) and update Icon.astro (which currently sets role="img") to
render role="img" only for non-decorative icons and use aria-hidden or
role="presentation" for decorative ones so screen readers only read the overall
rating/figure.
In `@packages/core/components/Icon.astro`:
- Around line 116-125: The SVG never references the stable description id
created (desc id), so when a description exists and the icon is not decorative
you must add an aria-describedby attribute to the normalizedProps so the <svg>
is programmatically bound to the <desc>; update the normalizedProps object (used
to render the <svg>) to include something like aria-describedby: descId (only
when descId is defined and isDecorative is false) while keeping the existing
aria-hidden/role logic in the isDecorative branch and ensuring the attribute is
applied alongside focusable and other attrs in normalizedProps.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 59b9c41b-dc92-4105-890c-c7d4a0ff89b4
📒 Files selected for processing (4)
demo/src/layouts/base.astrodemo/src/pages/index.astrodemo/src/pages/map.astropackages/core/components/Icon.astro
- Add biome-ignore on normalizedBody variable (used in Astro template, outside Biome JS scope) to prevent false unused-variable warning - Apply biome import order: sort imports alphabetically in demo pages - Apply biome ternary formatting in Icon.astro (cosmetic only) - Use --force-with-lease on push to guard against concurrent pushes while still protecting against accidental overwrite of others' work
- Icon.astro: add aria-describedby to normalizedProps when desc is provided and icon is meaningful, binding <svg> to its <desc id> (WCAG 1.1.1 / 4.1.2) - map.astro: remove per-star title props so each icon is decorative; the <figure aria-label> and <figcaption> carry the collective meaning and screen readers no longer announce each star separately
|
yacosta738
added a commit
that referenced
this pull request
May 16, 2026
- Add explicit permissions blocks to all CI jobs (actions/missing-workflow-permissions) - lint job: contents: write (needed for git push on auto-fix) - smoke job: contents: read (read-only, follows least privilege) - Add pnpm override for yaml >= 2.8.3 to patch stack overflow vulnerability (GHSA-xxx, yaml >= 2.0.0 < 2.8.3) Closes code-scanning alerts #6 and #7 Closes dependabot alert #13
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.
Summary
This PR fixes several WCAG 2.1 accessibility violations found in the
Iconcomponent and the demo pages. Changes are grouped into three focused commits for easier review.Issues Fixed
packages/core/components/Icon.astro— Critical_idvariable used in template but computed as_id—<symbol>and<use>referenced an undefinedid_id→id,_normalizedBody→normalizedBodyaria-hidden="true"when notitleprop is providedrole="img"whentitleprop is providedfocusable="false"unconditionallydemo/src/layouts/base.astro— Serious<main>landmark — AT users couldn't navigate to main content<slot />in<main id="main-content" tabindex="-1">#main-content<meta name="description">demo/src/pages/index.astroandmap.astro— Moderatetitle— invisible to ATtitleprop to each meaningful icon_iconvariable shadowed — runtime reference would fail_icon→icon<figure aria-label="Rating: 4 out of 5 stars">+ visually-hidden<figcaption>importstatements forBaseandIconHow to Verify
Keyboard
<main>Screen reader (VoiceOver / NVDA)
titlemust be announced as"<title text>, image""Rating: 4 out of 5 stars"(or equivalent)Automated
WCAG References
Accessibility review:
role="img"+titlefor meaningful icons;aria-hiddenfor decorative<main>landmark;focusable="false"on SVGs