Skip to content

feat: adds alt prop for accessibility #39

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
anechol wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: adds alt prop for accessibility #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions src/components.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
import { HTMLStencilElement, JSXBase } from "@stencil/core/internal";
export namespace Components {
interface PdsIcon {
/**
* The `alt` attribute of the icon.
*/
"alt"?: string;

@pixelflips pixelflips May 5, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Have we considered using aria-hidden="true" instead of alt="" and role="img"? Since we typically use them as just decorative icons, it might avoid sending mixed signals to screen readers.

I briefly tried to find any usage of standalone icons (outside of button, link, etc), but haven't so far. Just an additional thought, and I may not fully be understanding the problem with the image role and the aria-label.

@anechol anechol May 5, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This can be an option, yes. There's different avenues we can pursue for this. I added a quick meeting on the books to discuss the broader aspect of accessibility in Pine with our inherited attributes utility.

/**
* The color of the icon
*/
Expand Down Expand Up @@ -51,6 +55,10 @@ declare global {
}
declare namespace LocalJSX {
interface PdsIcon {
/**
* The `alt` attribute of the icon.
*/
"alt"?: string;
/**
* The color of the icon
*/
Expand Down
7 changes: 6 additions & 1 deletion src/components/pds-icon/pds-icon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ export class PdsIcon {
@State() private isVisible = false;
@State() private svgContent?: string;

/**
* The `alt` attribute of the icon.
*/
@Prop() alt?: string;

/**
*
* The color of the icon
Expand Down Expand Up @@ -157,7 +162,7 @@ export class PdsIcon {

<Host
aria-label={ariaLabel !== undefined && !this.hasAriaHidden() ? ariaLabel : null }
alt=""
alt={this.alt}
role="img"
class={{
...createColorClasses(this.color),
Expand Down
1 change: 1 addition & 0 deletions src/components/pds-icon/readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

| Property | Attribute | Description | Type | Default |
| --------- | ---------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | --------- | ----------- |
| `alt` | `alt` | The `alt` attribute of the icon. | `string` | `undefined` |
| `color` | `color` | The color of the icon | `string` | `undefined` |
| `flipRtl` | `flip-rtl` | Determines if the icon should be flipped when the `dir` is right-to-left (`"rtl"`). This is automatically enabled for icons that are in the `ICONS_TO_FLIP` list and when the `dir` is `"rtl"`. If `flipRtl` is set to `false`, the icon will not be flipped even if the `dir` is `"rtl"`. | `boolean` | `undefined` |
| `icon` | `icon` | This is a combination of both `name` and `src`. If a `src` URL is detected, it will set the `src` property. Otherwise it assumes it's a built-in named SVG and sets the `name` property. | `any` | `undefined` |
Expand Down
30 changes: 22 additions & 8 deletions src/components/pds-icon/test/pds-icon.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('pds-icon', () => {
html: '<pds-icon></pds-icon>',
});
expect(root).toEqualHtml(`
<pds-icon alt="" role="img" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: currentColor;">
<pds-icon role="img" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: currentColor;">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
Expand All @@ -22,21 +22,35 @@ describe('pds-icon', () => {
html: '<pds-icon size="small"></pds-icon>',
});
expect(root).toEqualHtml(`
<pds-icon alt="" role="img" size="small" style="--dimension-icon-height: 12px; --dimension-icon-width: 12px; --color-icon-fill: currentColor;">
<pds-icon role="img" size="small" style="--dimension-icon-height: 12px; --dimension-icon-width: 12px; --color-icon-fill: currentColor;">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
</pds-icon>
`);
});

it('renders the alt attribute when prop is set', async () => {
const { root } = await newSpecPage({
components: [PdsIcon],
html: `<pds-icon alt="Alternative text"></pds-icon>`,
})
expect(root).toEqualHtml(`
<pds-icon alt="Alternative text" role="img" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: currentColor;">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
</pds-icon>
`)
})

it('allows custom size', async () => {
const { root } = await newSpecPage({
components: [PdsIcon],
html: '<pds-icon size="32px"></pds-icon>',
});
expect(root).toEqualHtml(`
<pds-icon alt="" role="img" size="32px" style="--dimension-icon-height: 32px; --dimension-icon-width: 32px; --color-icon-fill: currentColor;">
<pds-icon role="img" size="32px" style="--dimension-icon-height: 32px; --dimension-icon-width: 32px; --color-icon-fill: currentColor;">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
Expand All @@ -50,7 +64,7 @@ describe('pds-icon', () => {
html: '<pds-icon name="archive"></pds-icon>',
});
expect(root).toEqualHtml(`
<pds-icon alt="" aria-label="archive" name="archive" role="img" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: currentColor;">
<pds-icon aria-label="archive" name="archive" role="img" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: currentColor;">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
Expand All @@ -64,7 +78,7 @@ describe('pds-icon', () => {
html: '<pds-icon name="archive" color="red"></pds-icon>',
});
expect(root).toEqualHtml(`
<pds-icon alt="" aria-label="archive" class="pds-color pds-color-red" color="red" name="archive" role="img" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: red">
<pds-icon aria-label="archive" class="pds-color pds-color-red" color="red" name="archive" role="img" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: red">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
Expand All @@ -79,7 +93,7 @@ describe('pds-icon', () => {
});

expect(root).toEqualHtml(`
<pds-icon alt="" name="star" role="img" aria-label="custom label" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: currentColor;">
<pds-icon name="star" role="img" aria-label="custom label" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: currentColor;">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
Expand All @@ -96,7 +110,7 @@ describe('pds-icon', () => {
const icon = page.root;

expect(icon).toEqualHtml(`
<pds-icon alt="" name="youtube" role="img" aria-label="custom label" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: currentColor;">
<pds-icon name="youtube" role="img" aria-label="custom label" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: currentColor;">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
Expand All @@ -109,7 +123,7 @@ describe('pds-icon', () => {
await page.waitForChanges();

expect(icon).toEqualHtml(`
<pds-icon alt="" name="trash" role="img" aria-label="custom label" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: currentColor;">
<pds-icon name="trash" role="img" aria-label="custom label" size="regular" style="--dimension-icon-height: 16px; --dimension-icon-width: 16px; --color-icon-fill: currentColor;">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
Expand Down