Skip to content

Commit

Permalink
Rewrite Button component
Browse files Browse the repository at this point in the history
- Prefer `aria-label` over `title`
- Don't duplicate button content to `title` attribute
- Warn in debug mode if button has no accessible content
- Use modern JS/TS syntax (`||=`, spread, etc)
  • Loading branch information
davwheat committed Jul 20, 2021
1 parent 17e24ca commit 9ba836c
Showing 1 changed file with 40 additions and 19 deletions.
59 changes: 40 additions & 19 deletions js/src/common/components/Button.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type Mithril from 'mithril';
import Component, { ComponentAttrs } from '../Component';
import fireDebugWarning from '../helpers/fireDebugWarning';
import icon from '../helpers/icon';
import classList from '../utils/classList';
import extract from '../utils/extract';
import extractText from '../utils/extractText';
import LoadingIndicator from './LoadingIndicator';

Expand All @@ -28,12 +28,24 @@ export interface ButtonAttrs extends ComponentAttrs {
*/
loading?: boolean;
/**
* **DEPRECATED:** Please use the `aria-label` attribute instead. For tooltips, use
* the `<Tooltip>` component.
*
* Accessible text for the button. This should always be present if the button only
* contains an icon.
*
* The textual content of this attribute is passed to the DOM element as `aria-label`.
*
* @deprecated
*/
title?: string | Mithril.ChildArray;
/**
* Accessible text for the button. This should always be present if the button only
* contains an icon.
*
* The textual content of this attribute is passed to the DOM element as `aria-label`.
*/
'aria-label'?: string | Mithril.ChildArray;
/**
* Button type.
*
Expand All @@ -56,36 +68,45 @@ export interface ButtonAttrs extends ComponentAttrs {
*/
export default class Button extends Component<ButtonAttrs> {
view(vnode: Mithril.Vnode<ButtonAttrs, never>) {
const attrs = Object.assign({}, this.attrs);
let { type, title, 'aria-label': ariaLabel, icon: iconName, disabled, loading, className, class: _class, ...attrs } = this.attrs;

attrs.type = attrs.type || 'button';
// If no `type` attr provided, set to "button"
type ||= 'button';

// If a tooltip was provided for buttons without additional content, we also
// use this tooltip as text for screen readers
if (attrs.title && !vnode.children) {
attrs['aria-label'] = attrs.title;
}
// Use `title` attribute as `aria-label` if none provided
ariaLabel ||= title;

// If given a translation object, extract the text.
if (typeof attrs.title === 'object') {
attrs.title = extractText(attrs.title);
if (typeof ariaLabel === 'object') {
ariaLabel = extractText(ariaLabel);
}

// If nothing else is provided, we use the textual button content as tooltip
if (!attrs.title && vnode.children) {
attrs.title = extractText(vnode.children);
if (disabled || loading) {
delete attrs.onclick;
}

const iconName = extract(attrs, 'icon');
className = classList(_class, className, {
hasIcon: iconName,
disabled: disabled || loading,
loading: loading,
});

const loading = extract(attrs, 'loading');
if (attrs.disabled || loading) {
delete attrs.onclick;
if (!ariaLabel && !extractText(vnode.children) && !this.element?.getAttribute?.('aria-label')) {
fireDebugWarning(
'[Flarum Accessibility Warning] This button has no content but does not have any accessible label. This means that screen-readers will not be able to interpret its meaning. Consider providing accessible text via the `aria-label` attribute.\n\nLearn more: https://web.dev/button-name',
this.element
);
}

attrs.className = classList([attrs.className, iconName && 'hasIcon', (attrs.disabled || loading) && 'disabled', loading && 'loading']);
const buttonAttrs = {
disabled,
className,
type,
'aria-label': ariaLabel,
...attrs,
};

return <button {...attrs}>{this.getButtonContent(vnode.children)}</button>;
return <button {...buttonAttrs}>{this.getButtonContent(vnode.children)}</button>;
}

/**
Expand Down

0 comments on commit 9ba836c

Please sign in to comment.