feat: add histogram mode#218
Conversation
Codecov Report
@@ Coverage Diff @@
## master #218 +/- ##
==========================================
+ Coverage 97.82% 97.84% +0.02%
==========================================
Files 36 36
Lines 2571 2647 +76
Branches 566 590 +24
==========================================
+ Hits 2515 2590 +75
Misses 49 49
- Partials 7 8 +1
Continue to review full report at Codecov.
|
src/lib/axes/axis_utils.ts
Outdated
| axisScaleDomain: xDomain.domain, | ||
| axisScaleType: xDomain.scaleType, | ||
| axisScaleDomain: xDomain.domain, // is this used anymore? | ||
| axisScaleType: xDomain.scaleType, // is this used anymore? |
There was a problem hiding this comment.
I don't think these are used anymore (and they are being hard coded always for the xDomain), is it ok to remove these?
|
Notes from our sync via zoom:
|
|
@markov00 Have updated per our discussion on Zoom in response to the items you mentioned in review: (1) Line annotations will always align with the axis line and are unaffected by the HistogramModeAlignment: (2) Per our conversation, the default HistogramModeAlignment will be (3) The RectAnnotation rendering has been improved so that: in Histogram Mode, the RectAnnotation xDomain coordinates align with the axis, similar to LineAnnotation. when not in Histogram Mode, if there is a bar series, then regardless of the type of scale, the xDomain coordinates will snap to the nearest tick values (This is with the xDomain coordinates defined as when not in Histogram Mode, if there is only a line or bar series, then the xDomain coordinates will not snap and will align with the axis similar to how LineAnnotations always behave Lastly, the validation for annotations has changed so that if we are in histogram mode with an additional tick, then the validation expands the end of the domain to domainEnd + scale.minInterval (see the annotation from 3 to 4): |
markov00
left a comment
There was a problem hiding this comment.
@emmacunningham I've checked again that PR. Everything is fine! I want only to suggest to change the default bar padding for the histogram mode to something relatively small like 0.05 or less. You can maybe introduce a a new histogramPadding value on the style and use that instead of barPadding.
@markov00 👍 have added |
# [5.1.0](v5.0.0...v5.1.0) (2019-06-11) ### Features * add histogram mode ([#218](#218)) ([b418b67](b418b67))
|
🎉 This PR is included in version 5.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [5.1.0](elastic/elastic-charts@v5.0.0...v5.1.0) (2019-06-11) ### Features * add histogram mode ([opensearch-project#218](elastic/elastic-charts#218)) ([26b67c1](elastic/elastic-charts@26b67c1))





Summary
This PR introduces histogram mode which will ensure that bar series bars are aligned with ticks along the xDomain axis such that the tick is aligned with the starting edge of a bar instead of in the center as is the current only option for bar series bars.
This mode can be enabled in one of two ways:
enableHistogramModeprop on aBarSeriesSpecHistogramBarSeriesSpecwhich is identical to theBarSeriesSpecexcept that theenableHistogramModedoes not need to be specifiedIf a user enables histogram mode in either of these ways, any split or additional defined bar series will be rendered as stacked bars as histogram mode does not allow clustering of bars.
In the story below, we toggle

enableHistogramModeon individualBarSeriesSpecs (there are two distinct specs in the story) and if either one has it enabled, all series will stack; likewise, when checkinghasHistogramBarSerieswill add aHistogramBarSeriesSpec, causing all of the series to stack.Additional features
Things get a little bit weird in Histogram Mode when we add Line & Area series because the bucketed points need to align with something. We expose a prop (
histogramModeAlignmentwith optionsstart,center,end) onLineSeriesSpec,AreaSeriesSpec, andLineAnnotationSpecto allow the user to define how these points should align in Histogram Mode (this is not specified forRectAnnotationSpecbecauseRectAnnotationsoperate more like bars / bands so they are always aligned relative to starting edge of the histogram bars):We also add one more tick to the available ticks for the xAxis in histogram mode. This is because, when the ticks get shifted, we need to also mark the ending edge of the last bar in the series:
A note about implementation:
There are two ways to approach aligning the bars correctly:
(1) shift the position of the bars by
xScale.bandwidth / 2-xScale.band / 2so that the first bar aligns with the starting edge of the chart and the last bar does not overflow the ending edge of the chart. we also need to shift the bars highlighting byxScale.bandwidth / 2.(2) shift the position of the axis ticks by
- xScale.bandwidth / 2-xScale.bandwidth / 2(line and area) so their points align correctly with the starting edge of a bar instead of the center. bars highlighting does not need to be shifted in this case.The advantage of (1) is that it does not require touching axis tick positioning or other chart elements, which it might seem weird to adjust those positions for
histogramModewhich seems to be a very bar series specific concern. However (1) has a couple of disadvantages:- the first axis tick may slightly overflow the starting edge of the chart
- we need
xScale.bandwidthbefore it is available (xScalecomputation relies onchartDimensionsso on first render, we won't have an offset value)Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.src/index.ts(and stories only import from../srcexcept for test data & storybook)