From 90ffa6305649116b4ce3d85320af5d7f039a0892 Mon Sep 17 00:00:00 2001 From: David Wheatley Date: Wed, 21 Apr 2021 12:26:09 +0100 Subject: [PATCH] Fix uses of loading spinner (#2797) * Update Loading Indicator - Fix mistake in LoadingIndicator Less - Middle align the loading indicator when inline - Fix Loading Indicator not correctly accepting container class names - Add inline and block attributes * Fix loading indicator in composer * Fix loading indicator on notification list * Fix loading indicator on discussion page * Fix loading indicator on button * Update more uses of loading indicator * Fix loading indicator in Search box * Fix AvatarEditor loading spinner * Set default spinner props * Replace "tiny" with "small" in Less * Improve spinner vertical centring in buttons * Reduce size specificity * Use single attribute for block/inline * Use new display attribute * Use classes for different sizes * Use `display=block` by default --- .../core/js/src/common/components/Button.js | 2 +- .../core/js/src/common/components/Checkbox.js | 2 +- .../common/components/LoadingIndicator.tsx | 31 +++++++++++----- .../js/src/forum/components/AvatarEditor.js | 8 ++++- .../js/src/forum/components/ComposerBody.js | 3 +- .../js/src/forum/components/DiscussionList.js | 2 +- .../js/src/forum/components/DiscussionPage.js | 36 ++++++++++--------- .../src/forum/components/NotificationList.js | 2 +- .../core/js/src/forum/components/Search.js | 2 +- .../core/js/src/forum/components/UserPage.js | 2 +- framework/core/less/common/Button.less | 11 +++--- .../core/less/common/LoadingIndicator.less | 28 +++++++++------ framework/core/less/common/Search.less | 15 ++++++-- framework/core/less/forum/AvatarEditor.less | 2 +- framework/core/less/forum/Composer.less | 2 +- 15 files changed, 92 insertions(+), 56 deletions(-) diff --git a/framework/core/js/src/common/components/Button.js b/framework/core/js/src/common/components/Button.js index 6ae10a530..2d9bfc83d 100644 --- a/framework/core/js/src/common/components/Button.js +++ b/framework/core/js/src/common/components/Button.js @@ -69,7 +69,7 @@ export default class Button extends Component { return [ iconName && iconName !== true ? icon(iconName, { className: 'Button-icon' }) : '', children ? {children} : '', - this.attrs.loading ? : '', + this.attrs.loading ? : '', ]; } } diff --git a/framework/core/js/src/common/components/Checkbox.js b/framework/core/js/src/common/components/Checkbox.js index ebcda7e80..47f0f7e0e 100644 --- a/framework/core/js/src/common/components/Checkbox.js +++ b/framework/core/js/src/common/components/Checkbox.js @@ -46,7 +46,7 @@ export default class Checkbox extends Component { * @protected */ getDisplay() { - return this.attrs.loading ? : icon(this.attrs.state ? 'fas fa-check' : 'fas fa-times'); + return this.attrs.loading ? : icon(this.attrs.state ? 'fas fa-check' : 'fas fa-times'); } /** diff --git a/framework/core/js/src/common/components/LoadingIndicator.tsx b/framework/core/js/src/common/components/LoadingIndicator.tsx index 1404ab5eb..a4b024a7b 100644 --- a/framework/core/js/src/common/components/LoadingIndicator.tsx +++ b/framework/core/js/src/common/components/LoadingIndicator.tsx @@ -18,6 +18,12 @@ export interface LoadingIndicatorAttrs extends ComponentAttrs { * Optional attributes to apply to the loading indicator's container. */ containerAttrs?: Partial; + /** + * Display type of the spinner. + * + * @default 'block' + */ + display?: 'block' | 'inline' | 'unset'; } /** @@ -26,12 +32,13 @@ export interface LoadingIndicatorAttrs extends ComponentAttrs { * To set a custom color, use the CSS `color` property. * * To increase spacing around the spinner, use the CSS `height` property on the - * spinner's **container**. + * spinner's **container**. Setting the `display` attribute to `block` will set + * a height of `100px` by default. * * To apply a custom size to the loading indicator, set the `--size` and - * `--thickness` custom properties on the loading indicator itself. + * `--thickness` CSS custom properties on the loading indicator container. * - * If you really want to change how this looks as part of your custom theme, + * If you *really* want to change how this looks as part of your custom theme, * you can override the `border-radius` and `border` then set either a * background image, or use `content: "\"` (e.g. `content: "\f1ce"`) * and `font-family: 'Font Awesome 5 Free'` to set an FA icon if you'd rather. @@ -40,17 +47,23 @@ export interface LoadingIndicatorAttrs extends ComponentAttrs { * * - `containerClassName` Class name(s) to apply to the indicator's parent * - `className` Class name(s) to apply to the indicator itself - * - `size` Size of the loading indicator + * - `display` Determines how the spinner should be displayed (`inline`, `block` (default) or `unset`) + * - `size` Size of the loading indicator (`small`, `medium` or `large`) * - `containerAttrs` Optional attrs to be applied to the container DOM element * * All other attrs will be assigned as attributes on the DOM element. */ export default class LoadingIndicator extends Component { view() { - const { size, ...attrs } = this.attrs; + const { display = 'block', size = 'medium', containerClassName, className, ...attrs } = this.attrs; - attrs.className = classList({ LoadingIndicator: true, [attrs.className || '']: true }); - attrs.containerClassName = classList({ 'LoadingIndicator-container': true, [attrs.containerClassName || '']: true }); + const completeClassName = classList('LoadingIndicator', className); + const completeContainerClassName = classList( + 'LoadingIndicator-container', + display !== 'unset' && `LoadingIndicator-container--${display}`, + size && `LoadingIndicator-container--${size}`, + containerClassName + ); return (
{ role="status" {...attrs.containerAttrs} data-size={size} - className={attrs.containerClassName} + className={completeContainerClassName} > -
+
); } diff --git a/framework/core/js/src/forum/components/AvatarEditor.js b/framework/core/js/src/forum/components/AvatarEditor.js index 3cb28ad64..b734943f2 100644 --- a/framework/core/js/src/forum/components/AvatarEditor.js +++ b/framework/core/js/src/forum/components/AvatarEditor.js @@ -52,7 +52,13 @@ export default class AvatarEditor extends Component { ondragend={this.disableDragover.bind(this)} ondrop={this.dropUpload.bind(this)} > - {this.loading ? : user.avatarUrl() ? icon('fas fa-pencil-alt') : icon('fas fa-plus-circle')} + {this.loading ? ( + + ) : user.avatarUrl() ? ( + icon('fas fa-pencil-alt') + ) : ( + icon('fas fa-plus-circle') + )}
    {listItems(this.controlItems().toArray())}
diff --git a/framework/core/js/src/forum/components/ComposerBody.js b/framework/core/js/src/forum/components/ComposerBody.js index c5bcb1547..03e827864 100644 --- a/framework/core/js/src/forum/components/ComposerBody.js +++ b/framework/core/js/src/forum/components/ComposerBody.js @@ -5,6 +5,7 @@ import TextEditor from '../../common/components/TextEditor'; import avatar from '../../common/helpers/avatar'; import listItems from '../../common/helpers/listItems'; import ItemList from '../../common/utils/ItemList'; +import classList from '../../common/utils/classList'; /** * The `ComposerBody` component handles the body, or the content, of the @@ -66,7 +67,7 @@ export default class ComposerBody extends Component { })}
- {LoadingIndicator.component({ className: 'ComposerBody-loading' + (this.loading ? ' active' : '') })} + ); diff --git a/framework/core/js/src/forum/components/DiscussionList.js b/framework/core/js/src/forum/components/DiscussionList.js index 9ca3f5866..8ceee8266 100644 --- a/framework/core/js/src/forum/components/DiscussionList.js +++ b/framework/core/js/src/forum/components/DiscussionList.js @@ -19,7 +19,7 @@ export default class DiscussionList extends Component { let loading; if (state.isLoading()) { - loading = LoadingIndicator.component(); + loading = ; } else if (state.moreResults) { loading = Button.component( { diff --git a/framework/core/js/src/forum/components/DiscussionPage.js b/framework/core/js/src/forum/components/DiscussionPage.js index 7516df018..0374e0773 100644 --- a/framework/core/js/src/forum/components/DiscussionPage.js +++ b/framework/core/js/src/forum/components/DiscussionPage.js @@ -73,23 +73,25 @@ export default class DiscussionPage extends Page {
- {discussion - ? [ - DiscussionHero.component({ discussion }), -
- -
- {PostStream.component({ - discussion, - stream: this.stream, - onPositionChange: this.positionChanged.bind(this), - })} -
-
, - ] - : LoadingIndicator.component({ className: 'LoadingIndicator--block' })} + {discussion ? ( + [ + DiscussionHero.component({ discussion }), +
+ +
+ {PostStream.component({ + discussion, + stream: this.stream, + onPositionChange: this.positionChanged.bind(this), + })} +
+
, + ] + ) : ( + + )}
); diff --git a/framework/core/js/src/forum/components/NotificationList.js b/framework/core/js/src/forum/components/NotificationList.js index 31a079cc0..ab7a62d71 100644 --- a/framework/core/js/src/forum/components/NotificationList.js +++ b/framework/core/js/src/forum/components/NotificationList.js @@ -84,7 +84,7 @@ export default class NotificationList extends Component { }) : ''} {state.isLoading() ? ( - + ) : pages.length ? ( '' ) : ( diff --git a/framework/core/js/src/forum/components/Search.js b/framework/core/js/src/forum/components/Search.js index 248f9e1fc..bd496b3be 100644 --- a/framework/core/js/src/forum/components/Search.js +++ b/framework/core/js/src/forum/components/Search.js @@ -98,7 +98,7 @@ export default class Search extends Component { onblur={() => (this.hasFocus = false)} /> {this.loadingSources ? ( - LoadingIndicator.component({ size: 'tiny', className: 'Button Button--icon Button--link' }) + ) : currentSearch ? (