feat(plugin-chart-echarts): arrange legend and zoom (apache#920)

* feat(plugin-chart-echarts): subject Arrange legend and zoom * fix * WIP * Fix * set zoomable false * fix tests * Solve comments * Add constants * fix * fix lint * fix lint * fix lint * snake_case to camelCase * remove unused property
zhaoyongjie · Nov 25, 2021 · 2679fae · 2679fae
1 parent 5a37ecb
commit 2679fae
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 15 deletions.
diff --git a/...ary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts b/...ary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts
@@ -34,7 +34,7 @@ import {
 } from '@superset-ui/core';
 import { EChartsOption, SeriesOption } from 'echarts';
 import { DEFAULT_FORM_DATA, EchartsTimeseriesFormData } from './types';
-import { EchartsProps, ForecastSeriesEnum, ProphetValue } from '../types';
+import { EchartsProps, ForecastSeriesEnum, ProphetValue, LegendOrientation } from '../types';
 import { parseYAxisBound } from '../utils/controls';
 import { extractTimeseriesSeries, getChartPadding, getLegendProps } from '../utils/series';
 import { extractAnnotationLabels } from '../utils/annotation';
@@ -52,6 +52,7 @@ import {
   transformSeries,
   transformTimeseriesAnnotation,
 } from './transformers';
+import { TIMESERIES_CONSTANTS } from '../constants';
 
 export default function transformProps(chartProps: ChartProps): EchartsProps {
   const { width, height, formData, queriesData } = chartProps;
@@ -143,10 +144,15 @@ export default function transformProps(chartProps: ChartProps): EchartsProps {
     grid: {
       ...defaultGrid,
       ...getChartPadding(showLegend, legendOrientation, legendMargin, {
-        top: 20,
-        bottom: zoomable ? 80 : 20,
-        left: 20,
-        right: 20,
+        top: TIMESERIES_CONSTANTS.gridOffsetTop,
+        bottom: zoomable
+          ? TIMESERIES_CONSTANTS.gridOffsetBottomZoomable
+          : TIMESERIES_CONSTANTS.gridOffsetBottom,
+        left: TIMESERIES_CONSTANTS.gridOffsetLeft,
+        right:
+          showLegend && legendOrientation === LegendOrientation.Right
+            ? 0
+            : TIMESERIES_CONSTANTS.gridOffsetRight,
       }),
     },
     xAxis: {
@@ -194,7 +200,7 @@ export default function transformProps(chartProps: ChartProps): EchartsProps {
       },
     },
     legend: {
-      ...getLegendProps(legendType, legendOrientation, showLegend),
+      ...getLegendProps(legendType, legendOrientation, showLegend, zoomable),
       // @ts-ignore
       data: rawSeries
         .filter(
@@ -208,6 +214,8 @@ export default function transformProps(chartProps: ChartProps): EchartsProps {
     series,
     toolbox: {
       show: zoomable,
+      top: TIMESERIES_CONSTANTS.toolboxTop,
+      right: TIMESERIES_CONSTANTS.toolboxRight,
       feature: {
         dataZoom: {
           yAxisIndex: false,
@@ -222,9 +230,9 @@ export default function transformProps(chartProps: ChartProps): EchartsProps {
       ? [
           {
             type: 'slider',
-            start: 0,
-            end: 100,
-            bottom: 20,
+            start: TIMESERIES_CONSTANTS.dataZoomStart,
+            end: TIMESERIES_CONSTANTS.dataZoomEnd,
+            bottom: TIMESERIES_CONSTANTS.zoomBottom,
           },
         ]
       : [],

diff --git a/...-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/constants.ts b/...-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/constants.ts
@@ -18,3 +18,18 @@
  */
 // eslint-disable-next-line import/prefer-default-export
 export const NULL_STRING = '<NULL>';
+
+export const TIMESERIES_CONSTANTS = {
+  gridOffsetRight: 40,
+  gridOffsetLeft: 20,
+  gridOffsetTop: 20,
+  gridOffsetBottom: 20,
+  gridOffsetBottomZoomable: 80,
+  legendRightTopOffset: 30,
+  legendTopRightOffset: 55,
+  zoomBottom: 30,
+  toolboxTop: 0,
+  toolboxRight: 5,
+  dataZoomStart: 0,
+  dataZoomEnd: 100,
+};
diff --git a/...rset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/types.ts b/...rset-frontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/types.ts
@@ -64,7 +64,7 @@ export type ProphetValue = {
 
 export type EchartsLegendFormData = {
   legendMargin: number | null | string;
-  legendOrientation: LegendOrientation.Top;
+  legendOrientation: LegendOrientation;
   legendType: LegendType;
   showLegend: boolean;
 };

diff --git a/...ontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/utils/series.ts b/...ontend/temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/src/utils/series.ts
@@ -25,7 +25,7 @@ import {
   TimeseriesDataRecord,
 } from '@superset-ui/core';
 import { LegendComponentOption, SeriesOption } from 'echarts';
-import { NULL_STRING } from '../constants';
+import { NULL_STRING, TIMESERIES_CONSTANTS } from '../constants';
 import { LegendOrientation, LegendType } from '../types';
 import { defaultLegendPadding } from '../defaults';
 
@@ -93,6 +93,7 @@ export function getLegendProps(
   type: LegendType,
   orientation: LegendOrientation,
   show: boolean,
+  zoomable = false,
 ): LegendComponentOption | LegendComponentOption[] {
   const legend: LegendComponentOption | LegendComponentOption[] = {
     orient: [LegendOrientation.Top, LegendOrientation.Bottom].includes(orientation)
@@ -107,13 +108,15 @@ export function getLegendProps(
       break;
     case LegendOrientation.Right:
       legend.right = 0;
+      legend.top = zoomable ? TIMESERIES_CONSTANTS.legendRightTopOffset : 0;
       break;
     case LegendOrientation.Bottom:
       legend.bottom = 0;
       break;
     case LegendOrientation.Top:
     default:
       legend.top = 0;
+      legend.right = zoomable ? TIMESERIES_CONSTANTS.legendTopRightOffset : 0;
       break;
   }
   return legend;

diff --git a/.../temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/test/utils/series.test.ts b/.../temporary_superset_ui/superset-ui/plugins/plugin-chart-echarts/test/utils/series.test.ts
@@ -135,10 +135,21 @@ describe('formatSeriesName', () => {
   });
 
   describe('getLegendProps', () => {
-    it('should return the correct props for scroll type with top orientation', () => {
-      expect(getLegendProps(LegendType.Scroll, LegendOrientation.Top, true)).toEqual({
+    it('should return the correct props for scroll type with top orientation without zoom', () => {
+      expect(getLegendProps(LegendType.Scroll, LegendOrientation.Top, true, false)).toEqual({
         show: true,
         top: 0,
+        right: 0,
+        orient: 'horizontal',
+        type: 'scroll',
+      });
+    });
+
+    it('should return the correct props for scroll type with top orientation with zoom', () => {
+      expect(getLegendProps(LegendType.Scroll, LegendOrientation.Top, true, true)).toEqual({
+        show: true,
+        top: 0,
+        right: 55,
         orient: 'horizontal',
         type: 'scroll',
       });
@@ -153,10 +164,21 @@ describe('formatSeriesName', () => {
       });
     });
 
-    it('should return the correct props for plain type with right orientation', () => {
-      expect(getLegendProps(LegendType.Plain, LegendOrientation.Right, false)).toEqual({
+    it('should return the correct props for plain type with right orientation without zoom', () => {
+      expect(getLegendProps(LegendType.Plain, LegendOrientation.Right, false, false)).toEqual({
+        show: false,
+        right: 0,
+        top: 0,
+        orient: 'vertical',
+        type: 'plain',
+      });
+    });
+
+    it('should return the correct props for plain type with right orientation with zoom', () => {
+      expect(getLegendProps(LegendType.Plain, LegendOrientation.Right, false, true)).toEqual({
         show: false,
         right: 0,
+        top: 30,
         orient: 'vertical',
         type: 'plain',
       });