From c196b413c84b38b99cfd00bb38b6d5837e29d474 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Thu, 18 Apr 2019 14:15:07 -0700 Subject: [PATCH] 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 --- .../plugins/superset-ui-plugins/package.json | 2 +- .../.storybook/addons.js | 2 +- .../superset-ui-plugins-demo/package.json | 12 +-- .../Line/stories/{basic.jsx => basic.tsx} | 20 +++- .../src/Line/ChartFormData.ts | 10 ++ .../src/Line/Line.tsx | 6 +- .../src/Line/buildQuery.ts | 6 ++ .../src/Line/index.ts | 5 +- .../src/Line/transformProps.ts | 3 +- .../src/encodeable/AxisAgent.ts | 91 +++++++++---------- .../src/encodeable/ChannelEncoder.ts | 2 +- .../src/encodeable/types/Axis.ts | 2 +- 12 files changed, 96 insertions(+), 65 deletions(-) rename superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy/Line/stories/{basic.jsx => basic.tsx} (67%) create mode 100644 superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/ChartFormData.ts create mode 100644 superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/buildQuery.ts diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/package.json b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/package.json index 9a1eb62b37503..4a6b4733984eb 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/package.json +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/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/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/.storybook/addons.js b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/.storybook/addons.js index 74d263f3d97ba..cc58570b9acbc 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/.storybook/addons.js +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/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/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/package.json b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/package.json index 94262853fd270..0e2fccc2edbbb 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/package.json +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/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/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy/Line/stories/basic.jsx b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy/Line/stories/basic.tsx similarity index 67% rename from superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy/Line/stories/basic.jsx rename to superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy/Line/stories/basic.tsx index a4194d9db1d68..d9d07c548d0aa 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy/Line/stories/basic.jsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-plugins-demo/storybook/stories/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/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/ChartFormData.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/ChartFormData.ts new file mode 100644 index 0000000000000..07b176ca1f0d0 --- /dev/null +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/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/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/Line.tsx b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/Line.tsx index 481d94a10b462..f19dd5a88eade 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/Line.tsx +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/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 { 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/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/buildQuery.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/buildQuery.ts new file mode 100644 index 0000000000000..312783e99d91c --- /dev/null +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/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/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/index.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/index.ts index 1acc16a916934..b5cad765d9ac9 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/index.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/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 { constructor() { super({ + buildQuery, loadChart: () => import('./Line'), metadata, transformProps, diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/transformProps.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/transformProps.ts index d75251efacd50..d9c6e4e3845ab 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/transformProps.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/Line/transformProps.ts @@ -4,7 +4,7 @@ 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 { @@ -12,5 +12,6 @@ export default function transformProps(chartProps: ChartProps) { width, height, encoding, + margin, }; } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/encodeable/AxisAgent.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/encodeable/AxisAgent.ts index 5610e44df621c..94c1737c307ea 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/encodeable/AxisAgent.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/encodeable/AxisAgent.ts @@ -54,8 +54,20 @@ export default class AxisAgent, 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,8 +91,9 @@ export default class AxisAgent, Output extends Va return []; } + // eslint-disable-next-line complexity computeLayout({ - axisLabelHeight = 20, + axisTitleHeight = 20, axisWidth, gapBetweenAxisLabelAndBorder = 8, gapBetweenTickAndTickLabel = 4, @@ -88,7 +101,7 @@ export default class AxisAgent, Output extends Va tickLength, tickTextStyle, }: { - axisLabelHeight?: number; + axisTitleHeight?: number; axisWidth: number; gapBetweenAxisLabelAndBorder?: number; gapBetweenTickAndTickLabel?: number; @@ -110,68 +123,52 @@ export default class AxisAgent, 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/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/encodeable/ChannelEncoder.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/encodeable/ChannelEncoder.ts index e20254d24e43a..fe2403beeb983 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/encodeable/ChannelEncoder.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/encodeable/ChannelEncoder.ts @@ -93,7 +93,7 @@ export default class ChannelEncoder, Output exten return this.definition.title || this.definition.field; } - return undefined; + return ''; } hasLegend() { diff --git a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/encodeable/types/Axis.ts b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/encodeable/types/Axis.ts index fbcf5f7cd5475..eca14c7421559 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/packages/superset-ui-preset-chart-xy/src/encodeable/types/Axis.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/plugins/superset-ui-plugins/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[]; }