Beta XYChart Component #309

mattapperson · 2018-01-16T22:11:54Z

Resolves #536

Items out of scope of current PR

Legend (too many unknowns)
- Color picker in legend, toggle series hide/show
- EUILegend Component exported
- Ability to pass custom legend
- Pass in legend props directly to chart
- For no data provide interaction option (show error, or not be selectable at all) Allow users to configure fitting/filling function for missing/null buckets kibana#17717
- Legends need to group data not grouped in the series

Docs:

add demo for adding data (@mattapperson)
Add high density demo using over 200 data points (@mattapperson)
Add beta flag to docs (@mattapperson)
Add notes to all prop types (@mattapperson)

Nice to haves

Add a stream series component
Determine long-term a11y approach
support aria description and/or described by (@chandlerprall)
auto-color series based on EUI color options
add empty state
add tests for user interactions (@chandlerprall)
add a DefaultAxis component (@markov00)

Overall todo

General series todo

is clickable option to each data point (@markov00 )
add clickable prop
Fix hover when series data on x axis are not identical
Add stacked series

Chart component todo list

fix crosshair displaying (sometimes) outside of chart
add show axis (x and y) prop
add axis position (x and y) prop
add mode prop (liner, time...)

mattapperson · 2018-05-10T18:13:27Z

Jenkins test this

sorenlouv · 2018-05-11T15:50:32Z

Nice work on this @chandlerprall and @mattapperson ! I've been busy with APM stuff this week - will try to make time next week to take a stab at this.

markov00 · 2018-05-31T11:41:28Z

@mattapperson I think we need to rewrite the EUIBarSeries component and split it into 4 different components to correctly handle either bar charts and histograms, in vertical or horizontal mode.

From a DataViz perspective there is a difference between bar charts and histograms:
bar charts are used for discrete/categorical visualization of bars where histograms are used for continuous data inside an interval.
This leads to:

different axis visualization for the two categories:
- bar charts have a discrete x-axis and show 1 tick per x value.
- histograms have a continuous x-axis and can display intermediate ticks between values
different bar width dimensions:
- bar charts have fixed width with padding between bars and aligned with their tick value
- histograms usually have the width depending on their bin dimension, but small-to-zero padding is involved and bars are aligned between their bin range.

I'm looking through the Kibana issues but seems that no one noticed that creating a bar histogram like the following shows misunderstanding ticks in the x-axis (seems that the ranges are -500 bytes to 500 bytes, 500b to 1500b and so on, when the correct values are 0 - 1000, 1000 - 2000) (@timroes @ppisljar)

react-vis has two main type of bars: Bars and Rects, bars for bar chart and rects for histograms (they also specify the needs to use Rects for histograms instead of bars).
Each abstract component is split into Vertical and Horizontal.

I think we can extend them having 4 EUIBars components or find a way for one configurable component.

What do you think?

mattapperson · 2018-05-31T12:13:18Z

@markov00 it seems to me it’s not about splitting the bar series, but simply adding 3 more?

markov00 · 2018-05-31T13:06:14Z

@mattapperson yes 3 more component and renaming the current one respecting its "vertical nature". So something like EUIVerticalBar, EUIHorizontalBar, EUIVerticalRect, EUIHorizontalRect

mattapperson · 2018-05-31T13:07:10Z

@markov00 Then yes, I fully support this

markov00 · 2018-05-31T13:11:51Z

Ok I will create a PR with these 4 components. I think we can also take this circumstance to fix exports and components naming (currently they are named EUIBarSeries exported as EuiBar)
@mattapperson Do you think we need to add a "chart" prefix?

mattapperson · 2018-05-31T13:15:26Z

@markov00 That would be great! it was on my todo list for tomorrow :)

markov00 · 2018-06-19T12:22:51Z

@chandlerprall @mattapperson I've updated my PR here. I think it's in a good shape and it's time to review it a bit before merging.
As already specified with @mattapperson the EUI charts can be flagged as beta, some of the API may change a bit in the future (like align props naming, merge few components into simplified ones etc.).
I also need to add tests on most of the components because I prefer to have a good shape of the API/Components before start working with tests.

sorenlouv · 2018-07-03T18:54:12Z

package.json

@@ -47,6 +47,8 @@
    "react-datepicker": "v1.4.1",
    "react-input-autosize": "^2.2.1",
    "react-virtualized": "^9.18.5",
+    "react-vis": "^1.9.3",


1.10.1 was released yesterday. Should we upgrade?

Yes, strange that the package.json is not updated. I've upgraded react-vis and it's reflected into the yarn.lock.
@mattapperson could you please check and upgrade correctly?

mattapperson · 2018-07-06T11:30:16Z

@sqren @markov00 @snide Ready for final PR review

mattapperson · 2018-07-06T11:30:41Z

I pulled Marko's latest updates and it LGTM

cchaos · 2018-07-06T13:21:28Z

We'll do a separate design PR. @chandlerprall Can you take a quick review of this?

chandlerprall

Needs an entry to CHANGELOG.md , otherwise code and docs LGTM

Add a more generic BarSeries and HistogramSeries, controlled by XYChart orientation prop. Fixed Examples to use this convention instead of single Vertical/Horizontal components. Add the default margin to XYChart.

markov00 · 2018-07-06T20:38:58Z

@chandlerprall I've updated the CHANGELOG. After the CI pass I will merge this ok?

sorenlouv · 2018-08-15T15:43:56Z

src/components/xy_chart/axis/_grids.scss

+  stroke-opacity: 0.3;
+  // support a clean mouse over when grids is above elements
+  pointer-events: none;
+}


@mattapperson @markov00 This rule caused some (minor) issues for the APM graphs so @formgeist overwrote this in:
elastic/kibana#21723

Not sure if the css is scoped by plugin or will affect all of kibana. Anyway, it seems suboptimal to overwrite styles like this. Any ideas for a better solution?

Yeah, these kinds of styles should be scoped with some .eui--- classname. We haven't yet done a design audit of the new chart component. Sorry for the override, we'll make sure not to have generic library selectors after the audit.

No worries. Just an fyi.
And sounds good with no generic library selectors.

mattapperson added the work in progress label Jan 17, 2018

mattapperson requested review from bevacqua and removed request for bevacqua January 18, 2018 17:22

This was referenced Mar 9, 2018

[WIP] Incorporate point series design guidelines elastic/kibana#13847

Closed

Visualize base histogram styling elastic/kibana#13335

Closed

timroes requested review from timroes and ppisljar March 20, 2018 14:31

sorenlouv reviewed Jul 3, 2018

View reviewed changes

mattapperson force-pushed the chart branch from f380125 to 09554fd Compare July 5, 2018 15:56

mattapperson changed the title ~~WIP chart component~~ Beta XYChart Component Jul 6, 2018

mattapperson requested review from snide and removed request for timroes and ppisljar July 6, 2018 11:29

chandlerprall suggested changes Jul 6, 2018

View reviewed changes

Matt Apperson added 4 commits July 6, 2018 22:36

WIP chart

b6ac55e

fix bug with tooltip when only one series skips data points

1654195

finish commit

4f24d16

update new path

f00be1f

markov00 and others added 23 commits July 6, 2018 22:36

Add crosshair tests

155c2db

Add basic test for selection brush

04cbc0c

Partial fix brush compute values on ordinal scales

cd3e044

Updated react-vis to 1.10.0. Moved react-vis scss to eui scss.

6b909b4

Extracted basic styling to scss classes

074d4fa

Fix some inverted examples

6ce7d75

Refactoring and abstracting BarSeries and HistogramSeries

bf0db79

Add a more generic BarSeries and HistogramSeries, controlled by XYChart orientation prop. Fixed Examples to use this convention instead of single Vertical/Horizontal components. Add the default margin to XYChart.

Fix typo prop in XYChart

265f0e7

Hide orientation props from bars and histograms

101db8d

LineSeries default to sparkline

9aec73c

Mark as experimental horizontal charts and orientation props

5017295

Add Range area chart example

8efb8f3

Remove use of orientation specific BarSeries on examples

c3797d0

Rename tspanWrapper into labelWordWrap

6e1f842

Add colors props type

f1fb5fe

Add color type prop test

ca9987f

Update tests with props

8d579af

Add onValueClick props to histogram

c203549

Add series names to crosshair

fe05969

last min version bump and add meta warnings, also linting fixes

0055aed

Fix failing tests on charts after upgrading react-vis 1.10.1

3b47859

Fix example typos

dc0b1d0

Updated changelog

60da33e

markov00 force-pushed the chart branch from 772c799 to 60da33e Compare July 6, 2018 20:38

markov00 removed the work in progress label Jul 6, 2018

mattapperson merged commit 5bef1c7 into elastic:master Jul 6, 2018

cchaos mentioned this pull request Jul 10, 2018

Quick fix to title sizing #997

Merged

sorenlouv reviewed Aug 15, 2018

View reviewed changes

sorenlouv mentioned this pull request Aug 17, 2018

EUI affecting react-vis styles #1121

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Beta XYChart Component #309

Beta XYChart Component #309

mattapperson commented Jan 16, 2018 •

edited

Loading

mattapperson commented May 10, 2018

sorenlouv commented May 11, 2018

markov00 commented May 31, 2018 •

edited

Loading

mattapperson commented May 31, 2018

markov00 commented May 31, 2018

mattapperson commented May 31, 2018

markov00 commented May 31, 2018 •

edited

Loading

mattapperson commented May 31, 2018

markov00 commented Jun 19, 2018

sorenlouv Jul 3, 2018

markov00 Jul 3, 2018

mattapperson commented Jul 6, 2018

mattapperson commented Jul 6, 2018

cchaos commented Jul 6, 2018

chandlerprall left a comment

markov00 commented Jul 6, 2018

sorenlouv Aug 15, 2018 •

edited

Loading

cchaos Aug 15, 2018

sorenlouv Aug 15, 2018

Beta XYChart Component #309

Beta XYChart Component #309

Conversation

mattapperson commented Jan 16, 2018 • edited Loading

mattapperson commented May 10, 2018

sorenlouv commented May 11, 2018

markov00 commented May 31, 2018 • edited Loading

mattapperson commented May 31, 2018

markov00 commented May 31, 2018

mattapperson commented May 31, 2018

markov00 commented May 31, 2018 • edited Loading

mattapperson commented May 31, 2018

markov00 commented Jun 19, 2018

sorenlouv Jul 3, 2018

Choose a reason for hiding this comment

markov00 Jul 3, 2018

Choose a reason for hiding this comment

mattapperson commented Jul 6, 2018

mattapperson commented Jul 6, 2018

cchaos commented Jul 6, 2018

chandlerprall left a comment

Choose a reason for hiding this comment

markov00 commented Jul 6, 2018

sorenlouv Aug 15, 2018 • edited Loading

Choose a reason for hiding this comment

cchaos Aug 15, 2018

Choose a reason for hiding this comment

sorenlouv Aug 15, 2018

Choose a reason for hiding this comment

mattapperson commented Jan 16, 2018 •

edited

Loading

markov00 commented May 31, 2018 •

edited

Loading

markov00 commented May 31, 2018 •

edited

Loading

sorenlouv Aug 15, 2018 •

edited

Loading