fix(action-card): render a button for proper accessibility#2625
fix(action-card): render a button for proper accessibility#2625spike-rabbit wants to merge 1 commit into
Conversation
✅ Deploy Preview for ix-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthrough
ChangesAction Card Accessibility Fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the accessibility of the ix-action-card component by wrapping its content in a button element to make it keyboard-focusable. The feedback highlights critical improvements to this implementation: explicitly setting type='button' to prevent accidental form submissions, conditionally rendering the button wrapper only when the card is active (not passive), and resetting default browser styles on the button to avoid layout issues. Additionally, in accordance with the repository style guide, the reviewer requests adding an axe-based component test to verify the accessibility changes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/components/action-card/action-card.scss`:
- Around line 25-35: The `button` wrapper in `action-card.scss` is now the
containing box for `ix-card`, but it does not inherit the host size, so the card
can shrink to content instead of filling the available space. Update the
`button` rule in `action-card.scss` to size the wrapper itself to the full
container dimensions, keeping the existing `button` and `ix-card` structure
intact and preserving the current focus styles.
In `@packages/core/src/components/action-card/action-card.tsx`:
- Around line 78-121: The new native button wrapper in ActionCard still renders
an unrestricted slot through ix-card-content, which can place arbitrary consumer
controls inside a button and create invalid nested interactive content. Update
ActionCard’s render structure so the slot is either constrained to
non-interactive content or moved outside the native button, and verify the
aria-label/aria-labelledby behavior in action-card.tsx still works with ix-card,
ix-icon, and ix-typography.
- Around line 78-85: The accessible name is currently applied to the inner
ix-card instead of the focusable button in action-card, which leaves the
interactive control unnamed. Move aria-label and/or aria-labelledby from ix-card
onto the wrapping button in action-card.tsx, and keep ix-card purely
presentational so ariaLabelCard still names the actual button.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 231c4dac-615f-4705-aee5-02d23323b42e
📒 Files selected for processing (3)
.changeset/pink-sloths-drop.mdpackages/core/src/components/action-card/action-card.scsspackages/core/src/components/action-card/action-card.tsx
spike-rabbit
force-pushed
the
fix/action-card/render-as-button
branch
from
589663b to
3dcbc1d
Compare
🦋 Changeset detectedLatest commit: 8ea02ce The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/core/src/components/action-card/action-card.tsx (1)
90-120: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftThe native
<button>still wraps unrestricted slotted content.
<slot>can project links, buttons, or inputs into this subtree, which creates invalid interactive descendants inside a native button and breaks keyboard/assistive-technology behavior. This needs a slot contract change or a different DOM structure before shipping.As per path instructions, prioritize accessibility regressions in
packages/core/**changes.🤖 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 `@packages/core/src/components/action-card/action-card.tsx` around lines 90 - 120, The ActionCard content currently renders unrestricted slotted children inside the native button structure in action-card.tsx, which can create invalid interactive descendants. Update the ActionCard DOM/slot contract so the slot no longer lives inside the clickable <button> subtree, or otherwise restrict the slot to non-interactive content; use the ActionCard render path and its heading/subheading/content structure to relocate or refactor the slot while preserving accessibility.Source: Path instructions
🤖 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 `@packages/core/src/components/action-card/action-card.scss`:
- Around line 25-39: The ActionCard still shows a pointer cursor on
passive/disabled states because the host-level cursor styling is not overridden
by the new button rule. Update the action-card styling so the disabled/passive
state explicitly resets the cursor on the button (or host) when the card is
non-interactive, using the existing ActionCard/`:host`/`button` selectors and
the `[disabled]` state to ensure it no longer advertises clickability.
---
Duplicate comments:
In `@packages/core/src/components/action-card/action-card.tsx`:
- Around line 90-120: The ActionCard content currently renders unrestricted
slotted children inside the native button structure in action-card.tsx, which
can create invalid interactive descendants. Update the ActionCard DOM/slot
contract so the slot no longer lives inside the clickable <button> subtree, or
otherwise restrict the slot to non-interactive content; use the ActionCard
render path and its heading/subheading/content structure to relocate or refactor
the slot while preserving accessibility.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 76ba6c5b-887c-43c0-925a-ef9c4d453aec
📒 Files selected for processing (4)
.changeset/pink-sloths-drop.mdpackages/core/src/components/action-card/action-card.scsspackages/core/src/components/action-card/action-card.tsxpackages/core/src/components/action-card/test/action-card.ct.ts
spike-rabbit
force-pushed
the
fix/action-card/render-as-button
branch
2 times, most recently
from
8e5afe0 to
e0d7c2a
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/core/src/components/action-card/action-card.tsx (1)
81-127: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftThe native button still accepts arbitrary slotted interactive content.
The new note documents the restriction, but this render path still allows consumers to project links, buttons, or inputs into the
<slot>, which creates invalid descendants of a native<button>and can break keyboard/assistive-technology behavior. Please either move the slot outside the button or enforce a non-interactive slot contract before shipping this as a patch. As per path instructions, "Prioritize correctness, regressions, accessibility, release impact, and missing validation."🤖 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 `@packages/core/src/components/action-card/action-card.tsx` around lines 81 - 127, The action-card render path still places the default <slot> inside a native <button>, so arbitrary slotted interactive descendants remain allowed and can create invalid nested controls. Update the ActionCardComponent render logic to either move the <slot> outside the button or otherwise prevent interactive projected content from being rendered inside the button, while keeping the existing ix-card, ix-typography, and icon structure intact.Source: Path instructions
packages/core/src/components/action-card/action-card.scss (1)
25-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPassive cards still advertise clickability.
Line 18 keeps
cursor: pointer, and the newbuttonrule never overrides it for[disabled], so passive cards still show a pointer cursor.Suggested fix
button { width: 100%; height: 100%; display: block; background: transparent; padding: 0; border: 0; border-radius: var(--theme-btn--border-radius); box-shadow: initial; + &:disabled { + cursor: default; + } + &:focus-visible { outline: 1px solid var(--theme-color-focus-bdr); outline-offset: var(--theme-btn--focus--outline-offset); } }🤖 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 `@packages/core/src/components/action-card/action-card.scss` around lines 25 - 39, The `action-card` styles keep the pointer cursor on the wrapper even when the inner `button` is disabled, so passive cards still look clickable. Update the `button` rule in `action-card.scss` to override the inherited cursor for the disabled state, ideally targeting the disabled button state within the same `button` selector so the card no longer advertises clickability when it cannot be interacted with.
🤖 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.
Duplicate comments:
In `@packages/core/src/components/action-card/action-card.scss`:
- Around line 25-39: The `action-card` styles keep the pointer cursor on the
wrapper even when the inner `button` is disabled, so passive cards still look
clickable. Update the `button` rule in `action-card.scss` to override the
inherited cursor for the disabled state, ideally targeting the disabled button
state within the same `button` selector so the card no longer advertises
clickability when it cannot be interacted with.
In `@packages/core/src/components/action-card/action-card.tsx`:
- Around line 81-127: The action-card render path still places the default
<slot> inside a native <button>, so arbitrary slotted interactive descendants
remain allowed and can create invalid nested controls. Update the
ActionCardComponent render logic to either move the <slot> outside the button or
otherwise prevent interactive projected content from being rendered inside the
button, while keeping the existing ix-card, ix-typography, and icon structure
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f9944493-a756-4c34-b3b2-a10c175ec654
📒 Files selected for processing (4)
.changeset/pink-sloths-drop.mdpackages/core/src/components/action-card/action-card.scsspackages/core/src/components/action-card/action-card.tsxpackages/core/src/components/action-card/test/action-card.ct.ts
Wraps the inner `ix-card` of the `ix-action-card` in a `button`. This is aligned with the element approach.
spike-rabbit
force-pushed
the
fix/action-card/render-as-button
branch
from
e0d7c2a to
8ea02ce
Compare
|
1 participant
Description
Wraps the inner
ix-cardofix-action-cardin abuttonelement so the card is keyboard-focusable and exposed as a button to assistive technologies.User-facing impact
ix-action-cardis now properly accessible: it is focusable and announced as a button.Affected packages
@siemens/ix(patch)Validation
@siemens/ixpatch)Release impact
Patch — accessibility bug fix, no breaking change.
Summary by CodeRabbit
ix-action-cardby rendering the card as a native<button>with focus-visible styling.aria-label/aria-labelledby).passiveis set, the card’s button is disabled.passivevariants.ix-action-cardaccessibility improvements.