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

feat: more helpful error #410

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: more helpful error #410

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1ae6bf
feat: more helpful error
rusackas Mar 24, 2020
7cc2ea2
fix: fixing linting error about throwing literlal errors
rusackas Mar 24, 2020
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
3 changes: 3 additions & 0 deletions packages/superset-ui-legacy-plugin-chart-horizon/src/HorizonChart.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ class HorizonChart extends React.PureComponent {
yDomain = d3Extent(allValues, d => d.y);
}

if (data.length <= 1)
throw new Error('Please select a "Group by" option to enable multiple rows.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Will it be possible to have one row even if group by is specified? And one record or no result is actually allowed?

image

(To be honest, I don't know what this chart is for).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree with @ktmud on supporting one row.

(To be honest, I don't know what this chart is for).

It is a technique for small multiples of time-series published in 2009, useful when each series has very different max value (e.g one has y = 0-10, another has y = 0-10000. If plot basic line chart with same max y (10000), the first time-series will look like a flat line. If each chart use its own max then 10 on the first chart will look as tall as 10000 on the second chart). Horizon chart was developed to address this situation.

Although less commonly used, since general audience does not know how to interpret it without training.

https://observablehq.com/@d3/horizon-chart

Original paper:
http://vis.berkeley.edu/papers/horizon/2009-TimeSeries-CHI.pdf

Copy link
Contributor

@ktmud ktmud Mar 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the explanation, @kristw ! I wonder if there's an opportunity to add a "Pro tip" section to the chart explorer where we display such explanation and tips for best practices (my example query above obviously doesn't make sense for this chart).

Copy link
Collaborator

@kristw kristw Mar 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Good idea on tips. Each chart has description field in metadata. We could write markdown and have it rendered as tips, or can also add new field to metadata. Can note this down for project ideas.


return (
<div className={`superset-legacy-chart-horizon ${className}`} style={{ height }}>
{data.map(row => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's better to add const groupKeys = Array.isArray(row.key) ? row.key : [row.key] here.

Expand Down