Skip to content

fix(action-card): make action card focusable and keyboard activatable #2598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lakshmi-priya-b wants to merge 4 commits into
base: main
Choose a base branch
from
+107 −39
Open

fix(action-card): make action card focusable and keyboard activatable #2598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5aa12a3
fix(action-card): make action card focusable and keyboard activatable
lakshmi-priya-b Jun 12, 2026
ff31bd9
fix sonarqube issue and add axe tests
lakshmi-priya-b Jun 15, 2026
d4e88e8
fix(action-card): use native button for interactive cards to improve …
lakshmi-priya-b Jun 21, 2026
f1ad162
remove aria-pressed attribute from the button
lakshmi-priya-b Jun 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion packages/core/src/components/action-card/action-card.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,38 @@
width: 13.375rem;
height: 7.5rem;
min-height: 7.5rem;
cursor: pointer;

margin: 0.25rem 0;

@include component.ix-component;
}

button.card-button {
all: unset;
display: block;
box-sizing: border-box;
width: 100%;
height: 100%;
cursor: pointer;

ix-card {
width: 100%;
height: 100%;
}

&:focus-visible {
outline: var(--theme-focus-border-thickness) solid
var(--theme-color-focus-bdr);
outline-offset: var(--theme-focus-outline-offset);
border-radius: var(--theme-card--border-radius);
}

:host(.passive) & {
pointer-events: none;
cursor: default;
}

:host(:not(.passive)) &:active ix-card {
background-color: var(--theme-color-ghost--active);
Comment on lines +44 to +50

@coderabbitai coderabbitai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Passive-state CSS selectors won’t match the component’s passive attribute.

Line 44 and Line 49 target :host(.passive), but the passive state is used as an attribute (passive), so these branches won’t apply reliably and passive styling/interaction gating can regress.

🔧 Proposed fix 
-  :host(.passive) & {
+  :host([passive]) & {
     pointer-events: none;
     cursor: default;
   }

-  :host(:not(.passive)) &:active ix-card {
+  :host(:not([passive])) &:active ix-card {
     background-color: var(--theme-color-ghost--active);
   }
📝 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
:host(.passive) & {
pointer-events: none;
cursor: default;
}
:host(:not(.passive)) &:active ix-card {
background-color: var(--theme-color-ghost--active);
:host([passive]) & {
pointer-events: none;
cursor: default;
}
:host(:not([passive])) &:active ix-card {
background-color: var(--theme-color-ghost--active);
}
🤖 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 44 -
50, The CSS selectors in the action-card.scss file are using the class selector
notation (`:host(.passive)`) but the passive state is applied as an attribute
rather than a class. Update the selectors `:host(.passive)` on line 44 and
`:host(:not(.passive))` on line 49 to use attribute selector notation instead,
changing them to `:host([passive])` and `:host(:not([passive]))` respectively.
This will ensure the passive styling and interaction rules apply correctly based
on the passive attribute.

}
}
81 changes: 43 additions & 38 deletions packages/core/src/components/action-card/action-card.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,48 +75,53 @@ export class IxActionCard {

return (
<Host>
<ix-card
selected={this.selected}
variant={this.variant}
passive={this.passive}
class={'pointer'}
<button
class="card-button"
tabIndex={this.passive ? -1 : 0}
aria-label={this.ariaLabelCard}
aria-labelledby={ariaLabelledBy}
role={ariaLabelledBy ? 'group' : undefined}
>
<ix-card-content>
{this.icon ? (
<ix-icon
class={'icon'}
name={this.icon}
size="32"
aria-label={
this.ariaLabelIcon || getFallbackLabelFromIconName(this.icon)
}
></ix-icon>
) : null}
<div>
{this.heading ? (
<ix-typography
id="ix-action-card-heading"
aria-hidden={a11yBoolean(!ariaLabelledBy)}
format="h4"
>
{this.heading}
</ix-typography>
<ix-card
selected={this.selected}
variant={this.variant}
passive={this.passive}
class={'pointer'}
>
<ix-card-content>
{this.icon ? (
<ix-icon
class={'icon'}
name={this.icon}
size="32"
aria-label={
this.ariaLabelIcon ||
getFallbackLabelFromIconName(this.icon)
}
></ix-icon>
) : null}
{this.subheading ? (
<ix-typography
format="h5"
text-color={this.getSubheadingTextColor()}
>
{this.subheading}
</ix-typography>
) : null}
<slot></slot>
</div>
</ix-card-content>
</ix-card>
<div>
{this.heading ? (
<ix-typography
id="ix-action-card-heading"
aria-hidden={a11yBoolean(!ariaLabelledBy)}
format="h4"
>
{this.heading}
</ix-typography>
) : null}
{this.subheading ? (
<ix-typography
format="h5"
text-color={this.getSubheadingTextColor()}
>
{this.subheading}
</ix-typography>
) : null}
<slot></slot>
</div>
</ix-card-content>
</ix-card>
</button>
</Host>
);
}
Expand Down
39 changes: 39 additions & 0 deletions packages/core/src/components/action-card/test/action-card.ct.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* SPDX-FileCopyrightText: 2024 Siemens AG
*
* SPDX-License-Identifier: MIT
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import { expect } from '@playwright/test';
import { regressionTest } from '@utils/test';

regressionTest.describe('accessibility', () => {
regressionTest('default', async ({ mount, makeAxeBuilder }) => {
await mount(`
<ix-action-card
heading="Scan for new devices"
subheading="Secondary text"
></ix-action-card>
`);

const accessibilityScanResults = await makeAxeBuilder().analyze();

expect(accessibilityScanResults.violations).toEqual([]);
});

regressionTest('passive', async ({ mount, makeAxeBuilder }) => {
await mount(`
<ix-action-card
heading="Scan for new devices"
subheading="Secondary text"
passive
></ix-action-card>
`);

const accessibilityScanResults = await makeAxeBuilder().analyze();

expect(accessibilityScanResults.violations).toEqual([]);
});
Comment on lines +13 to +38

@coderabbitai coderabbitai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Add keyboard behavior regression tests for Enter/Space and passive mode.

The PR’s core change is keyboard activatability, but this file does not assert Enter/Space click activation or passive-mode non-activation, so the behavior can regress undetected.

⌨️ Proposed behavior tests 
+regressionTest('keyboard activates on Enter and Space', async ({ mount, page }) => {
+  await mount(`
+    <ix-action-card heading="Scan for new devices" subheading="Secondary text"></ix-action-card>
+  `);
+
+  const host = page.locator('ix-action-card');
+  await expect(host).toHaveClass(/\bhydrated\b/);
+
+  const button = page.getByRole('button');
+  await button.focus();
+
+  const clickSpy = page.waitForEvent('console'); // replace with component click-event listener helper used in this repo
+  await page.keyboard.press('Enter');
+  await page.keyboard.press('Space');
+  await clickSpy;
+});
+
+regressionTest('passive is not keyboard-focusable', async ({ mount, page }) => {
+  await mount(`
+    <ix-action-card heading="Scan for new devices" subheading="Secondary text" passive></ix-action-card>
+  `);
+
+  await expect(page.locator('ix-action-card')).toHaveClass(/\bhydrated\b/);
+  await expect(page.getByRole('button')).toHaveAttribute('tabindex', '-1');
+});

As per coding guidelines, “Follow this test order within each file … 4. Behavior tests (user interactions, state 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/test/action-card.ct.ts` around lines
13 - 38, The existing tests only validate accessibility violations but do not
test the keyboard interaction behavior that is the core change in this PR. Add
new regression tests after the existing 'default' and 'passive' accessibility
test cases to verify keyboard activation behavior. Create a behavior test for
the default mode that mounts an ix-action-card component, simulates pressing
Enter or Space keys, and asserts that the activation is triggered (by checking
for a click event or state change). Then create another behavior test for
passive mode that verifies pressing Enter or Space keys does NOT trigger any
activation. Follow the test ordering guideline by placing these behavior tests
(user interactions) after the accessibility violation tests.

Source: Coding guidelines

});
Comment on lines +12 to +39

@coderabbitai coderabbitai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Add the required renders test and hydration assertions.

This file currently runs axe scans only. It is missing the mandatory renders test, and hydration is not asserted before accessibility checks.

✅ Proposed test additions 
 regressionTest.describe('accessibility', () => {
   regressionTest('default', async ({ mount, makeAxeBuilder, page }) => {
     await mount(`
       <ix-action-card
         heading="Scan for new devices"
         subheading="Secondary text"
       ></ix-action-card>
     `);

+    await expect(page.locator('ix-action-card')).toHaveClass(/\bhydrated\b/);
+
     const accessibilityScanResults = await makeAxeBuilder().analyze();

     expect(accessibilityScanResults.violations).toEqual([]);
   });
@@
     await mount(`
       <ix-action-card
         heading="Scan for new devices"
         subheading="Secondary text"
         passive
       ></ix-action-card>
     `);

+    await expect(page.locator('ix-action-card')).toHaveClass(/\bhydrated\b/);
+
     const accessibilityScanResults = await makeAxeBuilder().analyze();

     expect(accessibilityScanResults.violations).toEqual([]);
   });
+
+  regressionTest('renders', async ({ mount, page }) => {
+    await mount(`
+      <ix-action-card
+        heading="Scan for new devices"
+        subheading="Secondary text"
+      ></ix-action-card>
+    `);
+
+    await expect(page.locator('ix-action-card')).toHaveClass(/\bhydrated\b/);
+    await expect(page.getByRole('button')).toBeVisible();
+  });
 });

As per coding guidelines, “Every test file should include a 'renders' test verifying basic hydration” and “Always verify components are hydrated before making further assertions … /\bhydrated\b/”; as per path instructions for packages/core/src/components/**/test/*.ct.ts, tests must include both accessibility and renders hydration coverage.

🤖 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/test/action-card.ct.ts` around lines
12 - 39, Add a mandatory `renders` test before the accessibility describe block
that verifies the ix-action-card component hydrates correctly without any
attributes. In both the 'default' and 'passive' accessibility tests, add a
hydration assertion after mounting the ix-action-card component but before
calling makeAxeBuilder().analyze(), ensuring the component is fully hydrated
before performing the accessibility scan. The hydration check should verify that
the mounted element contains the expected hydrated state before running axe
accessibility tests.

Sources: Coding guidelines, Path instructions

Loading