more consistent structure for Loading component

Author: alexcjohnson

src/components/Loading.react.js

@@ -5,7 +5,6 @@ import DefaultSpinner from '../fragments/Loading/spinners/DefaultSpinner.jsx';
 import CubeSpinner from '../fragments/Loading/spinners/CubeSpinner.jsx';
 import CircleSpinner from '../fragments/Loading/spinners/CircleSpinner.jsx';
 import DotSpinner from '../fragments/Loading/spinners/DotSpinner.jsx';
-import {type} from 'ramda';
 
 function getSpinner(spinnerType) {
     switch (spinnerType) {
@@ -22,6 +21,19 @@ function getSpinner(spinnerType) {
     }
 }
 
+const hiddenContainer = {visibility: 'hidden', position: 'relative'};
+
+const coveringSpinner = {
+    visibility: 'visible',
+    position: 'absolute',
+    top: '0',
+    height: '100%',
+    width: '100%',
+    display: 'flex',
+    justifyContent: 'center',
+    alignItems: 'center',
+};
+
 /**
  * A Loading component that wraps any other component and displays a spinner until the wrapped component has rendered.
  */
@@ -37,40 +49,26 @@ export default class Loading extends Component {
             type: spinnerType,
         } = this.props;
 
-        if (loading_state && loading_state.is_loading) {
-            const Spinner = getSpinner(spinnerType);
-            return (
-                <div style={{visibility: 'hidden', position: 'relative'}}>
-                    {this.props.children}
-                    <Spinner
-                        className={className}
-                        style={{
-                            visibility: 'visible',
-                            position: 'absolute',
-                            top: '0',
-                            height: '100%',
-                            width: '100%',
-                            display: 'flex',
-                            justifyContent: 'center',
-                            alignItems: 'center',
-                            ...style,
-                        }}
-                        status={loading_state}
-                        color={color}
-                        debug={debug}
-                        fullscreen={fullscreen}
-                    />
+        const isLoading = loading_state && loading_state.is_loading;
+        const Spinner = isLoading && getSpinner(spinnerType);
+
+        return (
+            <div style={isLoading ? hiddenContainer : {}}>
+                {this.props.children}
+                <div style={isLoading ? coveringSpinner : {}}>
+                    {isLoading && (
+                        <Spinner
+                            className={className}
+                            style={style}
+                            status={loading_state}
+                            color={color}
+                            debug={debug}
+                            fullscreen={fullscreen}
+                        />
+                    )}
                 </div>
-            );
-        }
-
-        if (
-            type(this.props.children) !== 'Object' ||
-            type(this.props.children) !== 'Function'
-        ) {
-            return <div className={className}>{this.props.children}</div>;
-        }
-        return this.props.children;
+            </div>
+        );
     }
 }
 
@@ -98,27 +96,29 @@ Loading.propTypes = {
     ]),
 
     /**
-     * Property that determines which spinner to show - one of 'graph', 'cube', 'circle', 'dot', or 'default'.
+     * Property that determines which spinner to show
+     * one of 'graph', 'cube', 'circle', 'dot', or 'default'.
      */
     type: PropTypes.oneOf(['graph', 'cube', 'circle', 'dot', 'default']),
 
     /**
-     * Boolean that determines if the loading spinner will be displayed full-screen or not
+     * Boolean that makes the spinner display full-screen
      */
     fullscreen: PropTypes.bool,
 
     /**
-     * Boolean that determines if the loading spinner will display the status.prop_name and component_name
+     * If true, the spinner will display the component_name and prop_name
+     * while loading
      */
     debug: PropTypes.bool,
 
     /**
-     * Additional CSS class for the root DOM node
+     * Additional CSS class for the spinner root DOM node
      */
     className: PropTypes.string,
 
     /**
-     * Additional CSS styling for the root DOM node
+     * Additional CSS styling for the spinner root DOM node
      */
     style: PropTypes.object,
 

tests/unit/Loading.test.js

@@ -28,7 +28,7 @@ test('Loading renders without loading_state', () => {
         </Loading>
     );
 
-    expect(loading.html()).toEqual('<div>Loading is done!</div>');
+    expect(loading.html()).toEqual('<div>Loading is done!</div><div></div>');
 });
 test('Loading renders without loading_state.is_loading', () => {
     const statusMock = {
@@ -41,7 +41,7 @@ test('Loading renders without loading_state.is_loading', () => {
         </Loading>
     );
 
-    expect(loading.html()).toEqual('<div>Loading is done!</div>');
+    expect(loading.html()).toEqual('<div>Loading is done!</div><div></div>');
 });
 test('Loading renders without prop_name', () => {
     const statusMock = {

tests/unit/__snapshots__/Loading.test.js.snap

@@ -1,6 +1,6 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`Loading checks all it's children for a loading_state: Loading spinner for children 1`] = `"<div>Child 1</div><div>Child 2</div><div loading_state=\\"[object Object]\\">Child 3</div>"`;
+exports[`Loading checks all it's children for a loading_state: Loading spinner for children 1`] = `"<div>Child 1</div><div>Child 2</div><div loading_state=\\"[object Object]\\">Child 3</div><div></div>"`;
 
 exports[`Loading renders with multiple children: Loading with is_loading=true 1`] = `
 "<div class=\\"dash-spinner dash-default-spinner\\"><div class=\\"dash-default-spinner-rect1\\"></div><div class=\\"dash-default-spinner-rect2\\"></div><div class=\\"dash-default-spinner-rect3\\"></div><div class=\\"dash-default-spinner-rect4\\"></div><div class=\\"dash-default-spinner-rect5\\"></div></div><style>