You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by cc...@apache.org on 2018/04/18 22:20:35 UTC

[incubator-superset] 01/02: [dashboard builder] fix dashboard filters and some css

This is an automated email from the ASF dual-hosted git repository.

ccwilliams pushed a commit to branch chris--dashboard-perf
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git

commit 2e5295da13a7bfb69544f91aba6336d1627b0bc5
Author: Chris Williams <ch...@airbnb.com>
AuthorDate: Wed Apr 18 11:38:56 2018 -0700

    [dashboard builder] fix dashboard filters and some css
---
 .../javascripts/dashboard/components/Dashboard.jsx | 112 ++++-----------------
 .../dashboard/components/SliceHeader.jsx           |  75 +++++++-------
 .../v2/components/gridComponents/Chart.jsx         |  10 +-
 .../dashboard/v2/stylesheets/components/chart.less |  35 ++++++-
 .../v2/util/charts/getFormDataWithExtraFilters.js  |  25 +++--
 superset/assets/stylesheets/dashboard.less         |  80 +--------------
 superset/assets/visualizations/nvd3_vis.css        |   4 +
 7 files changed, 116 insertions(+), 225 deletions(-)

diff --git a/superset/assets/javascripts/dashboard/components/Dashboard.jsx b/superset/assets/javascripts/dashboard/components/Dashboard.jsx
index 292c359..7ab9a9d 100644
--- a/superset/assets/javascripts/dashboard/components/Dashboard.jsx
+++ b/superset/assets/javascripts/dashboard/components/Dashboard.jsx
@@ -3,12 +3,16 @@ import PropTypes from 'prop-types';
 
 import AlertsWrapper from '../../components/AlertsWrapper';
 import DashboardBuilder from '../v2/containers/DashboardBuilder';
-// import GridLayout from './GridLayout';
 import { slicePropShape } from '../reducers/propShapes';
-// import { exportChart } from '../../explore/exploreUtils';
 import { areObjectsEqual } from '../../reduxUtils';
-import { Logger, ActionLog, LOG_ACTIONS_PAGE_LOAD,
-  LOG_ACTIONS_LOAD_EVENT, LOG_ACTIONS_RENDER_EVENT } from '../../logger';
+import getFormDataWithExtraFilters from '../v2/util/charts/getFormDataWithExtraFilters';
+import {
+  Logger,
+  ActionLog,
+  LOG_ACTIONS_PAGE_LOAD,
+  LOG_ACTIONS_LOAD_EVENT,
+  LOG_ACTIONS_RENDER_EVENT,
+} from '../../logger';
 import { t } from '../../locales';
 
 import '../../../stylesheets/dashboard.less';
@@ -57,24 +61,6 @@ class Dashboard extends React.PureComponent {
       eventNames: [LOG_ACTIONS_LOAD_EVENT, LOG_ACTIONS_RENDER_EVENT],
     });
     Logger.start(this.loadingLog);
-
-    // this.rerenderCharts = this.rerenderCharts.bind(this);
-    // this.getFormDataExtra = this.getFormDataExtra.bind(this);
-    // this.exploreChart = this.exploreChart.bind(this);
-    // this.exportCSV = this.exportCSV.bind(this);
-
-    // this.props.actions.saveSliceName = this.props.actions.saveSliceName.bind(this);
-    // this.props.actions.removeSliceFromDashboard =
-      // this.props.actions.removeSliceFromDashboard.bind(this);
-    // this.props.actions.toggleExpandSlice =
-      // this.props.actions.toggleExpandSlice.bind(this);
-    // this.props.actions.addFilter = this.props.actions.addFilter.bind(this);
-    // this.props.actions.removeFilter = this.props.actions.removeFilter.bind(this);
-  }
-
-  componentDidMount() {
-    // grid does this now
-    // window.addEventListener('resize', this.rerenderCharts);
   }
 
   componentWillReceiveProps(nextProps) {
@@ -144,17 +130,6 @@ class Dashboard extends React.PureComponent {
     return Object.values(this.props.charts);
   }
 
-  // getFormDataExtra(chart) {
-  //   const formDataExtra = Object.assign({}, chart.formData);
-  //   const extraFilters = this.effectiveExtraFilters(chart.slice_id);
-  //   formDataExtra.extra_filters = formDataExtra.filters.concat(extraFilters);
-  //   return formDataExtra;
-  // }
-
-  getFilters(sliceId) {
-    return this.props.filters[sliceId];
-  }
-
   getChartKeysFromLayout(layout) {
     return Object.values(layout)
       .reduce((chartKeys, value) => {
@@ -171,78 +146,27 @@ class Dashboard extends React.PureComponent {
     return message; // Gecko + Webkit, Safari, Chrome etc.
   }
 
-  // effectiveExtraFilters(sliceId) {
-  //   const metadata = this.props.dashboard.metadata;
-  //   const filters = this.props.filters;
-  //   const f = [];
-  //   const immuneSlices = metadata.filter_immune_slices || [];
-  //   if (sliceId && immuneSlices.includes(sliceId)) {
-  //     // The slice is immune to dashboard filters
-  //     return f;
-  //   }
-  //
-  //   // Building a list of fields the slice is immune to filters on
-  //   let immuneToFields = [];
-  //   if (
-  //     sliceId &&
-  //     metadata.filter_immune_slice_fields &&
-  //     metadata.filter_immune_slice_fields[sliceId]) {
-  //     immuneToFields = metadata.filter_immune_slice_fields[sliceId];
-  //   }
-  //   for (const filteringSliceId in filters) {
-  //     if (filteringSliceId === sliceId.toString()) {
-  //       // Filters applied by the slice don't apply to itself
-  //       continue;
-  //     }
-  //     for (const field in filters[filteringSliceId]) {
-  //       if (!immuneToFields.includes(field)) {
-  //         f.push({
-  //           col: field,
-  //           op: 'in',
-  //           val: filters[filteringSliceId][field],
-  //         });
-  //       }
-  //     }
-  //   }
-  //   return f;
-  // }
-
   refreshExcept(filterKey) {
     const immune = this.props.dashboard.metadata.filter_immune_slices || [];
     let charts = this.getAllCharts();
     if (filterKey) {
-      charts = charts.filter(slice => (
-        String(slice.slice_id) !== filterKey &&
-        immune.indexOf(slice.slice_id) === -1
+      charts = charts.filter(chart => (
+        String(chart.slice_id) !== filterKey &&
+        immune.indexOf(chart.slice_id) === -1
       ));
     }
     charts.forEach((chart) => {
-      const updatedFormData = this.getFormDataExtra(chart);
+      const updatedFormData = getFormDataWithExtraFilters({
+        chart,
+        dashboardMetadata: this.props.dashboard.metadata,
+        filters: this.props.filters,
+        sliceId: chart.chartKey,
+      });
+
       this.props.actions.runQuery(updatedFormData, false, this.props.timeout, chart.chartKey);
     });
   }
 
-  // exploreChart(chartKey) {
-  //   const chart = this.props.charts[chartKey];
-  //   const formData = this.getFormDataExtra(chart);
-  //   exportChart(formData);
-  // }
-  //
-  // exportCSV(chartKey) {
-  //   const chart = this.props.charts[chartKey];
-  //   const formData = this.getFormDataExtra(chart);
-  //   exportChart(formData, 'csv');
-  // }
-
-  // re-render chart without fetch
-  // rerenderCharts() {
-  //   this.getAllCharts().forEach((chart) => {
-  //     setTimeout(() => {
-  //       this.props.actions.renderTriggered(new Date().getTime(), chart.chartKey);
-  //     }, 50);
-  //   });
-  // }
-
   render() {
     return (
       <div id="dashboard-container">
diff --git a/superset/assets/javascripts/dashboard/components/SliceHeader.jsx b/superset/assets/javascripts/dashboard/components/SliceHeader.jsx
index d291b3b..adfa4c6 100644
--- a/superset/assets/javascripts/dashboard/components/SliceHeader.jsx
+++ b/superset/assets/javascripts/dashboard/components/SliceHeader.jsx
@@ -64,45 +64,44 @@ class SliceHeader extends React.PureComponent {
 
     return (
       <div className="chart-header" ref={innerRef}>
-        <div className="col-md-12">
-          <div className="header">
-            <EditableTitle
-              title={slice.slice_name}
-              canEdit={!!this.props.updateSliceName && this.props.editMode}
-              onSaveTitle={this.onSaveTitle}
-              noPermitTooltip={'You don\'t have the rights to alter this dashboard.'}
+        <div className="header">
+          <EditableTitle
+            title={slice.slice_name}
+            canEdit={!!this.props.updateSliceName && this.props.editMode}
+            onSaveTitle={this.onSaveTitle}
+            noPermitTooltip={'You don\'t have the rights to alter this dashboard.'}
+            showTooltip={!!this.props.updateSliceName && this.props.editMode}
+          />
+          {!!Object.values(this.props.annotationQuery || {}).length &&
+            <TooltipWrapper
+              label="annotations-loading"
+              placement="top"
+              tooltip={annoationsLoading}
+            >
+              <i className="fa fa-refresh warning" />
+            </TooltipWrapper>
+          }
+          {!!Object.values(this.props.annotationError || {}).length &&
+            <TooltipWrapper
+              label="annoation-errors"
+              placement="top"
+              tooltip={annoationsError}
+            >
+              <i className="fa fa-exclamation-circle danger" />
+            </TooltipWrapper>
+          }
+          {!this.props.editMode &&
+            <SliceHeaderControls
+              slice={slice}
+              isCached={isCached}
+              isExpanded={isExpanded}
+              cachedDttm={cachedDttm}
+              toggleExpandSlice={toggleExpandSlice}
+              forceRefresh={forceRefresh}
+              exploreChart={exploreChart}
+              exportCSV={exportCSV}
             />
-            {!!Object.values(this.props.annotationQuery || {}).length &&
-              <TooltipWrapper
-                label="annotations-loading"
-                placement="top"
-                tooltip={annoationsLoading}
-              >
-                <i className="fa fa-refresh warning" />
-              </TooltipWrapper>
-            }
-            {!!Object.values(this.props.annotationError || {}).length &&
-              <TooltipWrapper
-                label="annoation-errors"
-                placement="top"
-                tooltip={annoationsError}
-              >
-                <i className="fa fa-exclamation-circle danger" />
-              </TooltipWrapper>
-            }
-            {!this.props.editMode &&
-              <SliceHeaderControls
-                slice={slice}
-                isCached={isCached}
-                isExpanded={isExpanded}
-                cachedDttm={cachedDttm}
-                toggleExpandSlice={toggleExpandSlice}
-                forceRefresh={forceRefresh}
-                exploreChart={exploreChart}
-                exportCSV={exportCSV}
-              />
-            }
-          </div>
+          }
         </div>
       </div>
     );
diff --git a/superset/assets/javascripts/dashboard/v2/components/gridComponents/Chart.jsx b/superset/assets/javascripts/dashboard/v2/components/gridComponents/Chart.jsx
index 889a455..bc872ef 100644
--- a/superset/assets/javascripts/dashboard/v2/components/gridComponents/Chart.jsx
+++ b/superset/assets/javascripts/dashboard/v2/components/gridComponents/Chart.jsx
@@ -1,3 +1,4 @@
+import cx from 'classnames';
 import React from 'react';
 import PropTypes from 'prop-types';
 
@@ -51,6 +52,9 @@ class Chart extends React.Component {
   }
 
   shouldComponentUpdate(nextProps, nextState) {
+    // this logic mostly pertains to chart resizing. we keep a copy of the dimensions in
+    // state so that we can buffer component size updates and only update on the final call
+    // which improves performance significantly
     if (nextState.width !== this.state.width || nextState.height !== this.state.height) {
       return true;
     }
@@ -58,7 +62,6 @@ class Chart extends React.Component {
     for (let i = 0; i < updateOnPropChange.length; i += 1) {
       const prop = updateOnPropChange[i];
       if (nextProps[prop] !== this.props[prop]) {
-        console.log(prop, 'changed')
         return true;
       }
     }
@@ -104,7 +107,7 @@ class Chart extends React.Component {
     this.setState(() => ({ width, height }));
   }
 
-  addFilter(args) {
+  addFilter(...args) {
     this.props.addFilter(this.props.chart, ...args);
   }
 
@@ -141,9 +144,10 @@ class Chart extends React.Component {
     const { queryResponse } = chart;
     const isCached = queryResponse && queryResponse.is_cached;
     const cachedDttm = queryResponse && queryResponse.cached_dttm;
+    const overflowable = slice && slice.viz_type === 'filter_box';
 
     return (
-      <div className="dashboard-chart">
+      <div className={cx('dashboard-chart', overflowable && 'dashboard-chart--overflowable')}>
         <SliceHeader
           innerRef={this.setHeaderRef}
           slice={slice}
diff --git a/superset/assets/javascripts/dashboard/v2/stylesheets/components/chart.less b/superset/assets/javascripts/dashboard/v2/stylesheets/components/chart.less
index 8419b48..80204fd 100644
--- a/superset/assets/javascripts/dashboard/v2/stylesheets/components/chart.less
+++ b/superset/assets/javascripts/dashboard/v2/stylesheets/components/chart.less
@@ -7,12 +7,38 @@
   overflow: hidden;
 }
 
+.dashboard-chart {
+  overflow: hidden;
+}
+
+.dashboard-chart.dashboard-chart--overflowable {
+  overflow: visible;
+}
+
 .dashboard-v2--editing .dashboard-component-chart {
   border: 1px solid transparent;
 }
 
-.dashboard-v2--editing .dashboard-component-chart:hover {
-  border: 1px solid @indicator-color;
+.dashboard-v2--editing .dashboard-component-chart:after {
+  content: "";
+  position: absolute;
+  width: 100%;
+  height: 100%;
+  top: 0px;
+  left: 0px;
+  z-index: 1;
+  pointer-events: none;
+  border: 1px solid transparent;
+}
+
+
+.dashboard-v2--editing .resizable-container:hover > .dashboard-component-chart:after,
+.dashboard-v2--editing .dashboard-component-chart:hover:after {
+  border: 1.4px solid @gray;
+}
+
+.dashboard-v2--editing .resizable-container.resizable-container--resizing:hover > .dashboard-component-chart:after {
+  border: 1.4px solid @indicator-color;
 }
 
 .dashboard-v2--editing .dashboard-component-chart .dashboard-chart .chart-container {
@@ -24,12 +50,13 @@
   opacity: 1;
 }
 
-
 .dashboard-v2--editing .dashboard-component-chart .dashboard-chart .slice_container {
   /* disable chart interactions in edit mode */
   pointer-events: none;
 }
 
 .chart-header {
-  padding: 16px;
+  font-size: 16px;
+  font-weight: bold;
+  padding: 16px 0 0 16px;
 }
diff --git a/superset/assets/javascripts/dashboard/v2/util/charts/getFormDataWithExtraFilters.js b/superset/assets/javascripts/dashboard/v2/util/charts/getFormDataWithExtraFilters.js
index ebb66e3..c43afbf 100644
--- a/superset/assets/javascripts/dashboard/v2/util/charts/getFormDataWithExtraFilters.js
+++ b/superset/assets/javascripts/dashboard/v2/util/charts/getFormDataWithExtraFilters.js
@@ -2,8 +2,9 @@ import getEffectiveExtraFilters from './getEffectiveExtraFilters';
 
 // We cache formData objects so that our connected container components don't always trigger
 // render cascades. we cannot leverage the reselect library because our cache size is >1
-let cachedMetadata = null;
-let cachedFormdata = {};
+const cachedDashboardMetadataByChart = {};
+const cachedFiltersByChart = {};
+const cachedFormdataByChart = {};
 
 export default function getFormDataWithExtraFilters({
   chart,
@@ -11,13 +12,15 @@ export default function getFormDataWithExtraFilters({
   filters,
   sliceId,
 }) {
-  // dashboard metadata has not changed use cache if possible
-  if (cachedMetadata === dashboardMetadata && cachedFormdata[sliceId]) {
-    return cachedFormdata[sliceId];
-  } else if (cachedMetadata !== dashboardMetadata) {
-    // changes to dashboardMetadata should invalidate all caches
-    cachedMetadata = dashboardMetadata;
-    cachedFormdata = {};
+  // if dashboard metadata + filters have not changed, use cache if possible
+  if (
+    cachedDashboardMetadataByChart[sliceId] &&
+    cachedDashboardMetadataByChart[sliceId] === dashboardMetadata &&
+    cachedFiltersByChart[sliceId] &&
+    cachedFiltersByChart[sliceId] === filters &&
+    cachedFormdataByChart[sliceId]
+  ) {
+    return cachedFormdataByChart[sliceId];
   }
 
   const extraFilters = getEffectiveExtraFilters({
@@ -34,7 +37,9 @@ export default function getFormDataWithExtraFilters({
     ],
   };
 
-  cachedFormdata[sliceId] = formData;
+  cachedDashboardMetadataByChart[sliceId] = dashboardMetadata;
+  cachedFiltersByChart[sliceId] = filters;
+  cachedFormdataByChart[sliceId] = formData;
 
   return formData;
 }
diff --git a/superset/assets/stylesheets/dashboard.less b/superset/assets/stylesheets/dashboard.less
index f9c9b3d..ce69ac7 100644
--- a/superset/assets/stylesheets/dashboard.less
+++ b/superset/assets/stylesheets/dashboard.less
@@ -1,35 +1,11 @@
 @import "./less/cosmo/variables.less";
 
-.dashboard a i {
-  cursor: pointer;
-}
-.dashboard i.drag {
-  cursor: move !important;
-}
-.dashboard .slice-grid .preview-holder {
-  z-index: 1;
-  position: absolute;
-  background-color: #AAA;
-  border-color: #AAA;
-  opacity: 0.3;
-}
-.dashboard .widget {
-  position: absolute;
-  top: 16px;
-  left: 16px;
-  box-shadow: none;
-  background-color: transparent;
-  overflow: visible;
-
-  &.is-edit {
-    //opacity: 0.45;
-  }
-}
 .dashboard .chart-header {
+  position: relative;
+
   .dropdown.btn-group {
     position: absolute;
-    top: 0;
-    right: 0;
+    right: 16px;
   }
 
   .dropdown-menu.dropdown-menu-right {
@@ -73,27 +49,6 @@
   background-color: #fff;
 }
 
-.dashboard .slice-grid .dragging,
-.dashboard .slice-grid .resizing {
-  opacity: 0.5;
-}
-.dashboard img.loading {
-  width: 20px;
-  margin: 5px;
-  position: absolute;
-}
-
-.dashboard .slice_title {
-  text-align: center;
-  font-weight: bold;
-  font-size: 14px;
-  padding: 5px;
-}
-.dashboard div.slice_content {
-  width: 100%;
-  height: 100%;
-}
-
 .modal img.loading {
   width: 50px;
   margin: 0;
@@ -109,27 +64,6 @@
   display: none;
 }
 
-.slice-cell {
-  box-shadow: 0px 0px 20px 5px rgba(0,0,0,0);
-  transition: box-shadow 1s ease-in;
-
-  .dropdown,
-  .dropdown-menu {
-    .fa {
-      font-size: 14px;
-    }
-  }
-}
-
-.slice-cell-highlight {
-  box-shadow: 0px 0px 20px 5px rgba(0,0,0,0.2);
-  height: 100%;
-}
-
-.dashboard-chart .editable-title input[type="button"] {
-  font-weight: bold;
-}
-
 .dashboard .separator.widget .slice_container {
   padding: 0;
   overflow: visible;
@@ -138,9 +72,7 @@
   margin-top: 5px;
   margin-bottom: 5px;
 }
-.chart-container {
-  box-sizing: border-box;
-}
+
 .separator .chart-container {
   position: absolute;
   left: 0;
@@ -159,10 +91,6 @@
   top: -5px;
 }
 
-.chart-header .header {
-  font-size: 16px;
-  margin: 0 -10px;
-}
 .ace_gutter {
     z-index: 0;
 }
diff --git a/superset/assets/visualizations/nvd3_vis.css b/superset/assets/visualizations/nvd3_vis.css
index 6b3b25d..f7539e1 100644
--- a/superset/assets/visualizations/nvd3_vis.css
+++ b/superset/assets/visualizations/nvd3_vis.css
@@ -11,6 +11,10 @@ text.nv-axislabel {
   font-size: 14px;
 }
 
+.slice_container.dist_bar {
+  overflow-x: auto !important;
+}
+
 .dist_bar svg.nvd3-svg {
   width: auto;
   font-size: 14px;

-- 
To stop receiving notification emails like this one, please contact
ccwilliams@apache.org.