Skip to content

Commit 1038a7f

Browse files
liamdebeasiliamdebeasi
authored
fix(icon): avoid type collisions with ariaLabel and ariaHidden (#1014)
1 parent 947091a commit 1038a7f

File tree

5 files changed

+155
-50
lines changed

5 files changed

+155
-50
lines changed

‎src/components.d.ts

-16
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,6 @@
77
import { HTMLStencilElement, JSXBase } from "@stencil/core/internal";
88
export namespace Components {
99
interface IonIcon {
10-
/**
11-
* Set the icon to hidden, respectively `true`, to remove it from the accessibility tree.
12-
*/
13-
"ariaHidden"?: string;
14-
/**
15-
* Specifies the label to use for accessibility. Defaults to the icon name.
16-
*/
17-
"ariaLabel"?: string;
1810
/**
1911
* The color to use for the background of the item.
2012
*/
@@ -75,14 +67,6 @@ declare global {
7567
}
7668
declare namespace LocalJSX {
7769
interface IonIcon {
78-
/**
79-
* Set the icon to hidden, respectively `true`, to remove it from the accessibility tree.
80-
*/
81-
"ariaHidden"?: string;
82-
/**
83-
* Specifies the label to use for accessibility. Defaults to the icon name.
84-
*/
85-
"ariaLabel"?: string;
8670
/**
8771
* The color to use for the background of the item.
8872
*/

‎src/components/icon/icon.tsx

+29-18
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Build, Component, Element, Host, Prop, State, Watch, h } from '@stencil/core';
22
import { getSvgContent, ioniconContent } from './request';
3-
import { getName, getUrl } from './utils';
3+
import { getName, getUrl, inheritAttributes } from './utils';
44

55
@Component({
66
tag: 'ion-icon',
@@ -11,11 +11,13 @@ import { getName, getUrl } from './utils';
1111
export class Icon {
1212
private io?: IntersectionObserver;
1313
private iconName: string | null = null;
14+
private inheritedAttributes: { [k: string]: any } = {};
1415

1516
@Element() el!: HTMLElement;
1617

1718
@State() private svgContent?: string;
1819
@State() private isVisible = false;
20+
@State() private ariaLabel?: string;
1921

2022
/**
2123
* The mode determines which platform styles to use.
@@ -27,16 +29,6 @@ export class Icon {
2729
*/
2830
@Prop() color?: string;
2931

30-
/**
31-
* Specifies the label to use for accessibility. Defaults to the icon name.
32-
*/
33-
@Prop({ mutable: true, reflect: true }) ariaLabel?: string;
34-
35-
/**
36-
* Set the icon to hidden, respectively `true`, to remove it from the accessibility tree.
37-
*/
38-
@Prop({ reflect: true }) ariaHidden?: string;
39-
4032
/**
4133
* Specifies which icon to use on `ios` mode.
4234
*/
@@ -88,6 +80,10 @@ export class Icon {
8880
* @default true
8981
*/
9082
@Prop() sanitize = true;
83+
84+
componentWillLoad() {
85+
this.inheritedAttributes = inheritAttributes(this.el, ['aria-label']);
86+
}
9187

9288
connectedCallback() {
9389
// purposely do not return the promise here because loading
@@ -126,6 +122,12 @@ export class Icon {
126122
cb();
127123
}
128124
}
125+
126+
private hasAriaHidden = () => {
127+
const { el } = this;
128+
129+
return el.hasAttribute('aria-hidden') && el.getAttribute('aria-hidden') === 'true';
130+
}
129131

130132
@Watch('name')
131133
@Watch('src')
@@ -146,33 +148,42 @@ export class Icon {
146148

147149
const label = this.iconName = getName(this.name, this.icon, this.mode, this.ios, this.md);
148150

149-
if (!this.ariaLabel && this.ariaHidden !== 'true') {
150-
// user did not provide a label
151-
// come up with the label based on the icon name
152-
if (label) {
153-
this.ariaLabel = label.replace(/\-/g, ' ');
154-
}
151+
/**
152+
* Come up with a default label
153+
* in case user does not provide their own.
154+
*/
155+
if (label) {
156+
this.ariaLabel = label.replace(/\-/g, ' ');
155157
}
156158
}
157159

158160
render() {
159-
const { iconName } = this;
161+
const { iconName, ariaLabel, inheritedAttributes } = this;
160162
const mode = this.mode || 'md';
161163
const flipRtl =
162164
this.flipRtl ||
163165
(iconName &&
164166
(iconName.indexOf('arrow') > -1 || iconName.indexOf('chevron') > -1) &&
165167
this.flipRtl !== false);
166168

169+
/**
170+
* Only set the aria-label if a) we have generated
171+
* one for the icon and if aria-hidden is not set to "true".
172+
* If developer wants to set their own aria-label, then
173+
* inheritedAttributes down below will override whatever
174+
* default label we have set.
175+
*/
167176
return (
168177
<Host
178+
aria-label={ariaLabel !== undefined && !this.hasAriaHidden() ? ariaLabel : null}
169179
role="img"
170180
class={{
171181
[mode]: true,
172182
...createColorClasses(this.color),
173183
[`icon-${this.size}`]: !!this.size,
174184
'flip-rtl': !!flipRtl && (this.el.ownerDocument as Document).dir === 'rtl',
175185
}}
186+
{...inheritedAttributes}
176187
>
177188
{Build.isBrowser && this.svgContent ? (
178189
<div class="icon-inner" innerHTML={this.svgContent}></div>

‎src/components/icon/readme.md

+13-15
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,19 @@
77

88
## Properties
99

10-
| Property | Attribute | Description | Type | Default |
11-
| ------------ | ------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------- | -------------- |
12-
| `ariaHidden` | `aria-hidden` | Set the icon to hidden, respectively `true`, to remove it from the accessibility tree. | `string \| undefined` | `undefined` |
13-
| `ariaLabel` | `aria-label` | Specifies the label to use for accessibility. Defaults to the icon name. | `string \| undefined` | `undefined` |
14-
| `color` | `color` | The color to use for the background of the item. | `string \| undefined` | `undefined` |
15-
| `flipRtl` | `flip-rtl` | Specifies whether the icon should horizontally flip when `dir` is `"rtl"`. | `boolean \| undefined` | `undefined` |
16-
| `icon` | `icon` | 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 set the `name` property. | `any` | `undefined` |
17-
| `ios` | `ios` | Specifies which icon to use on `ios` mode. | `string \| undefined` | `undefined` |
18-
| `lazy` | `lazy` | If enabled, ion-icon will be loaded lazily when it's visible in the viewport. Default, `false`. | `boolean` | `false` |
19-
| `md` | `md` | Specifies which icon to use on `md` mode. | `string \| undefined` | `undefined` |
20-
| `mode` | `mode` | The mode determines which platform styles to use. | `string` | `getIonMode()` |
21-
| `name` | `name` | Specifies which icon to use from the built-in set of icons. | `string \| undefined` | `undefined` |
22-
| `sanitize` | `sanitize` | When set to `false`, SVG content that is HTTP fetched will not be checked if the response SVG content has any `<script>` elements, or any attributes that start with `on`, such as `onclick`. | `boolean` | `true` |
23-
| `size` | `size` | The size of the icon. Available options are: `"small"` and `"large"`. | `string \| undefined` | `undefined` |
24-
| `src` | `src` | Specifies the exact `src` of an SVG file to use. | `string \| undefined` | `undefined` |
10+
| Property | Attribute | Description | Type | Default |
11+
| ---------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------- | -------------- |
12+
| `color` | `color` | The color to use for the background of the item. | `string \| undefined` | `undefined` |
13+
| `flipRtl` | `flip-rtl` | Specifies whether the icon should horizontally flip when `dir` is `"rtl"`. | `boolean \| undefined` | `undefined` |
14+
| `icon` | `icon` | 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 set the `name` property. | `any` | `undefined` |
15+
| `ios` | `ios` | Specifies which icon to use on `ios` mode. | `string \| undefined` | `undefined` |
16+
| `lazy` | `lazy` | If enabled, ion-icon will be loaded lazily when it's visible in the viewport. Default, `false`. | `boolean` | `false` |
17+
| `md` | `md` | Specifies which icon to use on `md` mode. | `string \| undefined` | `undefined` |
18+
| `mode` | `mode` | The mode determines which platform styles to use. | `string` | `getIonMode()` |
19+
| `name` | `name` | Specifies which icon to use from the built-in set of icons. | `string \| undefined` | `undefined` |
20+
| `sanitize` | `sanitize` | When set to `false`, SVG content that is HTTP fetched will not be checked if the response SVG content has any `<script>` elements, or any attributes that start with `on`, such as `onclick`. | `boolean` | `true` |
21+
| `size` | `size` | The size of the icon. Available options are: `"small"` and `"large"`. | `string \| undefined` | `undefined` |
22+
| `src` | `src` | Specifies the exact `src` of an SVG file to use. | `string \| undefined` | `undefined` |
2523

2624

2725
----------------------------------------------

‎src/components/icon/test/icon.spec.ts

+86
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,90 @@ describe('icon', () => {
4545
</ion-icon>
4646
`);
4747
});
48+
49+
it('renders default aria-label', async () => {
50+
const { root } = await newSpecPage({
51+
components: [Icon],
52+
html: `<ion-icon name="chevron-forward"></ion-icon>`,
53+
});
54+
55+
expect(root).toEqualHtml(`
56+
<ion-icon class="md" name="chevron-forward" role="img" aria-label="chevron forward">
57+
<mock:shadow-root>
58+
<div class="icon-inner"></div>
59+
</mock:shadow-root>
60+
</ion-icon>
61+
`);
62+
});
63+
64+
it('renders custom aria-label', async () => {
65+
const { root } = await newSpecPage({
66+
components: [Icon],
67+
html: `<ion-icon name="chevron-forward" aria-label="custom label"></ion-icon>`,
68+
});
69+
70+
expect(root).toEqualHtml(`
71+
<ion-icon class="md" name="chevron-forward" role="img" aria-label="custom label">
72+
<mock:shadow-root>
73+
<div class="icon-inner"></div>
74+
</mock:shadow-root>
75+
</ion-icon>
76+
`);
77+
});
78+
79+
it('renders custom label after changing source', async () => {
80+
const page = await newSpecPage({
81+
components: [Icon],
82+
html: `<ion-icon name="chevron-forward" aria-label="custom label"></ion-icon>`,
83+
});
84+
85+
const icon = page.root;
86+
87+
expect(icon).toEqualHtml(`
88+
<ion-icon class="md" name="chevron-forward" role="img" aria-label="custom label">
89+
<mock:shadow-root>
90+
<div class="icon-inner"></div>
91+
</mock:shadow-root>
92+
</ion-icon>
93+
`);
94+
95+
icon.name = 'trash';
96+
await page.waitForChanges();
97+
98+
expect(icon).toEqualHtml(`
99+
<ion-icon class="md" name="trash" role="img" aria-label="custom label">
100+
<mock:shadow-root>
101+
<div class="icon-inner"></div>
102+
</mock:shadow-root>
103+
</ion-icon>
104+
`);
105+
});
106+
107+
it('renders default label after changing source', async () => {
108+
const page = await newSpecPage({
109+
components: [Icon],
110+
html: `<ion-icon name="chevron-forward"></ion-icon>`,
111+
});
112+
113+
const icon = page.root;
114+
115+
expect(icon).toEqualHtml(`
116+
<ion-icon class="md" name="chevron-forward" role="img" aria-label="chevron forward">
117+
<mock:shadow-root>
118+
<div class="icon-inner"></div>
119+
</mock:shadow-root>
120+
</ion-icon>
121+
`);
122+
123+
icon.name = 'trash';
124+
await page.waitForChanges();
125+
126+
expect(icon).toEqualHtml(`
127+
<ion-icon class="md" name="trash" role="img" aria-label="trash">
128+
<mock:shadow-root>
129+
<div class="icon-inner"></div>
130+
</mock:shadow-root>
131+
</ion-icon>
132+
`);
133+
});
48134
});

‎src/components/icon/utils.ts

+27-1
Original file line numberDiff line numberDiff line change
@@ -113,4 +113,30 @@ export const isSrc = (str: string) => str.length > 0 && /(\/|\.)/.test(str);
113113

114114
export const isStr = (val: any): val is string => typeof val === 'string';
115115

116-
export const toLower = (val: string) => val.toLowerCase();
116+
export const toLower = (val: string) => val.toLowerCase();
117+
118+
/**
119+
* Elements inside of web components sometimes need to inherit global attributes
120+
* set on the host. For example, the inner input in `ion-input` should inherit
121+
* the `title` attribute that developers set directly on `ion-input`. This
122+
* helper function should be called in componentWillLoad and assigned to a variable
123+
* that is later used in the render function.
124+
*
125+
* This does not need to be reactive as changing attributes on the host element
126+
* does not trigger a re-render.
127+
*/
128+
export const inheritAttributes = (el: HTMLElement, attributes: string[] = []) => {
129+
const attributeObject: { [k: string]: any } = {};
130+
131+
attributes.forEach(attr => {
132+
if (el.hasAttribute(attr)) {
133+
const value = el.getAttribute(attr);
134+
if (value !== null) {
135+
attributeObject[attr] = el.getAttribute(attr);
136+
}
137+
el.removeAttribute(attr);
138+
}
139+
});
140+
141+
return attributeObject;
142+
}

0 commit comments

Comments
 (0)