Skip to content

Commit

Permalink
fix(icon): avoid type collisions with ariaLabel and ariaHidden (#1014)
Browse files Browse the repository at this point in the history
  • Loading branch information
@liamdebeasi
liamdebeasi authored Nov 8, 2021
1 parent 947091a commit 1038a7f
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 50 deletions.
16 changes: 0 additions & 16 deletions src/components.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,6 @@
import { HTMLStencilElement, JSXBase } from "@stencil/core/internal";
export namespace Components {
interface IonIcon {
/**
* Set the icon to hidden, respectively `true`, to remove it from the accessibility tree.
*/
"ariaHidden"?: string;
/**
* Specifies the label to use for accessibility. Defaults to the icon name.
*/
"ariaLabel"?: string;
/**
* The color to use for the background of the item.
*/
Expand Down Expand Up @@ -75,14 +67,6 @@ declare global {
}
declare namespace LocalJSX {
interface IonIcon {
/**
* Set the icon to hidden, respectively `true`, to remove it from the accessibility tree.
*/
"ariaHidden"?: string;
/**
* Specifies the label to use for accessibility. Defaults to the icon name.
*/
"ariaLabel"?: string;
/**
* The color to use for the background of the item.
*/
Expand Down
47 changes: 29 additions & 18 deletions src/components/icon/icon.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Build, Component, Element, Host, Prop, State, Watch, h } from '@stencil/core';
import { getSvgContent, ioniconContent } from './request';
import { getName, getUrl } from './utils';
import { getName, getUrl, inheritAttributes } from './utils';

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

@Element() el!: HTMLElement;

@State() private svgContent?: string;
@State() private isVisible = false;
@State() private ariaLabel?: string;

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

/**
* Specifies the label to use for accessibility. Defaults to the icon name.
*/
@Prop({ mutable: true, reflect: true }) ariaLabel?: string;

/**
* Set the icon to hidden, respectively `true`, to remove it from the accessibility tree.
*/
@Prop({ reflect: true }) ariaHidden?: string;

/**
* Specifies which icon to use on `ios` mode.
*/
Expand Down Expand Up @@ -88,6 +80,10 @@ export class Icon {
* @default true
*/
@Prop() sanitize = true;

componentWillLoad() {
this.inheritedAttributes = inheritAttributes(this.el, ['aria-label']);
}

connectedCallback() {
// purposely do not return the promise here because loading
Expand Down Expand Up @@ -126,6 +122,12 @@ export class Icon {
cb();
}
}

private hasAriaHidden = () => {
const { el } = this;

return el.hasAttribute('aria-hidden') && el.getAttribute('aria-hidden') === 'true';
}

@Watch('name')
@Watch('src')
Expand All @@ -146,33 +148,42 @@ export class Icon {

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

if (!this.ariaLabel && this.ariaHidden !== 'true') {
// user did not provide a label
// come up with the label based on the icon name
if (label) {
this.ariaLabel = label.replace(/\-/g, ' ');
}
/**
* Come up with a default label
* in case user does not provide their own.
*/
if (label) {
this.ariaLabel = label.replace(/\-/g, ' ');
}
}

render() {
const { iconName } = this;
const { iconName, ariaLabel, inheritedAttributes } = this;
const mode = this.mode || 'md';
const flipRtl =
this.flipRtl ||
(iconName &&
(iconName.indexOf('arrow') > -1 || iconName.indexOf('chevron') > -1) &&
this.flipRtl !== false);

/**
* Only set the aria-label if a) we have generated
* one for the icon and if aria-hidden is not set to "true".
* If developer wants to set their own aria-label, then
* inheritedAttributes down below will override whatever
* default label we have set.
*/
return (
<Host
aria-label={ariaLabel !== undefined && !this.hasAriaHidden() ? ariaLabel : null}
role="img"
class={{
[mode]: true,
...createColorClasses(this.color),
[`icon-${this.size}`]: !!this.size,
'flip-rtl': !!flipRtl && (this.el.ownerDocument as Document).dir === 'rtl',
}}
{...inheritedAttributes}
>
{Build.isBrowser && this.svgContent ? (
<div class="icon-inner" innerHTML={this.svgContent}></div>
Expand Down
28 changes: 13 additions & 15 deletions src/components/icon/readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,19 @@

## Properties

| Property | Attribute | Description | Type | Default |
| ------------ | ------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------- | -------------- |
| `ariaHidden` | `aria-hidden` | Set the icon to hidden, respectively `true`, to remove it from the accessibility tree. | `string \| undefined` | `undefined` |
| `ariaLabel` | `aria-label` | Specifies the label to use for accessibility. Defaults to the icon name. | `string \| undefined` | `undefined` |
| `color` | `color` | The color to use for the background of the item. | `string \| undefined` | `undefined` |
| `flipRtl` | `flip-rtl` | Specifies whether the icon should horizontally flip when `dir` is `"rtl"`. | `boolean \| undefined` | `undefined` |
| `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` |
| `ios` | `ios` | Specifies which icon to use on `ios` mode. | `string \| undefined` | `undefined` |
| `lazy` | `lazy` | If enabled, ion-icon will be loaded lazily when it's visible in the viewport. Default, `false`. | `boolean` | `false` |
| `md` | `md` | Specifies which icon to use on `md` mode. | `string \| undefined` | `undefined` |
| `mode` | `mode` | The mode determines which platform styles to use. | `string` | `getIonMode()` |
| `name` | `name` | Specifies which icon to use from the built-in set of icons. | `string \| undefined` | `undefined` |
| `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` |
| `size` | `size` | The size of the icon. Available options are: `"small"` and `"large"`. | `string \| undefined` | `undefined` |
| `src` | `src` | Specifies the exact `src` of an SVG file to use. | `string \| undefined` | `undefined` |
| Property | Attribute | Description | Type | Default |
| ---------- | ---------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------- | -------------- |
| `color` | `color` | The color to use for the background of the item. | `string \| undefined` | `undefined` |
| `flipRtl` | `flip-rtl` | Specifies whether the icon should horizontally flip when `dir` is `"rtl"`. | `boolean \| undefined` | `undefined` |
| `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` |
| `ios` | `ios` | Specifies which icon to use on `ios` mode. | `string \| undefined` | `undefined` |
| `lazy` | `lazy` | If enabled, ion-icon will be loaded lazily when it's visible in the viewport. Default, `false`. | `boolean` | `false` |
| `md` | `md` | Specifies which icon to use on `md` mode. | `string \| undefined` | `undefined` |
| `mode` | `mode` | The mode determines which platform styles to use. | `string` | `getIonMode()` |
| `name` | `name` | Specifies which icon to use from the built-in set of icons. | `string \| undefined` | `undefined` |
| `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` |
| `size` | `size` | The size of the icon. Available options are: `"small"` and `"large"`. | `string \| undefined` | `undefined` |
| `src` | `src` | Specifies the exact `src` of an SVG file to use. | `string \| undefined` | `undefined` |


----------------------------------------------
Expand Down
86 changes: 86 additions & 0 deletions src/components/icon/test/icon.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,90 @@ describe('icon', () => {
</ion-icon>
`);
});

it('renders default aria-label', async () => {
const { root } = await newSpecPage({
components: [Icon],
html: `<ion-icon name="chevron-forward"></ion-icon>`,
});

expect(root).toEqualHtml(`
<ion-icon class="md" name="chevron-forward" role="img" aria-label="chevron forward">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
</ion-icon>
`);
});

it('renders custom aria-label', async () => {
const { root } = await newSpecPage({
components: [Icon],
html: `<ion-icon name="chevron-forward" aria-label="custom label"></ion-icon>`,
});

expect(root).toEqualHtml(`
<ion-icon class="md" name="chevron-forward" role="img" aria-label="custom label">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
</ion-icon>
`);
});

it('renders custom label after changing source', async () => {
const page = await newSpecPage({
components: [Icon],
html: `<ion-icon name="chevron-forward" aria-label="custom label"></ion-icon>`,
});

const icon = page.root;

expect(icon).toEqualHtml(`
<ion-icon class="md" name="chevron-forward" role="img" aria-label="custom label">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
</ion-icon>
`);

icon.name = 'trash';
await page.waitForChanges();

expect(icon).toEqualHtml(`
<ion-icon class="md" name="trash" role="img" aria-label="custom label">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
</ion-icon>
`);
});

it('renders default label after changing source', async () => {
const page = await newSpecPage({
components: [Icon],
html: `<ion-icon name="chevron-forward"></ion-icon>`,
});

const icon = page.root;

expect(icon).toEqualHtml(`
<ion-icon class="md" name="chevron-forward" role="img" aria-label="chevron forward">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
</ion-icon>
`);

icon.name = 'trash';
await page.waitForChanges();

expect(icon).toEqualHtml(`
<ion-icon class="md" name="trash" role="img" aria-label="trash">
<mock:shadow-root>
<div class="icon-inner"></div>
</mock:shadow-root>
</ion-icon>
`);
});
});
28 changes: 27 additions & 1 deletion src/components/icon/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,30 @@ export const isSrc = (str: string) => str.length > 0 && /(\/|\.)/.test(str);

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

export const toLower = (val: string) => val.toLowerCase();
export const toLower = (val: string) => val.toLowerCase();

/**
* Elements inside of web components sometimes need to inherit global attributes
* set on the host. For example, the inner input in `ion-input` should inherit
* the `title` attribute that developers set directly on `ion-input`. This
* helper function should be called in componentWillLoad and assigned to a variable
* that is later used in the render function.
*
* This does not need to be reactive as changing attributes on the host element
* does not trigger a re-render.
*/
export const inheritAttributes = (el: HTMLElement, attributes: string[] = []) => {
const attributeObject: { [k: string]: any } = {};

attributes.forEach(attr => {
if (el.hasAttribute(attr)) {
const value = el.getAttribute(attr);
if (value !== null) {
attributeObject[attr] = el.getAttribute(attr);
}
el.removeAttribute(attr);
}
});

return attributeObject;
}

0 comments on commit 1038a7f

Please sign in to comment.