feat: improve line chart margin/axis and add buildquery (#66)

* fix: fallback to default margin when margin is partially set * feat: can disable axis title * feat: adjust margin according to axis title visibility * feat: include margin in formData * feat: add buildQuery * fix: address kyle comments * fix: remove string false
apache-superset · Apr 18, 2019 · 9c9dd9d · 9c9dd9d
1 parent eb9ffb5
commit 9c9dd9d
Show file tree

Hide file tree

Showing 12 changed files with 96 additions and 65 deletions.
diff --git a/package.json b/package.json
@@ -39,7 +39,7 @@
   ],
   "license": "Apache-2.0",
   "devDependencies": {
-    "@superset-ui/build-config": "^0.0.8",
+    "@superset-ui/build-config": "^0.0.9",
     "@superset-ui/commit-config": "^0.0.9",
     "@superset-ui/chart": "^0.11.3",
     "@superset-ui/color": "^0.11.3",

diff --git a/packages/superset-ui-plugins-demo/.storybook/addons.js b/packages/superset-ui-plugins-demo/.storybook/addons.js
@@ -1,5 +1,5 @@
 // note that the import order here determines the order in the UI!
+import '@storybook/addon-knobs/register';
 import '@storybook/addon-actions/register';
 import '@storybook/addon-links/register';
-import '@storybook/addon-knobs/register';
 import 'storybook-addon-jsx/register';
diff --git a/packages/superset-ui-plugins-demo/package.json b/packages/superset-ui-plugins-demo/package.json
@@ -31,13 +31,13 @@
   "dependencies": {
     "@babel/polyfill": "^7.4.3",
     "@data-ui/event-flow": "^0.0.54",
-    "@storybook/addon-actions": "^5.0.6",
-    "@storybook/addon-knobs": "^5.0.6",
-    "@storybook/addon-links": "^5.0.6",
-    "@storybook/addons": "^5.0.6",
-    "@storybook/react": "^5.0.6",
+    "@storybook/addon-actions": "^5.0.9",
+    "@storybook/addon-knobs": "^5.0.9",
+    "@storybook/addon-links": "^5.0.9",
+    "@storybook/addons": "^5.0.9",
+    "@storybook/react": "^5.0.9",
     "@types/react": "^16.8.8",
-    "@types/storybook__addon-knobs": "^4.0.4",
+    "@types/storybook__addon-knobs": "^4.0.5",
     "bootstrap": "^4.3.1",
     "react": "^16.6.0",
     "storybook-addon-jsx": "^7.1.0"

diff --git a/...es/preset-chart-xy/Line/stories/basic.jsx → ...es/preset-chart-xy/Line/stories/basic.tsx b/...es/preset-chart-xy/Line/stories/basic.jsx → ...es/preset-chart-xy/Line/stories/basic.tsx
@@ -24,8 +24,12 @@ export default [
                     type: 'time',
                   },
                   axis: {
-                    orient: radios('x.axis.orient', ['top', 'bottom'], 'bottom'),
-                    title: 'Time',
+                    orient: radios('x.axis.orient', { top: 'top', bottom: 'bottom' }, 'bottom'),
+                    title: radios(
+                      'x.axis.title',
+                      { enable: 'Time', disable: '', '': undefined },
+                      'Time',
+                    ),
                   },
                 },
                 y: {
@@ -35,8 +39,16 @@ export default [
                     type: 'linear',
                   },
                   axis: {
-                    orient: radios('y.axis.orient', ['left', 'right'], 'left'),
-                    title: 'Score',
+                    orient: radios(
+                      'y.axis.orient',
+                      { left: 'left', right: 'right', '': undefined },
+                      'left',
+                    ),
+                    title: radios(
+                      'y.axis.title',
+                      { enable: 'Score', disable: '', '': undefined },
+                      'Score',
+                    ),
                   },
                 },
                 color: {

diff --git a/packages/superset-ui-preset-chart-xy/src/Line/ChartFormData.ts b/packages/superset-ui-preset-chart-xy/src/Line/ChartFormData.ts
@@ -0,0 +1,10 @@
+import { ChartFormData } from '@superset-ui/chart';
+import { Margin } from '@superset-ui/dimension';
+import { Encoding } from './Encoder';
+
+type LineFormData = ChartFormData & {
+  encoding: Encoding;
+  margin?: Margin;
+};
+
+export default LineFormData;
diff --git a/packages/superset-ui-preset-chart-xy/src/Line/Line.tsx b/packages/superset-ui-preset-chart-xy/src/Line/Line.tsx
@@ -22,9 +22,11 @@ import ChartLegend from '../components/ChartLegend';
 
 chartTheme.gridStyles.stroke = '#f1f3f5';
 
+const DEFAULT_MARGIN = { top: 20, right: 20, left: 20, bottom: 20 };
+
 const defaultProps = {
   className: '',
-  margin: { top: 20, right: 20, left: 20, bottom: 20 },
+  margin: DEFAULT_MARGIN,
   theme: chartTheme,
 };
 
@@ -153,7 +155,7 @@ class LineChart extends PureComponent<Props> {
     const layout = new XYChartLayout({
       width,
       height,
-      margin,
+      margin: { ...DEFAULT_MARGIN, ...margin },
       theme,
       xEncoder: this.encoder.channels.x,
       yEncoder: this.encoder.channels.y,

diff --git a/packages/superset-ui-preset-chart-xy/src/Line/buildQuery.ts b/packages/superset-ui-preset-chart-xy/src/Line/buildQuery.ts
@@ -0,0 +1,6 @@
+import { buildQueryContext } from '@superset-ui/chart';
+import ChartFormData from './ChartFormData';
+
+export default function buildQuery(formData: ChartFormData) {
+  return buildQueryContext(formData);
+}
diff --git a/packages/superset-ui-preset-chart-xy/src/Line/index.ts b/packages/superset-ui-preset-chart-xy/src/Line/index.ts
@@ -1,10 +1,13 @@
 import { ChartPlugin } from '@superset-ui/chart';
 import metadata from './metadata';
 import transformProps from './transformProps';
+import buildQuery from './buildQuery';
+import ChartFormData from './ChartFormData';
 
-export default class LineChartPlugin extends ChartPlugin {
+export default class LineChartPlugin extends ChartPlugin<ChartFormData> {
   constructor() {
     super({
+      buildQuery,
       loadChart: () => import('./Line'),
       metadata,
       transformProps,

diff --git a/packages/superset-ui-preset-chart-xy/src/Line/transformProps.ts b/packages/superset-ui-preset-chart-xy/src/Line/transformProps.ts
@@ -4,13 +4,14 @@ import { ChartProps } from '@superset-ui/chart';
 
 export default function transformProps(chartProps: ChartProps) {
   const { width, height, formData, payload } = chartProps;
-  const { encoding } = formData;
+  const { encoding, margin } = formData;
   const { data } = payload;
 
   return {
     data,
     width,
     height,
     encoding,
+    margin,
   };
 }
diff --git a/packages/superset-ui-preset-chart-xy/src/encodeable/AxisAgent.ts b/packages/superset-ui-preset-chart-xy/src/encodeable/AxisAgent.ts
@@ -54,8 +54,20 @@ export default class AxisAgent<Def extends ChannelDef<Output>, Output extends Va
     return this.format || this.channelEncoder.formatValue;
   }
 
+  hasTitle() {
+    return this.getTitle() !== '';
+  }
+
   getTitle() {
-    return this.config.title || this.channelEncoder.getTitle();
+    const { title } = this.config;
+
+    if (title === undefined || title === true) {
+      return this.channelEncoder.getTitle();
+    } else if (title === false || title === '') {
+      return '';
+    }
+
+    return title;
   }
 
   getTickLabels() {
@@ -79,16 +91,17 @@ export default class AxisAgent<Def extends ChannelDef<Output>, Output extends Va
     return [];
   }
 
+  // eslint-disable-next-line complexity
   computeLayout({
-    axisLabelHeight = 20,
+    axisTitleHeight = 20,
     axisWidth,
     gapBetweenAxisLabelAndBorder = 8,
     gapBetweenTickAndTickLabel = 4,
     labelAngle = this.config.labelAngle,
     tickLength,
     tickTextStyle,
   }: {
-    axisLabelHeight?: number;
+    axisTitleHeight?: number;
     axisWidth: number;
     gapBetweenAxisLabelAndBorder?: number;
     gapBetweenTickAndTickLabel?: number;
@@ -110,68 +123,52 @@ export default class AxisAgent<Def extends ChannelDef<Output>, Output extends Va
     const maxWidth = Math.max(...labelDimensions.map(d => d.width));
 
     // TODO: Add other strategies: stagger, chop, wrap.
-    let strategy = labelOverlap;
-    if (strategy === 'auto') {
+    let strategyForLabelOverlap = labelOverlap;
+    if (strategyForLabelOverlap === 'auto') {
       // cheap heuristic, can improve
       const widthPerTick = axisWidth / tickLabels.length;
       if (this.channelEncoder.isY() || maxWidth <= widthPerTick) {
-        strategy = 'flat';
+        strategyForLabelOverlap = 'flat';
       } else {
-        strategy = 'rotate';
+        strategyForLabelOverlap = 'rotate';
       }
     }
 
+    const spaceForAxisTitle = this.hasTitle() ? labelPadding + axisTitleHeight : 0;
+    let tickTextAnchor;
+    let labelOffset: number = 0;
+    let requiredMargin =
+      tickLength + gapBetweenTickAndTickLabel + spaceForAxisTitle + gapBetweenAxisLabelAndBorder;
+
     if (this.channelEncoder.isX()) {
-      let labelOffset = 0;
-      let layout: {
-        labelAngle: number;
-        tickTextAnchor?: string;
-      } = { labelAngle };
-
-      if (strategy === 'flat') {
-        labelOffset = labelDimensions[0].height + labelPadding;
-        layout = { labelAngle: 0 };
-      } else if (strategy === 'rotate') {
+      if (strategyForLabelOverlap === 'flat') {
+        const labelHeight = labelDimensions[0].height;
+        labelOffset = labelHeight + labelPadding;
+        requiredMargin += labelHeight;
+      } else if (strategyForLabelOverlap === 'rotate') {
         const labelHeight = Math.ceil(Math.abs(maxWidth * Math.sin((labelAngle * Math.PI) / 180)));
         labelOffset = labelHeight + labelPadding;
-        layout = {
-          labelAngle,
-          tickTextAnchor:
-            (orient === 'top' && labelAngle > 0) || (orient === 'bottom' && labelAngle < 0)
-              ? 'end'
-              : 'start',
-        };
+        requiredMargin += labelHeight;
+        tickTextAnchor =
+          (orient === 'top' && labelAngle > 0) || (orient === 'bottom' && labelAngle < 0)
+            ? 'end'
+            : 'start';
       }
-
-      return {
-        ...layout,
-        labelOffset,
-        labelOverlap: strategy,
-        minMargin: {
-          [orient]: Math.ceil(
-            tickLength +
-              gapBetweenTickAndTickLabel +
-              labelOffset +
-              axisLabelHeight +
-              gapBetweenAxisLabelAndBorder +
-              8,
-          ),
-        },
-        orient,
-      };
+      requiredMargin += 8;
+    } else {
+      labelOffset = maxWidth + spaceForAxisTitle;
+      requiredMargin += maxWidth;
     }
 
-    const labelOffset = Math.ceil(maxWidth + labelPadding + axisLabelHeight);
-
     return {
-      labelAngle,
+      labelAngle: strategyForLabelOverlap === 'flat' ? 0 : labelAngle,
       labelOffset,
-      labelOverlap,
+      labelOverlap: strategyForLabelOverlap,
       minMargin: {
-        [orient]:
-          tickLength + gapBetweenTickAndTickLabel + labelOffset + gapBetweenAxisLabelAndBorder,
+        [orient]: Math.ceil(requiredMargin),
       },
       orient,
+      tickTextAnchor,
     };
   }
 }
diff --git a/packages/superset-ui-preset-chart-xy/src/encodeable/ChannelEncoder.ts b/packages/superset-ui-preset-chart-xy/src/encodeable/ChannelEncoder.ts
@@ -93,7 +93,7 @@ export default class ChannelEncoder<Def extends ChannelDef<Output>, Output exten
       return this.definition.title || this.definition.field;
     }
 
-    return undefined;
+    return '';
   }
 
   hasLegend() {

diff --git a/packages/superset-ui-preset-chart-xy/src/encodeable/types/Axis.ts b/packages/superset-ui-preset-chart-xy/src/encodeable/types/Axis.ts
@@ -11,7 +11,7 @@ export interface CoreAxis {
   labelPadding: number;
   orient: AxisOrient;
   tickCount: number;
-  title?: string;
+  title?: string | boolean;
   /** Explicitly set the visible axis tick values. */
   values?: string[] | number[] | boolean[] | DateTime[];
 }