Skip to content

Commit

Permalink
Rewrite Button to Typescript (#2984)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
4 people authored Aug 22, 2021
1 parent 01082a4 commit 3cf19dd
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 80 deletions.
75 changes: 0 additions & 75 deletions js/src/common/components/Button.js

This file was deleted.

130 changes: 130 additions & 0 deletions js/src/common/components/Button.tsx
Original file line number Diff line number Diff line change
@@ -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" />,
];
}
}
13 changes: 8 additions & 5 deletions js/src/common/components/TextEditorButton.js
Original file line number Diff line number Diff line change
@@ -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;
}
}
16 changes: 16 additions & 0 deletions js/src/common/helpers/fireDebugWarning.ts
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit 3cf19dd

Please sign in to comment.