Skip to content
This repository has been archived by the owner on Jun 25, 2020. It is now read-only.

feat: update line chart tooltip and use selector #31

Merged
merged 1 commit into from
Apr 1, 2019
Merged

feat: update line chart tooltip and use selector #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/superset-ui-plugins-demo/storybook/stories/preset-chart-xy/Line/stories/missing.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const missingData = {
keys: data.keys,
values: data.values.map(({ y, ...rest }) => ({
...rest,
y: Math.random() < 0.05 ? null : y,
y: Math.random() < 0.25 ? null : y,
})),
};

Expand Down
23 changes: 14 additions & 9 deletions packages/superset-ui-plugins-demo/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,28 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this file not generated by beemo?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... true. The one in demo is somehow not generated and is also checked in.

"compilerOptions": {
"allowJs": true,
"allowSyntheticDefaultImports": true,
"baseUrl": ".",
"outDir": "./dist",
"module": "commonjs",
"target": "es5",
"lib": ["es6", "dom"],
"sourceMap": true,
"allowJs": true,
"esModuleInterop": true,
"forceConsistentCasingInFileNames": true,
"importHelpers": true,
"jsx": "react",
"lib": ["dom", "esnext"],
"module": "commonjs",
"moduleResolution": "node",
"forceConsistentCasingInFileNames": true,
"noEmitOnError": true,
"noImplicitReturns": true,
"noImplicitThis": true,
"noImplicitAny": true,
"importHelpers": true,
"noUnusedLocals": true,
"pretty": true,
"removeComments": false,
"strictNullChecks": true,
"suppressImplicitAnyIndexErrors": true,
"noUnusedLocals": true,
"skipLibCheck": true
"skipLibCheck": true,
"sourceMap": true,
"target": "esnext"
},
"include": ["./storybook/stories/**/*", "../superset-ui-*/src/**/*", "./src/**/*", "./spec/**/*"]
}
19 changes: 15 additions & 4 deletions packages/superset-ui-preset-chart-xy/src/Line/Line.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import { chartTheme, ChartTheme } from '@data-ui/theme';
import { Margin, Dimension } from '@superset-ui/dimension';
import { groupBy, flatMap, uniqueId, values } from 'lodash';
import { createSelector } from 'reselect';
import createTooltip from './createTooltip';
import XYChartLayout from '../utils/XYChartLayout';
import WithLegend from '../components/WithLegend';
Expand Down Expand Up @@ -59,13 +60,22 @@ class LineChart extends PureComponent<Props> {

constructor(props: Props) {
super(props);
const { encoding } = this.props;

this.encoder = new Encoder({ encoding });
const createEncoder = createSelector(
(enc: Encoding) => enc,
(enc: Encoding) => new Encoder({ encoding: enc }),
);

this.createEncoder = () => {
this.encoder = createEncoder(this.props.encoding);
};

this.encoder = createEncoder(this.props.encoding);
this.renderChart = this.renderChart.bind(this);
}

encoder: Encoder;
private createEncoder: () => void;

renderChart(dim: Dimension) {
const { width, height } = dim;
Expand Down Expand Up @@ -113,6 +123,7 @@ class LineChart extends PureComponent<Props> {
/>,
<AreaSeries
key={`${series.key}-fill`}
seriesKey={series.key}
data={series.values}
interpolation="linear"
fill={`url(#${gradientId})`}
Expand Down Expand Up @@ -202,9 +213,9 @@ class LineChart extends PureComponent<Props> {
}

render() {
const { className, data, width, height, encoding } = this.props;
const { className, data, width, height } = this.props;

this.encoder = new Encoder({ encoding });
this.createEncoder();
const renderLegend = this.encoder.hasLegend()
? // eslint-disable-next-line react/jsx-props-no-multi-spaces
() => <ChartLegend<ChannelTypes, Outputs, Encoding> data={data} encoder={this.encoder} />
Expand Down
25 changes: 19 additions & 6 deletions packages/superset-ui-preset-chart-xy/src/Line/createTooltip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import TooltipTable from '../components/tooltip/TooltipTable';
import { Series, SeriesValue } from './Line';
import Encoder from './Encoder';

const MARK_STYLE = { marginRight: 4 };

export default function createTooltip(encoder: Encoder, allSeries: Series[]) {
function LineTooltip({
datum,
Expand All @@ -31,13 +33,24 @@ export default function createTooltip(encoder: Encoder, allSeries: Series[]) {
.filter(({ key }) => series[key])
.concat()
.sort((a, b) => series[b.key].y - series[a.key].y)
.map(({ key, color }) => ({
.map(({ key, color, strokeDasharray }) => ({
key,
keyStyle: {
color,
fontWeight: series[key] === datum ? 600 : 200,
},
value: encoder.channels.y.formatValue(series[key].y),
keyColumn: (
<>
<svg width="12" height="8" style={MARK_STYLE}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this glyph not match the legend? that seems ideal. it's tricky with the dashed line, tho. In that case I wonder if we should update the legend to be lines instead of circles or squares. wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps should update the legend to accept mark type and display line as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making the legend customizable seems like a good chunk of work itself. I will make it a follow-up PR.

<line
x2="12"
y1="3"
y2="3"
stroke={color}
strokeWidth="2"
strokeDasharray={strokeDasharray}
/>
</svg>
{series[key] === datum ? <b>{key}</b> : key}
</>
),
valueColumn: encoder.channels.y.formatValue(series[key].y),
}))}
/>
)}
Expand Down
13 changes: 7 additions & 6 deletions packages/superset-ui-preset-chart-xy/src/components/tooltip/TooltipTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React, { CSSProperties, PureComponent } from 'react';
import React, { CSSProperties, PureComponent, ReactNode } from 'react';

type Props = {
className?: string;
data: {
key: string;
key: string | number;
keyColumn: ReactNode;
keyStyle?: CSSProperties;
value: string | number;
valueColumn: ReactNode;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there cases where this is not string/number/null? ReactNode seems strange to have inside an array of data objects.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to be flexible in case extra formatting is needed or there are unforeseen thing we want to put in the value column. Also to make it same with keyColumn

valueStyle?: CSSProperties;
}[];
};
Expand All @@ -24,11 +25,11 @@ export default class TooltipTable extends PureComponent<Props, {}> {
return (
<table className={className}>
<tbody>
{data.map(({ key, keyStyle, value, valueStyle }) => (
{data.map(({ key, keyColumn, keyStyle, valueColumn, valueStyle }, i) => (
<tr key={key}>
<td style={keyStyle}>{key}</td>
<td style={keyStyle}>{keyColumn}</td>
<td style={valueStyle ? { ...VALUE_CELL_STYLE, ...valueStyle } : VALUE_CELL_STYLE}>
{value}
{valueColumn}
</td>
</tr>
))}
Expand Down