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

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 ? <span className="Button-label">{children}</span> : '',
-      this.attrs.loading ? <LoadingIndicator size="tiny" className="LoadingIndicator--inline" /> : '',
+      this.attrs.loading ? <LoadingIndicator size="small" display="inline" /> : '',
     ];
   }
 }

js/src/common/components/Checkbox.js

@@ -46,7 +46,7 @@ export default class Checkbox extends Component {
    * @protected
    */
   getDisplay() {
-    return this.attrs.loading ? <LoadingIndicator size="tiny" /> : icon(this.attrs.state ? 'fas fa-check' : 'fas fa-times');
+    return this.attrs.loading ? <LoadingIndicator display="unset" size="small" /> : icon(this.attrs.state ? 'fas fa-check' : 'fas fa-times');
   }
 
   /**

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<ComponentAttrs>;
+  /**
+   * 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: "\<glyph>"` (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<LoadingIndicatorAttrs> {
   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 (
       <div
@@ -58,9 +71,9 @@ export default class LoadingIndicator extends Component<LoadingIndicatorAttrs> {
         role="status"
         {...attrs.containerAttrs}
         data-size={size}
-        className={attrs.containerClassName}
+        className={completeContainerClassName}
       >
-        <div aria-hidden {...attrs} />
+        <div aria-hidden className={completeClassName} {...attrs} />
       </div>
     );
   }

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 ? <LoadingIndicator /> : user.avatarUrl() ? icon('fas fa-pencil-alt') : icon('fas fa-plus-circle')}
+          {this.loading ? (
+            <LoadingIndicator display="unset" size="large" />
+          ) : user.avatarUrl() ? (
+            icon('fas fa-pencil-alt')
+          ) : (
+            icon('fas fa-plus-circle')
+          )}
         </a>
         <ul className="Dropdown-menu Menu">{listItems(this.controlItems().toArray())}</ul>
       </div>

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 {
               })}
             </div>
           </div>
-          {LoadingIndicator.component({ className: 'ComposerBody-loading' + (this.loading ? ' active' : '') })}
+          <LoadingIndicator display="unset" containerClassName={classList('ComposerBody-loading', this.loading && 'active')} size="large" />
         </div>
       </ConfirmDocumentUnload>
     );

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 = <LoadingIndicator />;
     } else if (state.moreResults) {
       loading = Button.component(
         {

js/src/forum/components/DiscussionPage.js

@@ -73,23 +73,25 @@ export default class DiscussionPage extends Page {
       <div className="DiscussionPage">
         <DiscussionListPane state={app.discussions} />
         <div className="DiscussionPage-discussion">
-          {discussion
-            ? [
-                DiscussionHero.component({ discussion }),
-                <div className="container">
-                  <nav className="DiscussionPage-nav">
-                    <ul>{listItems(this.sidebarItems().toArray())}</ul>
-                  </nav>
-                  <div className="DiscussionPage-stream">
-                    {PostStream.component({
-                      discussion,
-                      stream: this.stream,
-                      onPositionChange: this.positionChanged.bind(this),
-                    })}
-                  </div>
-                </div>,
-              ]
-            : LoadingIndicator.component({ className: 'LoadingIndicator--block' })}
+          {discussion ? (
+            [
+              DiscussionHero.component({ discussion }),
+              <div className="container">
+                <nav className="DiscussionPage-nav">
+                  <ul>{listItems(this.sidebarItems().toArray())}</ul>
+                </nav>
+                <div className="DiscussionPage-stream">
+                  {PostStream.component({
+                    discussion,
+                    stream: this.stream,
+                    onPositionChange: this.positionChanged.bind(this),
+                  })}
+                </div>
+              </div>,
+            ]
+          ) : (
+            <LoadingIndicator />
+          )}
         </div>
       </div>
     );

js/src/forum/components/NotificationList.js

@@ -84,7 +84,7 @@ export default class NotificationList extends Component {
               })
             : ''}
           {state.isLoading() ? (
-            <LoadingIndicator className="LoadingIndicator--block" />
+            <LoadingIndicator />
           ) : pages.length ? (
             ''
           ) : (

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' })
+            <LoadingIndicator size="small" display="inline" containerClassName="Button Button--icon Button--link" />
           ) : currentSearch ? (
             <button className="Search-clear Button Button--icon Button--link" onclick={this.clear.bind(this)}>
               {icon('fas fa-times-circle')}

js/src/forum/components/UserPage.js

@@ -51,7 +51,7 @@ export default class UserPage extends Page {
                 </div>
               </div>,
             ]
-          : [<LoadingIndicator className="LoadingIndicator--block" />]}
+          : [<LoadingIndicator display="block" />]}
       </div>
     );
   }