From 3d10902665609e45c2de24463497e8e1d61e39da Mon Sep 17 00:00:00 2001 From: David Date: Tue, 20 Jul 2021 23:47:53 +0100 Subject: [PATCH 01/12] Rename Button file --- js/src/common/components/{Button.js => Button.tsx} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename js/src/common/components/{Button.js => Button.tsx} (100%) diff --git a/js/src/common/components/Button.js b/js/src/common/components/Button.tsx similarity index 100% rename from js/src/common/components/Button.js rename to js/src/common/components/Button.tsx From b9b4548d8c1f9d0bdf031d2cec88cea697c4e983 Mon Sep 17 00:00:00 2001 From: David Date: Tue, 20 Jul 2021 23:49:44 +0100 Subject: [PATCH 02/12] Convert to TS --- js/src/common/components/Button.tsx | 64 +++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 18 deletions(-) diff --git a/js/src/common/components/Button.tsx b/js/src/common/components/Button.tsx index 2d9bfc83d5..5777e26f46 100644 --- a/js/src/common/components/Button.tsx +++ b/js/src/common/components/Button.tsx @@ -1,30 +1,61 @@ -import Component from '../Component'; +import type Mithril from 'mithril'; +import Component, { ComponentAttrs } 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'; +export interface ButtonAttrs 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; + /** + * 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`. + */ + title?: 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. * - * ### 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. + * Other attrs will be assigned as attributes on the `; + return ; } /** From ba6840125bc9cea62525470b6c613fcf38f2905a Mon Sep 17 00:00:00 2001 From: David Date: Wed, 21 Jul 2021 00:58:42 +0100 Subject: [PATCH 06/12] Update to work with new Button component --- js/src/common/components/TextEditorButton.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/js/src/common/components/TextEditorButton.js b/js/src/common/components/TextEditorButton.js index 4f379ef400..fa517e627f 100644 --- a/js/src/common/components/TextEditorButton.js +++ b/js/src/common/components/TextEditorButton.js @@ -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 {originalView}; + return {originalView}; } static initAttrs(attrs) { super.initAttrs(attrs); attrs.className = attrs.className || 'Button Button--icon Button--link'; + attrs.tooltipText = attrs.title; } } From 7f6f4b8f9c27c93ae78d09bcfe606f29ea26858f Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Wed, 21 Jul 2021 09:44:01 +0100 Subject: [PATCH 07/12] Update warning Co-authored-by: Matt Kilgore --- js/src/common/components/Button.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/common/components/Button.tsx b/js/src/common/components/Button.tsx index 2dce1fe175..c4a4890979 100644 --- a/js/src/common/components/Button.tsx +++ b/js/src/common/components/Button.tsx @@ -93,7 +93,7 @@ export default class Button extends Component { 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', + '[Flarum Accessibility Warning] This button has no content and 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 ); } From 1ba1aec6a7eb634024e8849ae45a910bb985d04d Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Mon, 9 Aug 2021 17:07:41 +0100 Subject: [PATCH 08/12] Fire warning in `oncreate` --- js/src/common/components/Button.tsx | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/js/src/common/components/Button.tsx b/js/src/common/components/Button.tsx index c4a4890979..1ca9897b15 100644 --- a/js/src/common/components/Button.tsx +++ b/js/src/common/components/Button.tsx @@ -6,7 +6,7 @@ import classList from '../utils/classList'; import extractText from '../utils/extractText'; import LoadingIndicator from './LoadingIndicator'; -export interface ButtonAttrs extends ComponentAttrs { +export interface IButtonAttrs extends ComponentAttrs { /** * Class(es) of an optional icon to be rendered within the button. * @@ -66,8 +66,8 @@ export interface ButtonAttrs extends ComponentAttrs { * 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 extends Component { - view(vnode: Mithril.Vnode) { +export default class Button extends Component { + view(vnode: Mithril.Vnode) { let { type, title, 'aria-label': ariaLabel, icon: iconName, disabled, loading, className, class: _class, ...attrs } = this.attrs; // If no `type` attr provided, set to "button" @@ -91,13 +91,6 @@ export default class Button extends Component { loading: loading, }); - if (!ariaLabel && !extractText(vnode.children) && !this.element?.getAttribute?.('aria-label')) { - fireDebugWarning( - '[Flarum Accessibility Warning] This button has no content and 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 - ); - } - const buttonAttrs = { disabled, className, @@ -108,6 +101,19 @@ export default class Button extends Component { return ; } + + oncreate(vnode: Mithril.VnodeDOM) { + 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. From 32fa4f6f7de854e74c17837c002c0c1fa5b1c19d Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Mon, 9 Aug 2021 18:10:14 +0200 Subject: [PATCH 09/12] Format --- js/src/common/components/Button.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/src/common/components/Button.tsx b/js/src/common/components/Button.tsx index 1ca9897b15..1a6458fc62 100644 --- a/js/src/common/components/Button.tsx +++ b/js/src/common/components/Button.tsx @@ -101,10 +101,10 @@ export default class Button extends Component { return ; } - + oncreate(vnode: Mithril.VnodeDOM) { super.oncreate(vnode); - + const { 'aria-label': ariaLabel } = this.attrs; if (!ariaLabel && !extractText(vnode.children) && !this.element?.getAttribute?.('aria-label')) { From 15cda910270aef46173b672af7f7963373601ed2 Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Fri, 20 Aug 2021 22:51:00 +0100 Subject: [PATCH 10/12] Make Button have extensible Attributes type via generics --- js/src/common/components/Button.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/common/components/Button.tsx b/js/src/common/components/Button.tsx index 1a6458fc62..53ae9ef5c2 100644 --- a/js/src/common/components/Button.tsx +++ b/js/src/common/components/Button.tsx @@ -66,7 +66,7 @@ export interface IButtonAttrs extends ComponentAttrs { * 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 extends Component { +export default class Button = {}> extends Component { view(vnode: Mithril.Vnode) { let { type, title, 'aria-label': ariaLabel, icon: iconName, disabled, loading, className, class: _class, ...attrs } = this.attrs; From 47557b39b9b52d3ceaea650bcbb3dcde38d36dbd Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Fri, 20 Aug 2021 22:53:07 +0100 Subject: [PATCH 11/12] Update args type --- js/src/common/helpers/fireDebugWarning.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/common/helpers/fireDebugWarning.ts b/js/src/common/helpers/fireDebugWarning.ts index d8075ad928..8c58e79863 100644 --- a/js/src/common/helpers/fireDebugWarning.ts +++ b/js/src/common/helpers/fireDebugWarning.ts @@ -9,7 +9,7 @@ * inundated with do-gooders telling them they have an issue when it isn't something they * can fix. */ -export default function fireDebugWarning(...args: any[]): void { +export default function fireDebugWarning(...args: Parameters): void { if (!app.forum.attribute('debug')) return; console.warn(...args); From 048bab666dba68aaf69d73dc7f596bd5415466b4 Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Sat, 21 Aug 2021 00:24:23 +0200 Subject: [PATCH 12/12] Update js/src/common/components/Button.tsx --- js/src/common/components/Button.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/src/common/components/Button.tsx b/js/src/common/components/Button.tsx index 53ae9ef5c2..bec7aa1d0b 100644 --- a/js/src/common/components/Button.tsx +++ b/js/src/common/components/Button.tsx @@ -66,7 +66,7 @@ export interface IButtonAttrs extends ComponentAttrs { * 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 = {}> extends Component { +export default class Button extends Component { view(vnode: Mithril.Vnode) { let { type, title, 'aria-label': ariaLabel, icon: iconName, disabled, loading, className, class: _class, ...attrs } = this.attrs;