fix: Loading overlay bugfix and cleanup (apache#10105)

* fix: reordering DOM output, simplifying styles, Emotionalizing * simplification * converting RefreshChartOverlay to TS * Loading -> TS, stripping unused size prop * simplification... * just letting "position" prop act as a class name. Simpler! * consolidating styles, changing a className prop to a position prop. * nixing (unused) classname prop * replacing inline loading img with the proper Loading component * BY THERE. * position prop is optional!
airbnb · Jun 24, 2020 · 36ea42f · 36ea42f
1 parent c544ebd
commit 36ea42f
Show file tree

Hide file tree

Showing 10 changed files with 70 additions and 129 deletions.
diff --git a/superset-frontend/src/SqlLab/components/TableElement.jsx b/superset-frontend/src/SqlLab/components/TableElement.jsx
@@ -207,7 +207,7 @@ class TableElement extends React.PureComponent {
         </div>
         <div className="pull-right">
           {table.isMetadataLoading || table.isExtraMetadataLoading ? (
-            <Loading size={50} position="normal" className="margin-zero" />
+            <Loading position="inline" />
           ) : (
             <Fade in={this.state.hovered}>{this.renderControls()}</Fade>
           )}

diff --git a/superset-frontend/src/SqlLab/main.less b/superset-frontend/src/SqlLab/main.less
@@ -39,11 +39,6 @@ body {
   padding: 0px;
 }
 
-.loading {
-  width: 50px;
-  margin-top: 15px;
-}
-
 .pane-cell {
   padding: 10px;
   overflow: auto;

diff --git a/superset-frontend/src/chart/Chart.jsx b/superset-frontend/src/chart/Chart.jsx
@@ -182,7 +182,9 @@ class Chart extends React.PureComponent {
           className={`chart-container ${isLoading ? 'is-loading' : ''}`}
           style={containerStyles}
         >
-          {isLoading && <Loading size={50} />}
+          <div className={`slice_container ${isFaded ? ' faded' : ''}`}>
+            <ChartRenderer {...this.props} />
+          </div>
 
           {!isLoading && !chartAlert && isFaded && (
             <RefreshChartOverlay
@@ -191,9 +193,8 @@ class Chart extends React.PureComponent {
               onQuery={onQuery}
             />
           )}
-          <div className={`slice_container ${isFaded ? ' faded' : ''}`}>
-            <ChartRenderer {...this.props} />
-          </div>
+
+          {isLoading && <Loading />}
         </div>
       </ErrorBoundary>
     );

diff --git a/superset-frontend/src/components/Button.jsx b/superset-frontend/src/components/Button.jsx
@@ -27,6 +27,7 @@ import {
 
 const propTypes = {
   children: PropTypes.node,
+  className: PropTypes.string,
   tooltip: PropTypes.node,
   placement: PropTypes.string,
   onClick: PropTypes.func,

diff --git a/superset-frontend/src/components/Loading.jsx b/superset-frontend/src/components/Loading.jsx
diff --git a/...rset-frontend/src/components/Loading.less → superset-frontend/src/components/Loading.tsx b/...rset-frontend/src/components/Loading.less → superset-frontend/src/components/Loading.tsx
@@ -16,6 +16,37 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-img.loading.margin-zero {
-  margin: 0px;
+import React from 'react';
+import styled from '@superset-ui/style';
+
+interface Props {
+  position?: string;
+}
+
+const LoaderImg = styled.img`
+  z-index: 1000;
+  width: 50px;
+  position: relative;
+  margin: 10px;
+  &.inline {
+    margin: 0px;
+    width: 30px;
+  }
+  &.floating {
+    padding: 0;
+    margin: 0;
+    position: absolute;
+    left: 50%;
+    top: 50%;
+    transform: translate(-50%, -50%);
+  }
+`;
+export default function Loading({ position = 'floating' }: Props) {
+  return (
+    <LoaderImg
+      className={`loading ${position}`}
+      alt="Loading..."
+      src="/static/assets/images/loading.gif"
+    />
+  );
 }
diff --git a/...nd/src/components/RefreshChartOverlay.jsx → ...nd/src/components/RefreshChartOverlay.tsx b/...nd/src/components/RefreshChartOverlay.jsx → ...nd/src/components/RefreshChartOverlay.tsx
@@ -17,37 +17,50 @@
  * under the License.
  */
 import React from 'react';
-import PropTypes from 'prop-types';
 import { t } from '@superset-ui/translation';
-import Button from '../components/Button';
+import styled from '@superset-ui/style';
+import Button from 'src/components/Button';
 
-const propTypes = {
-  height: PropTypes.number.isRequired,
-  width: PropTypes.number.isRequired,
-  onQuery: PropTypes.func,
-};
+type Callback = (...args: any[]) => void;
 
-class RefreshChartOverlay extends React.PureComponent {
+interface Props {
+  height: number;
+  width: number;
+  onQuery: Callback;
+}
+
+const RefreshOverlayWrapper = styled.div`
+  position: absolute;
+  top: 0;
+  left: 0;
+  width: 100%;
+  height: 100%;
+  display: flex;
+  align-items: center;
+  justify-content: center;
+  text-align: center;
+
+  .refresh-btn {
+    font-weight: ${({ theme }) => theme.typography.weights.bold};
+  }
+`;
+
+class RefreshChartOverlay extends React.PureComponent<Props> {
   render() {
     return (
-      <div
-        style={{ height: this.props.height, width: this.props.width }}
-        className="explore-chart-overlay"
-      >
+      <RefreshOverlayWrapper>
         <div>
           <Button
-            className="refresh-overlay-btn"
+            className="refresh-btn"
             onClick={this.props.onQuery}
             bsStyle="primary"
           >
             {t('Run Query')}
           </Button>
         </div>
-      </div>
+      </RefreshOverlayWrapper>
     );
   }
 }
 
-RefreshChartOverlay.propTypes = propTypes;
-
 export default RefreshChartOverlay;
diff --git a/superset-frontend/src/dashboard/stylesheets/dashboard.less b/superset-frontend/src/dashboard/stylesheets/dashboard.less
@@ -151,12 +151,6 @@ body {
 }
 
 .modal {
-  img.loading {
-    width: 50px;
-    margin: 0;
-    position: relative;
-  }
-
   .modal-body {
     padding: 24px 24px 29px 24px;
   }

diff --git a/superset-frontend/src/explore/components/DisplayQueryButton.jsx b/superset-frontend/src/explore/components/DisplayQueryButton.jsx
@@ -208,13 +208,7 @@ export class DisplayQueryButton extends React.PureComponent {
   }
   renderSamplesModalBody() {
     if (this.state.isLoading) {
-      return (
-        <img
-          className="loading"
-          alt="Loading..."
-          src="/static/assets/images/loading.gif"
-        />
-      );
+      return <Loading />;
     } else if (this.state.error) {
       return <pre>{this.state.error}</pre>;
     } else if (this.state.data) {

diff --git a/superset-frontend/stylesheets/superset.less b/superset-frontend/stylesheets/superset.less
@@ -153,12 +153,6 @@ div.navbar {
   }
 }
 
-img.loading {
-  width: 40px;
-  position: relative;
-  margin: 10px;
-}
-
 img.viz-thumb-option {
   width: 100px;
   border: 1px solid @gray;
@@ -527,27 +521,6 @@ tr.reactable-column-header th.reactable-header-sortable {
   padding-right: 17px;
 }
 
-.explore-chart-overlay {
-  position: absolute;
-  z-index: @z-index-above-dashboard-charts;
-  display: flex;
-  align-items: center;
-  justify-content: center;
-  text-align: center;
-}
-
-.refresh-overlay-btn {
-  font-weight: @font-weight-bold;
-  margin-right: 10px;
-}
-
-.dismiss-overlay-btn {
-  font-weight: @font-weight-bold;
-  background: @lightest;
-  color: @brand-primary;
-  border-color: @brand-primary;
-}
-
 .fave-unfave-icon {
   .fa-star-o,
   .fa-star {