feat: line chart with revised encodeable utilities #26

kristw · 2019-03-23T01:21:59Z

tl;dr. This PR add the first working version of the new Line Chart.

🌕Code Overview

This PR adds two new LineChart plugins under the preset-chart-xy package. Both plugins point to the same Chart component but has different ChartMetadata and transformProps. The legacy-compatible plugin will have the same ChartMetadata with the useLegacyApi field set to true. The transformProps is the key difference as the legacy-compatible plugin converts legacy formData to the new formData while the new plugin's transformProps does very little.

📈 Line Chart

In Line.tsx, which is the main chart component. Line uses XYChartLayout, which is a cross-chart utility, to compute necessary margins and labelling strategy for the axes. It also uses ChartLegend to display legend. In addition, it has two chart-specific subcomponents.

Encoder

Once the Encoder is initialized and passed in the actual encoding config, it can provide utility functions for getting value from datum, encoding values, as well as things we usually want from scale or axis.

Tooltip

This simply renders chart-specific tooltip.

📐 Chart Layout

A chart has multiple wrappers. At the top level <WithLegend /> attaches legend to one side of the chart, splitting the container into two parts: legend and ChartFrame. Legend will take only necessary space and give the rest to ChartFrame. The actual chart is contained within the ChartFrame and usually has the same size. However, in some case when the chart has to guarantee minimum width and/or height to be usable and the ChartFrame is too small for that, the chart will be given the specified minimum width and/or height, making the ChartFrame scrollable instead.

📘 Documentation

Update the storybook for line chart in packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy

Basic line chart
Legacy-compatible version
Line chart with missing data
Encoding dasharray and fill

🔮 Future Work

There are still work remaining after this PR. This PR is a major milestone but not completion.

Optimize to create Encoder only when necessary.
Make creating an Encoder class more convenient.
Merge computeXAxisLayout() and computeYAxisLayout() into AxisAgent.
Make AxisAgent derive config type (XAxis | YAxis) from channel type.
Handle data points surrounded by missing data.
Make storybook demo tsx work with knobs without complaining about types

williaster

Looking great overall, thanks for all the effort here! 💛 💜

packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy/Line/data/data.js

williaster · 2019-03-26T21:55:45Z

packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy/Line/stories/basic.jsx

+                  type: 'time',
+                },
+                axis: {
+                  orientation: 'bottom',


could abstract these into knobs for easy debugging and demo-ing what it does.

I converted it to tsx then add knobs. Then it starts complaining for missing types definition for external module. I renamed it back to jsx and will take care of the tsx and external type def issue in a follow-up pr.

packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy/Line/index.js

packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy/Line/stories/basic.jsx

packages/superset-ui-preset-chart-xy/src/Line/Line.tsx

packages/superset-ui-preset-chart-xy/src/utils/XYChartLayout.tsx

packages/superset-ui-preset-chart-xy/src/utils/createTickComponent.tsx

packages/superset-ui-preset-chart-xy/types/@vx/legend/index.d.ts

conglei

Awesome work!! Just left some comments on

Customizability issue'
the style Props and defaultProps.

Both are nit :)

packages/superset-ui-preset-chart-xy/src/Line/Line.tsx

conglei · 2019-03-28T17:54:47Z

packages/superset-ui-preset-chart-xy/src/Line/Line.tsx

+  parent: Series;
+}
+
+const CIRCLE_STYLE = { strokeWidth: 1.5 };


does it mean users cannot customize the style? If the chart is tailored specially for superset, I won't see it will be an issue. But just was thinking of the customization part.

This is only for the highlighted circle when using crosshair. May have to think more carefully about how to expose customization.

conglei · 2019-03-28T18:08:34Z

packages/superset-ui-preset-chart-xy/src/components/ChartLegend.tsx

+      return (
+        <div key={field} style={LEGEND_CONTAINER_STYLE}>
+          <LegendOrdinal scale={scale} labelFormat={channelEncoder.formatValue}>
+            {(labels: Label[]) => (


Have we done the test with extremely large number of legends here? So far, there are a lot of chart in Superset that has more than I'd say 20 legends. At least we need to make sure the chart can be rendered.

LEGEND_CONTAINER_STYLE caps the legend with maxHeight.

conglei · 2019-03-28T18:11:52Z

packages/superset-ui-preset-chart-xy/src/components/WithLegend.tsx

@@ -29,34 +11,32 @@ type Props = {
  legendJustifyContent: 'center' | 'flex-start' | 'flex-end';
  position: 'top' | 'left' | 'bottom' | 'right';
  renderChart: (dim: { width: number; height: number }) => ReactNode;
-  renderLegend: (params: { direction: string }) => ReactNode;
-  hideLegend: boolean;
+  renderLegend?: (params: { direction: string }) => ReactNode;


I saw we have two different ways to define Props with defaultProps. Can we make the styles consistent? I do like the way you defined before, with & typeof defaultProps.

Yes, fixed.

packages/superset-ui-preset-chart-xy/src/encodeable/parsers/extractScale.ts

* feat: line chart * feat: implement scale extraction * refactor: no error * fix: handle null * fix: nicing and demo * fix: legend and demo * fix: remove commented code * fix: clean * fix: reviewer comments * fix: legend and series * docs: make demos tsx * fix: reactnode * fix: label angle * fix: resolve labelxxx field names * docs: try knobs * feat: improve axis * refactor: combine computelayout into axisagent * refactor: cleaner use of scale * fix: proptypes

kristw added 3 commits March 22, 2019 11:41

feat: line chart

b39b3af

feat: implement scale extraction

275b32d

refactor: no error

6fc01b6

kristw requested a review from a team as a code owner March 23, 2019 01:21

kristw added 5 commits March 25, 2019 15:55

fix: handle null

d4303e1

fix: nicing and demo

745b52a

fix: legend and demo

3063165

fix: remove commented code

7c1e430

fix: clean

b87402f

kristw mentioned this pull request Mar 26, 2019

[WIP] new line chart #11

Closed

williaster reviewed Mar 26, 2019

View reviewed changes

kristw added 10 commits March 26, 2019 18:12

fix: reviewer comments

c8a6ff0

fix: legend and series

e86282b

docs: make demos tsx

7a9cc2b

fix: reactnode

5452b45

fix: label angle

34130f6

fix: resolve labelxxx field names

87c5f88

docs: try knobs

23a41af

feat: improve axis

a6116c3

refactor: combine computelayout into axisagent

e6e3eaa

refactor: cleaner use of scale

b21703f

conglei approved these changes Mar 28, 2019

View reviewed changes

fix: proptypes

848ecfd

kristw merged commit c9b8195 into master Mar 28, 2019

delete-merged-branch bot deleted the kristw--line3 branch March 28, 2019 23:58

nytai referenced this pull request in preset-io/superset-ui-plugins Apr 27, 2020

add CODEOWNERS (#26)

b495213

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: line chart with revised encodeable utilities #26

feat: line chart with revised encodeable utilities #26

kristw commented Mar 23, 2019 •

edited

Loading

williaster left a comment

williaster Mar 26, 2019

kristw Mar 27, 2019

conglei left a comment

conglei Mar 28, 2019

kristw Mar 28, 2019

conglei Mar 28, 2019

kristw Mar 28, 2019

conglei Mar 28, 2019

kristw Mar 28, 2019

feat: line chart with revised encodeable utilities #26

feat: line chart with revised encodeable utilities #26

Conversation

kristw commented Mar 23, 2019 • edited Loading

🌕Code Overview

📈 Line Chart

Encoder

Tooltip

📐 Chart Layout

📘 Documentation

🔮 Future Work

williaster left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conglei left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kristw commented Mar 23, 2019 •

edited

Loading