Rewrite Button to Typescript (#2984)

* Rename Button file

* Convert to TS

* Add debug warning helper

Fires `console.warn`, but only when the forum is in debug mode. Can help to inform extension developers of possible issues with their JS code.

* Simplify button content template

* Rewrite Button component

- 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)

* Update to work with new Button component

* Update warning

Co-authored-by: Matt Kilgore <tankerkiller125@gmail.com>

* Fire warning in `oncreate`

* Format

* Make Button have extensible Attributes type via generics

* Update args type

* Update js/src/common/components/Button.tsx

Co-authored-by: Matt Kilgore <tankerkiller125@gmail.com>
Co-authored-by: David Sevilla Martin <me@datitisev.me>
Co-authored-by: Alexander Skvortsov <sasha.skvortsov109@gmail.com>
This commit is contained in:
David Wheatley 2021-08-22 20:38:01 +01:00 committed by GitHub
parent b1acb1ba5a
commit fcdc7930b1
4 changed files with 154 additions and 80 deletions

View File

@ -1,75 +0,0 @@
import Component from '../Component';
import icon from '../helpers/icon';
import classList from '../utils/classList';
import extract from '../utils/extract';
import extractText from '../utils/extractText';
import LoadingIndicator from './LoadingIndicator';
/**
* The `Button` component defines an element which, when clicked, performs an
* action.
*
* ### Attrs
*
* - `icon` The name of the icon class. If specified, the button will be given a
* 'has-icon' class name.
* - `disabled` Whether or not the button is disabled. If truthy, the button
* will be given a 'disabled' class name, and any `onclick` handler will be
* removed.
* - `loading` Whether or not the button should be in a disabled loading state.
*
* All other attrs will be assigned as attributes on the button element.
*
* Note that a Button has no default class names. This is because a Button can
* be used to represent any generic clickable control, like a menu item.
*/
export default class Button extends Component {
view(vnode) {
const attrs = Object.assign({}, this.attrs);
attrs.type = attrs.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;
}
// If given a translation object, extract the text.
if (typeof attrs.title === 'object') {
attrs.title = extractText(attrs.title);
}
// If nothing else is provided, we use the textual button content as tooltip
if (!attrs.title && vnode.children) {
attrs.title = extractText(vnode.children);
}
const iconName = extract(attrs, 'icon');
const loading = extract(attrs, 'loading');
if (attrs.disabled || loading) {
delete attrs.onclick;
}
attrs.className = classList([attrs.className, iconName && 'hasIcon', (attrs.disabled || loading) && 'disabled', loading && 'loading']);
return <button {...attrs}>{this.getButtonContent(vnode.children)}</button>;
}
/**
* Get the template for the button's content.
*
* @return {*}
* @protected
*/
getButtonContent(children) {
const iconName = this.attrs.icon;
return [
iconName && iconName !== true ? icon(iconName, { className: 'Button-icon' }) : '',
children ? <span className="Button-label">{children}</span> : '',
this.attrs.loading ? <LoadingIndicator size="small" display="inline" /> : '',
];
}
}

View File

@ -0,0 +1,130 @@
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 extractText from '../utils/extractText';
import LoadingIndicator from './LoadingIndicator';
export interface IButtonAttrs extends ComponentAttrs {
/**
* Class(es) of an optional icon to be rendered within the button.
*
* If provided, the button will gain a `has-icon` class.
*/
icon?: string;
/**
* Disables button from user input.
*
* Default: `false`
*/
disabled?: boolean;
/**
* Show a loading spinner within the button.
*
* If `true`, also disables the button.
*
* Default: `false`
*/
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.
*
* Default: `"button"`
*
* @see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#attr-type
*/
type?: string;
}
/**
* The `Button` component defines an element which, when clicked, performs an
* action.
*
* Other attrs will be assigned as attributes on the `<button>` element.
*
* Note that a Button has no default class names. This is because a Button can
* be used to represent any generic clickable control, like a menu item. Common
* styles can be applied by providing `className="Button"` to the Button component.
*/
export default class Button<CustomAttrs extends IButtonAttrs = IButtonAttrs> extends Component<CustomAttrs> {
view(vnode: Mithril.Vnode<IButtonAttrs, never>) {
let { type, title, 'aria-label': ariaLabel, icon: iconName, disabled, loading, className, class: _class, ...attrs } = this.attrs;
// If no `type` attr provided, set to "button"
type ||= 'button';
// Use `title` attribute as `aria-label` if none provided
ariaLabel ||= title;
// If given a translation object, extract the text.
if (typeof ariaLabel === 'object') {
ariaLabel = extractText(ariaLabel);
}
if (disabled || loading) {
delete attrs.onclick;
}
className = classList(_class, className, {
hasIcon: iconName,
disabled: disabled || loading,
loading: loading,
});
const buttonAttrs = {
disabled,
className,
type,
'aria-label': ariaLabel,
...attrs,
};
return <button {...buttonAttrs}>{this.getButtonContent(vnode.children)}</button>;
}
oncreate(vnode: Mithril.VnodeDOM<IButtonAttrs, this>) {
super.oncreate(vnode);
const { 'aria-label': ariaLabel } = this.attrs;
if (!ariaLabel && !extractText(vnode.children) && !this.element?.getAttribute?.('aria-label')) {
fireDebugWarning(
'[Flarum Accessibility Warning] Button has no content and no accessible label. This means that screen-readers will not be able to interpret its meaning and just read "Button". Consider providing accessible text via the `aria-label` attribute. https://web.dev/button-name',
this.element
);
}
}
/**
* Get the template for the button's content.
*/
protected getButtonContent(children: Mithril.Children): Mithril.ChildArray {
const iconName = this.attrs.icon;
return [
iconName && icon(iconName, { className: 'Button-icon' }),
children && <span className="Button-label">{children}</span>,
this.attrs.loading && <LoadingIndicator size="small" display="inline" />,
];
}
}

View File

@ -1,24 +1,27 @@
import extractText from '../utils/extractText';
import Button from './Button';
import Tooltip from './Tooltip';
/**
* The `TextEditorButton` component displays a button suitable for the text
* editor toolbar.
*
* Automatically creates tooltips using the Tooltip component and provided text.
*
* ## Attrs
* - `title` - Tooltip for the button
*/
export default class TextEditorButton extends Button {
view(vnode) {
const originalView = super.view(vnode);
// Steal tooltip label from the Button superclass
const tooltipText = originalView.attrs.title;
delete originalView.attrs.title;
return <Tooltip text={tooltipText}>{originalView}</Tooltip>;
return <Tooltip text={this.attrs.tooltipText || extractText(vnode.children)}>{originalView}</Tooltip>;
}
static initAttrs(attrs) {
super.initAttrs(attrs);
attrs.className = attrs.className || 'Button Button--icon Button--link';
attrs.tooltipText = attrs.title;
}
}

View File

@ -0,0 +1,16 @@
/**
* Calls `console.warn` with the provided arguments, but only if the forum is in debug mode.
*
* This function is intended to provide warnings to extension developers about issues with
* their extensions that may not be easily noticed when testing, such as accessibility
* issues.
*
* These warnings should be hidden on production forums to ensure webmasters are not
* inundated with do-gooders telling them they have an issue when it isn't something they
* can fix.
*/
export default function fireDebugWarning(...args: Parameters<typeof console.warn>): void {
if (!app.forum.attribute('debug')) return;
console.warn(...args);
}