-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
EUI Barchart and Histogram refactoring #2
Conversation
* Splitted into line, area, bar, histogram * Added new line chart examples
Reused as much as possible the existing charts implementation and existing event handling
This reverts commit 3340a43.
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.
title={title} | ||
position={titlePosition} | ||
orientation={orientation} | ||
on0={onZero} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be onZero
instead of on0
?
Btw. onZero
sounds more like an event handler than a boolean. Can't come up with a better name from the top of my head though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on0
is how react-vis call it. We changed to onZero
on our components. Maybe something like isOnZero
sounds more like a bool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed, isOnZero
is a better name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've given this a quick look. On the surface it looks good - I don't have any objections. if there is anything in particular you want me to look at, let me know. Else next steps would be to try it out, and see how it works for APM data.
Still LGTM |
The PR introduce the following components:
EuiBarSeries
for bar chartsEuiHistogramSeries
for histogramsEuiAreaSeries
for area chartsEuiLineSeries
for line chartsThe chart can be viewed in vertical (default) or horizontal mode.
A default set of axis is available, but developers can add their own configured axis.
Crosshair and Selection Brush tools are controlled and managed by XYChart.
General features
Charts to be refactored (WIP)
Show/Hide linein favour of scatterplotGeneral Todo:
Discuss:
Notes: The data processing pipeline will take care of process data in the correct format.
Some simple utility function can be built to support chart data format processing.
Notes: It will not be automatically configured to stack series together. The developer needs to know when and how to stack data