Refine PlotlyChart component; move stylesheets to visualization's folder

getredash · Sep 10, 2019 · ee47078 · ee47078
1 parent 73c2731
commit ee47078
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 44 deletions.
diff --git a/client/app/assets/less/main.less b/client/app/assets/less/main.less
@@ -56,7 +56,6 @@
 @import 'inc/visualizations/sankey';
 @import 'inc/visualizations/pivot-table';
 @import 'inc/visualizations/map';
-@import 'inc/visualizations/chart';
 @import 'inc/visualizations/sunburst';
 @import 'inc/visualizations/cohort';
 @import 'inc/visualizations/misc';

diff --git a/client/app/pages/dashboards/dashboard.less b/client/app/pages/dashboards/dashboard.less
@@ -80,7 +80,7 @@
     .map-visualization-container,
     .word-cloud-visualization-container,
     .box-plot-deprecated-visualization-container,
-    .plotly-chart-container {
+    .chart-visualization-container {
       position: absolute;
       left: 0;
       top: 0;

diff --git a/client/app/visualizations/EditVisualizationDialog.jsx b/client/app/visualizations/EditVisualizationDialog.jsx
@@ -191,6 +191,7 @@ function EditVisualizationDialog({ dialog, visualization, query, queryResult })
               options={options}
               visualizationName={name}
               onOptionsChange={onOptionsChanged}
+              context="query"
             />
           </div>
         </Grid.Col>

diff --git a/client/app/visualizations/chart/Renderer/PlotlyChart.jsx b/client/app/visualizations/chart/Renderer/PlotlyChart.jsx
@@ -1,43 +1,22 @@
 import { isArray, isObject } from 'lodash';
-import React, { useState, useEffect, useMemo } from 'react';
+import React, { useState, useEffect } from 'react';
 import { RendererPropTypes } from '@/visualizations';
 import resizeObserver from '@/services/resizeObserver';
 
 import getChartData from '../getChartData';
 import { Plotly, prepareData, prepareLayout, updateData, applyLayoutFixes } from '../plotly';
 
-export default function PlotlyChart(props) {
+export default function PlotlyChart({ options, data }) {
   const [container, setContainer] = useState(null);
 
-  const options = useMemo(() => {
-    const result = { ...props.options };
-    if (['normal', 'percent'].indexOf(result.series.stacking) >= 0) {
-      // Backward compatibility
-      result.series = {
-        ...result.series,
-        percentValues: result.series.stacking === 'percent',
-        stacking: 'stack',
-      };
-    }
-    return result;
-  }, [props.options]);
-
-  const plotlyData = useMemo(() => {
-    const series = getChartData(props.data.rows, options);
-    return prepareData(series, options);
-  }, [props.data, options]);
-
-  const plotlyLayout = useMemo(() => {
-    if (container) {
-      return prepareLayout(container, options, plotlyData);
-    }
-    return null;
-  }, [container, options, plotlyData]);
-
   useEffect(() => {
-    if (container && plotlyLayout) {
+    if (container) {
       const plotlyOptions = { showLink: false, displaylogo: false };
 
+      const chartData = getChartData(data.rows, options);
+      const plotlyData = prepareData(chartData, options);
+      const plotlyLayout = prepareLayout(container, options, plotlyData);
+
       // It will auto-purge previous graph
       Plotly.newPlot(container, plotlyData, plotlyLayout, plotlyOptions).then(() => {
         applyLayoutFixes(container, plotlyLayout, (e, u) => Plotly.relayout(e, u));
@@ -51,19 +30,22 @@ export default function PlotlyChart(props) {
           Plotly.relayout(container, plotlyLayout);
         }
       });
-    }
-  }, [options, plotlyData, plotlyLayout, container]);
 
-  useEffect(() => {
-    if (container) {
       const unwatch = resizeObserver(container, () => {
         applyLayoutFixes(container, plotlyLayout, (e, u) => Plotly.relayout(e, u));
       });
       return unwatch;
     }
-  }, [plotlyLayout, container]);
+  }, [options, data, container]);
+
+  // Cleanup when component destroyed
+  useEffect(() => {
+    if (container) {
+      return () => Plotly.purge(container);
+    }
+  }, [container]);
 
-  return <div className="chart-visualization-container plotly-chart-container" ref={setContainer} />;
+  return <div className="chart-visualization-container" ref={setContainer} />;
 }
 
 PlotlyChart.propTypes = RendererPropTypes;
diff --git a/client/app/visualizations/chart/Renderer/index.jsx b/client/app/visualizations/chart/Renderer/index.jsx
@@ -4,6 +4,8 @@ import { RendererPropTypes } from '@/visualizations';
 import PlotlyChart from './PlotlyChart';
 import CustomPlotlyChart from './CustomPlotlyChart';
 
+import './renderer.less';
+
 export default function Renderer({ options, ...props }) {
   if (options.globalSeriesType === 'custom') {
     return <CustomPlotlyChart options={options} {...props} />;

diff --git a/...assets/less/inc/visualizations/chart.less → ...sualizations/chart/Renderer/renderer.less b/...assets/less/inc/visualizations/chart.less → ...sualizations/chart/Renderer/renderer.less
@@ -1,4 +1,4 @@
-.plotly-chart-container {
+.chart-visualization-container {
   height: 400px;
   overflow: hidden;
 }
diff --git a/client/app/visualizations/chart/index.js b/client/app/visualizations/chart/index.js
@@ -316,10 +316,20 @@ export default function init(ngModule) {
       type: 'CHART',
       name: 'Chart',
       isDefault: true,
-      getOptions: options => merge({}, DEFAULT_OPTIONS, {
-        showDataLabels: options.globalSeriesType === 'pie',
-        dateTimeFormat: clientConfig.dateTimeFormat,
-      }, options),
+      getOptions: (options) => {
+        const result = merge({}, DEFAULT_OPTIONS, {
+          showDataLabels: options.globalSeriesType === 'pie',
+          dateTimeFormat: clientConfig.dateTimeFormat,
+        }, options);
+
+        // Backward compatibility
+        if (['normal', 'percent'].indexOf(result.series.stacking) >= 0) {
+          result.series.percentValues = result.series.stacking === 'percent';
+          result.series.stacking = 'stack';
+        }
+
+        return result;
+      },
       Renderer,
       Editor: angular2react('chartEditor', ChartEditor, $injector),
 

diff --git a/client/app/visualizations/chart/plotly/index.js b/client/app/visualizations/chart/plotly/index.js
@@ -29,13 +29,13 @@ Plotly.setPlotConfig({
 
 const PlotlyChart = () => ({
   restrict: 'E',
-  template: '<div class="plotly-chart-container" resize-event="handleResize()"></div>',
+  template: '<div class="chart-visualization-container" resize-event="handleResize()"></div>',
   scope: {
     options: '=',
     series: '=',
   },
   link(scope, element) {
-    const plotlyElement = element[0].querySelector('.plotly-chart-container');
+    const plotlyElement = element[0].querySelector('.chart-visualization-container');
     const plotlyOptions = { showLink: false, displaylogo: false };
     let layout = {};
     let data = [];
@@ -85,7 +85,7 @@ const PlotlyChart = () => ({
 
 const CustomPlotlyChart = clientConfig => ({
   restrict: 'E',
-  template: '<div class="plotly-chart-container" resize-event="handleResize()"></div>',
+  template: '<div class="chart-visualization-container" resize-event="handleResize()"></div>',
   scope: {
     series: '=',
     options: '=',