From 7d1136b0ec2a79202ebb71b0b760b8c494453dbe Mon Sep 17 00:00:00 2001
From: Chris Williams <williaster@users.noreply.github.com>
Date: Tue, 26 Jun 2018 16:47:41 -0700
Subject: [PATCH] fix sqllab <Loading /> css, fix double AddSliceCard margin
 and drag border (#5293)

* fix sqllab <Loading /> css, fix double AddSliceCard margin and drag border

* [dashboard v2] improve cached visual indicator, add last fetched messaging

* [dashboard v2] fix ctrl + cmd + z + UndoRedoKeylisteners

(cherry picked from commit 05a39b3)
---
 superset/assets/src/SqlLab/TODO.md            | 19 -------------
 .../src/SqlLab/components/ResultSet.jsx       |  6 +++--
 .../src/SqlLab/components/SouthPane.jsx       |  2 +-
 superset/assets/src/SqlLab/main.less          |  4 +--
 .../components/BuilderComponentPane.jsx       |  2 --
 .../src/dashboard/components/SliceHeader.jsx  |  4 +++
 .../components/SliceHeaderControls.jsx        | 27 +++++++++----------
 .../components/UndoRedoKeylisteners.jsx       |  7 +++--
 .../components/dnd/AddSliceDragPreview.jsx    |  1 -
 .../components/gridComponents/Chart.jsx       |  3 ++-
 .../assets/src/dashboard/containers/Chart.jsx |  4 ++-
 .../stylesheets/builder-sidepane.less         |  4 +--
 .../stylesheets/components/chart.less         |  5 ----
 13 files changed, 34 insertions(+), 54 deletions(-)
 delete mode 100644 superset/assets/src/SqlLab/TODO.md

diff --git a/superset/assets/src/SqlLab/TODO.md b/superset/assets/src/SqlLab/TODO.md
deleted file mode 100644
index db8f247abff05..0000000000000
--- a/superset/assets/src/SqlLab/TODO.md
+++ /dev/null
@@ -1,19 +0,0 @@
-
-# TODO
-* Figure out how to organize the left panel, integrate Search
-* collapse sql beyond 10 lines
-* Security per-database (dropdown)
-* Get a to work
-
-## Cosmetic
-* Result set font is too big
-* lmiit/timer/buttons wrap
-* table label is transparent
-* SqlEditor buttons
-* use react-bootstrap-prompt for query title input
-* Make tabs look great
-
-# PROJECT
-* Write Runbook
-* Confirm backups
-* merge chef branch
diff --git a/superset/assets/src/SqlLab/components/ResultSet.jsx b/superset/assets/src/SqlLab/components/ResultSet.jsx
index bd446bba53778..417a6d11aa5ef 100644
--- a/superset/assets/src/SqlLab/components/ResultSet.jsx
+++ b/superset/assets/src/SqlLab/components/ResultSet.jsx
@@ -31,6 +31,8 @@ const defaultProps = {
 
 const SEARCH_HEIGHT = 46;
 
+const LOADING_STYLES = { position: 'relative', height: 50 };
+
 export default class ResultSet extends React.PureComponent {
   constructor(props) {
     super(props);
@@ -234,9 +236,9 @@ export default class ResultSet extends React.PureComponent {
       );
     }
     return (
-      <div>
-        <Loading />
+      <div style={LOADING_STYLES}>
         <QueryStateLabel query={query} />
+        {!progressBar && <Loading />}
         {progressBar}
         <div>
           {trackingUrl}
diff --git a/superset/assets/src/SqlLab/components/SouthPane.jsx b/superset/assets/src/SqlLab/components/SouthPane.jsx
index 318b0af005c18..1ea8e27e8a13c 100644
--- a/superset/assets/src/SqlLab/components/SouthPane.jsx
+++ b/superset/assets/src/SqlLab/components/SouthPane.jsx
@@ -87,7 +87,7 @@ class SouthPane extends React.PureComponent {
             title={t('Query History')}
             eventKey="History"
           >
-            <div style={{ height: `${innerTabHeight}px`, overflow: 'scroll' }}>
+            <div style={{ height: `${innerTabHeight}px`, overflow: 'auto' }}>
               <QueryHistory queries={props.editorQueries} actions={props.actions} />
             </div>
           </Tab>
diff --git a/superset/assets/src/SqlLab/main.less b/superset/assets/src/SqlLab/main.less
index df3ba9b95e19e..be6d364a6bc92 100644
--- a/superset/assets/src/SqlLab/main.less
+++ b/superset/assets/src/SqlLab/main.less
@@ -45,7 +45,7 @@ body {
   left: 0px;
   right: 0px;
   bottom: 0px;
-  overflow: scroll;
+  overflow: auto;
   margin-right: 0px;
   margin-bottom: 0px;
 }
@@ -302,7 +302,7 @@ a.Link {
     margin-top: 10px;
     position: absolute;
     width: 100%;
-    overflow: scroll;
+    overflow: auto;
 }
 .nav-tabs > li.active > a,
 .nav-tabs > li.active > a:hover,
diff --git a/superset/assets/src/dashboard/components/BuilderComponentPane.jsx b/superset/assets/src/dashboard/components/BuilderComponentPane.jsx
index aafee5dcabce1..a21ec11de7ba3 100644
--- a/superset/assets/src/dashboard/components/BuilderComponentPane.jsx
+++ b/superset/assets/src/dashboard/components/BuilderComponentPane.jsx
@@ -87,11 +87,9 @@ class BuilderComponentPane extends React.PureComponent {
 
                           <i className="fa fa-arrow-right trigger" />
                         </div>
-
                         <NewTabs />
                         <NewRow />
                         <NewColumn />
-
                         <NewHeader />
                         <NewMarkdown />
                         <NewDivider />
diff --git a/superset/assets/src/dashboard/components/SliceHeader.jsx b/superset/assets/src/dashboard/components/SliceHeader.jsx
index d812181039cd0..f2d715b7144b1 100644
--- a/superset/assets/src/dashboard/components/SliceHeader.jsx
+++ b/superset/assets/src/dashboard/components/SliceHeader.jsx
@@ -12,6 +12,7 @@ const propTypes = {
   isExpanded: PropTypes.bool,
   isCached: PropTypes.bool,
   cachedDttm: PropTypes.string,
+  updatedDttm: PropTypes.number,
   updateSliceName: PropTypes.func,
   toggleExpandSlice: PropTypes.func,
   forceRefresh: PropTypes.func,
@@ -37,6 +38,7 @@ const defaultProps = {
   annotationQuery: {},
   annotationError: {},
   cachedDttm: null,
+  updatedDttm: null,
   isCached: false,
   isExpanded: false,
   sliceName: '',
@@ -51,6 +53,7 @@ class SliceHeader extends React.PureComponent {
       isExpanded,
       isCached,
       cachedDttm,
+      updatedDttm,
       toggleExpandSlice,
       forceRefresh,
       exploreChart,
@@ -102,6 +105,7 @@ class SliceHeader extends React.PureComponent {
               isCached={isCached}
               isExpanded={isExpanded}
               cachedDttm={cachedDttm}
+              updatedDttm={updatedDttm}
               toggleExpandSlice={toggleExpandSlice}
               forceRefresh={forceRefresh}
               exploreChart={exploreChart}
diff --git a/superset/assets/src/dashboard/components/SliceHeaderControls.jsx b/superset/assets/src/dashboard/components/SliceHeaderControls.jsx
index 7d25ab08e643f..4116a3431fa96 100644
--- a/superset/assets/src/dashboard/components/SliceHeaderControls.jsx
+++ b/superset/assets/src/dashboard/components/SliceHeaderControls.jsx
@@ -1,6 +1,5 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import cx from 'classnames';
 import moment from 'moment';
 import { Dropdown, MenuItem } from 'react-bootstrap';
 import {
@@ -17,6 +16,7 @@ const propTypes = {
   isCached: PropTypes.bool,
   isExpanded: PropTypes.bool,
   cachedDttm: PropTypes.string,
+  updatedDttm: PropTypes.number,
   supersetCanExplore: PropTypes.bool,
   sliceCanEdit: PropTypes.bool,
   toggleExpandSlice: PropTypes.func,
@@ -31,6 +31,7 @@ const defaultProps = {
   exploreChart: () => ({}),
   exportCSV: () => ({}),
   cachedDttm: null,
+  updatedDttm: null,
   isCached: false,
   isExpanded: false,
   supersetCanExplore: false,
@@ -101,19 +102,20 @@ class SliceHeaderControls extends React.PureComponent {
   }
 
   render() {
-    const slice = this.props.slice;
-    const isCached = this.props.isCached;
-    const cachedWhen = moment.utc(this.props.cachedDttm).fromNow();
-    const refreshTooltip = isCached ? t('Cached %s', cachedWhen) : '';
-
-    // @TODO account for
-    //  dashboard.dashboard.superset_can_explore
-    //  dashboard.dashboard.slice_can_edit
+    const { slice, isCached, cachedDttm, updatedDttm } = this.props;
+    const cachedWhen = moment.utc(cachedDttm).fromNow();
+    const updatedWhen = updatedDttm ? moment.utc(updatedDttm).fromNow() : '';
+    const refreshTooltip = isCached
+      ? t('Cached %s', cachedWhen)
+      : (updatedWhen && t('Fetched %s', updatedWhen)) || '';
+
     return (
       <Dropdown
         id={`slice_${slice.slice_id}-controls`}
-        className={cx(isCached && 'is-cached')}
         pullRight
+        // react-bootstrap handles visibility, but call toggle to force a re-render
+        // and update the fetched/cached timestamps
+        onToggle={this.toggleControls}
       >
         <Dropdown.Toggle className="slice-header-controls-trigger" noCaret>
           <VerticalDotsTrigger />
@@ -121,11 +123,8 @@ class SliceHeaderControls extends React.PureComponent {
 
         <Dropdown.Menu>
           <MenuItem onClick={this.refreshChart}>
-            {isCached && <span className="dot" />}
             {t('Force refresh')}
-            {isCached && (
-              <div className="refresh-tooltip">{refreshTooltip}</div>
-            )}
+            <div className="refresh-tooltip">{refreshTooltip}</div>
           </MenuItem>
 
           <MenuItem divider />
diff --git a/superset/assets/src/dashboard/components/UndoRedoKeylisteners.jsx b/superset/assets/src/dashboard/components/UndoRedoKeylisteners.jsx
index 5af0934a61c79..c6de151182c2b 100644
--- a/superset/assets/src/dashboard/components/UndoRedoKeylisteners.jsx
+++ b/superset/assets/src/dashboard/components/UndoRedoKeylisteners.jsx
@@ -21,13 +21,12 @@ class UndoRedoKeylisteners extends React.PureComponent {
   }
 
   handleKeydown(event) {
-    const controlOrCommand = event.keyCode === 90 || event.metaKey;
+    const controlOrCommand = event.ctrlKey || event.metaKey;
     if (controlOrCommand) {
       const isZChar = event.key === 'z' || event.keyCode === 90;
       const isYChar = event.key === 'y' || event.keyCode === 89;
-      const isEditingMarkdown = document.querySelector(
-        '.dashboard-markdown--editing',
-      );
+      const isEditingMarkdown =
+        document && document.querySelector('.dashboard-markdown--editing');
 
       if (!isEditingMarkdown && (isZChar || isYChar)) {
         event.preventDefault();
diff --git a/superset/assets/src/dashboard/components/dnd/AddSliceDragPreview.jsx b/superset/assets/src/dashboard/components/dnd/AddSliceDragPreview.jsx
index 2c1128e9bb3d1..485e4e317135b 100644
--- a/superset/assets/src/dashboard/components/dnd/AddSliceDragPreview.jsx
+++ b/superset/assets/src/dashboard/components/dnd/AddSliceDragPreview.jsx
@@ -11,7 +11,6 @@ import {
 
 const staticCardStyles = {
   position: 'fixed',
-  background: 'rgba(255, 255, 255, 0.7)',
   pointerEvents: 'none',
   top: 0,
   left: 0,
diff --git a/superset/assets/src/dashboard/components/gridComponents/Chart.jsx b/superset/assets/src/dashboard/components/gridComponents/Chart.jsx
index 39c1e81dcb899..4713c4361adc3 100644
--- a/superset/assets/src/dashboard/components/gridComponents/Chart.jsx
+++ b/superset/assets/src/dashboard/components/gridComponents/Chart.jsx
@@ -160,7 +160,7 @@ class Chart extends React.Component {
     if (!chart || !slice) return null;
 
     const { width } = this.state;
-    const { queryResponse } = chart;
+    const { queryResponse, chartUpdateEndTime } = chart;
     const isCached = queryResponse && queryResponse.is_cached;
     const cachedDttm = queryResponse && queryResponse.cached_dttm;
     const isOverflowable = OVERFLOWABLE_VIZ_TYPES.has(slice && slice.viz_type);
@@ -173,6 +173,7 @@ class Chart extends React.Component {
           isExpanded={!!isExpanded}
           isCached={isCached}
           cachedDttm={cachedDttm}
+          updatedDttm={chartUpdateEndTime}
           toggleExpandSlice={toggleExpandSlice}
           forceRefresh={this.forceRefresh}
           editMode={editMode}
diff --git a/superset/assets/src/dashboard/containers/Chart.jsx b/superset/assets/src/dashboard/containers/Chart.jsx
index b2f0795acb0ad..97b2bb8e18a6b 100644
--- a/superset/assets/src/dashboard/containers/Chart.jsx
+++ b/superset/assets/src/dashboard/containers/Chart.jsx
@@ -28,7 +28,9 @@ function mapStateToProps(
 
   return {
     chart,
-    datasource: (chart && datasources[chart.form_data.datasource]) || {},
+    datasource:
+      (chart && chart.form_data && datasources[chart.form_data.datasource]) ||
+      {},
     slice: sliceEntities.slices[id],
     timeout: dashboardInfo.common.conf.SUPERSET_WEBSERVER_TIMEOUT,
     filters: filters[id] || EMPTY_FILTERS,
diff --git a/superset/assets/src/dashboard/stylesheets/builder-sidepane.less b/superset/assets/src/dashboard/stylesheets/builder-sidepane.less
index e807bcc30ad5b..17c87a8f3c189 100644
--- a/superset/assets/src/dashboard/stylesheets/builder-sidepane.less
+++ b/superset/assets/src/dashboard/stylesheets/builder-sidepane.less
@@ -29,7 +29,6 @@
   .viewport {
     position: absolute;
     transform: none !important;
-    background: white;
     overflow: hidden;
     width: @builder-pane-width;
     height: 100%;
@@ -77,9 +76,10 @@
       border: 1px solid @gray-light;
       font-weight: 200;
       padding: 16px;
-      margin: 16px;
+      margin: 0 16px 16px 16px;
       position: relative;
       cursor: move;
+      background: rgba(255, 255, 255, 0.4);
 
       &:hover {
         background: @gray-bg;
diff --git a/superset/assets/src/dashboard/stylesheets/components/chart.less b/superset/assets/src/dashboard/stylesheets/components/chart.less
index bc0005dbb5eb7..b4e0f776d9087 100644
--- a/superset/assets/src/dashboard/stylesheets/components/chart.less
+++ b/superset/assets/src/dashboard/stylesheets/components/chart.less
@@ -83,11 +83,6 @@
   margin: 2px 0;
   display: inline-block;
 
-  .is-cached & {
-    background-color: @pink;
-    box-shadow: 0 0 5px 1.5px rgba(255, 0, 0, 0.3);
-  }
-
   .vertical-dots-container & {
     display: block;
   }