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

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

framework/core/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');
   }
 
   /**

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<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 });
+    const completeClassName = classList('LoadingIndicator', className);
-    attrs.containerClassName = classList({ 'LoadingIndicator-container': true, [attrs.containerClassName || '']: true });
+    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>
     );
   }

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

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

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

framework/core/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
+          {discussion ? (
-            ? [
+            [
-                DiscussionHero.component({ discussion }),
+              DiscussionHero.component({ discussion }),
-                <div className="container">
+              <div className="container">
-                  <nav className="DiscussionPage-nav">
+                <nav className="DiscussionPage-nav">
-                    <ul>{listItems(this.sidebarItems().toArray())}</ul>
+                  <ul>{listItems(this.sidebarItems().toArray())}</ul>
-                  </nav>
+                </nav>
-                  <div className="DiscussionPage-stream">
+                <div className="DiscussionPage-stream">
-                    {PostStream.component({
+                  {PostStream.component({
-                      discussion,
+                    discussion,
-                      stream: this.stream,
+                    stream: this.stream,
-                      onPositionChange: this.positionChanged.bind(this),
+                    onPositionChange: this.positionChanged.bind(this),
-                    })}
+                  })}
-                  </div>
+                </div>
-                </div>,
+              </div>,
-              ]
+            ]
-            : LoadingIndicator.component({ className: 'LoadingIndicator--block' })}
+          ) : (
+            <LoadingIndicator />
+          )}
         </div>
       </div>
     );

framework/core/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 ? (
             ''
           ) : (

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' })
+            <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')}

framework/core/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>
     );
   }

framework/core/less/common/Button.less

@@ -90,14 +90,11 @@
   .Button-label {
     .transition(margin-right 0.1s);
   }
-  .LoadingIndicator {
+
+  .LoadingIndicator-container {
     color: inherit;
-    margin: 0 -5px 0 -15px;
+    margin-top: -0.175em;
-  }
+    margin-left: 4px;
-  &.loading {
-    .Button-label {
-      margin-right: 20px;
-    }
   }
 }
 

framework/core/less/common/LoadingIndicator.less

@@ -3,17 +3,6 @@
 
 .LoadingIndicator {
   @spin-time: 750ms;
-  --size: 24px;
-  --thickness: 2px;
-
-  &-container[data-size="large"] & {
-    --size: 32px;
-    --thickness: 3px;
-  }
-
-  &-container[data-size="tiny"] & {
-    --size: 18px;
-  }
 
   // Use the value of `color` to maintain backwards compatibility
   border-color: currentColor;
@@ -30,6 +19,9 @@
   // <div> container around the spinner
   // Used for positioning
   &-container {
+    --size: 24px;
+    --thickness: 2px;
+
     color: @muted-color;
 
     // Center vertically and horizontally
@@ -38,12 +30,26 @@
     align-items: center;
     justify-content: center;
 
+    // Size
+
+    &--large {
+      --size: 32px;
+      --thickness: 3px;
+    }
+
+    &--small {
+      --size: 18px;
+    }
+
+    // Display types
+
     &--block {
       height: 100px;
     }
 
     &--inline {
       display: inline-block;
+      vertical-align: middle;
     }
   }
 }

framework/core/less/common/Search.less

@@ -8,7 +8,8 @@
     &.focused {
       margin-left: -400px;
 
-      input, .Search-results {
+      input,
+      .Search-results {
         width: 400px;
       }
     }
@@ -61,11 +62,21 @@
     .transition(all 0.4s);
     box-sizing: inherit !important;
   }
+
+  .LoadingIndicator-container {
+    height: 36px;
+  }
+
   .Button {
     float: left;
     margin-left: -36px;
-    width: 36px !important;
     outline: none;
+    width: 36px !important;
+
+    &.LoadingIndicator {
+      width: var(--size) !important;
+      padding: 0;
+    }
   }
 }
 

framework/core/less/forum/AvatarEditor.less

@@ -23,7 +23,7 @@
   &.dragover .Dropdown-toggle {
     opacity: 1;
   }
-  .LoadingIndicator {
+  .LoadingIndicator-container {
     color: #fff;
     position: absolute;
     left: 0;

framework/core/less/forum/Composer.less

@@ -92,7 +92,7 @@
   border-radius: @border-radius @border-radius 0 0;
 
   &.active {
-    display: block;
+    display: flex;
   }
 }
 .ComposerBody-editor {