feat: more helpful error #410
Conversation
|
Deploy preview for superset-ui-plugins ready! Built with commit 7cc2ea2 |
Codecov Report
@@ Coverage Diff @@
## master #410 +/- ##
=========================================
- Coverage 2.83% 2.83% -0.01%
=========================================
Files 186 186
Lines 5821 5823 +2
Branches 373 374 +1
=========================================
Hits 165 165
- Misses 5631 5633 +2
Partials 25 25
Continue to review full report at Codecov.
|
| @@ -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.'); | |||
There was a problem hiding this comment.
Will it be possible to have one row even if group by is specified? And one record or no result is actually allowed?
(To be honest, I don't know what this chart is for).
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
| @@ -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.'); | |||
|
|
|||
| return ( | |||
| <div className={`superset-legacy-chart-horizon ${className}`} style={{ height }}> | |||
| {data.map(row => ( | |||
There was a problem hiding this comment.
Maybe it's better to add const groupKeys = Array.isArray(row.key) ? row.key : [row.key] here.
🏆 Enhancements
There are probably a thousand of these we could do, but I found an easy yet cryptic error to replace with a more helpful one. Let me know if anyone has a preferred approach to these, or if I'm overlooking anything with this one.
Before:
After:
