fix: disable x filling for line and areas#833
Conversation
This fix removes the needs of filling the missing x values on line and area charts in the following case: non-stacked line or area chart, with continuous scale and no fit function. BREAKING CHANGE: On non-stacked line or area charts, with a continuous x scale and no fit function, the line between consecutive points, independently from the other data series, will be a continuous connecting line. fix elastic#825
nickofthyme
left a comment
nickofthyme
left a comment
There was a problem hiding this comment.
LGTM, path ordering story is still good.
|
|
||
| function isXFillNotRequired(spec: BasicSeriesSpec, groupScaleType: ScaleType, isStacked: boolean) { | ||
| const onlyNoFitAreaLine = (isAreaSeriesSpec(spec) || isLineSeriesSpec(spec)) && !spec.fit; | ||
| const onlyContinuous = groupScaleType === ScaleType.Linear || groupScaleType === ScaleType.Time; |
There was a problem hiding this comment.
How does this affect Ordinal scales? Do we not need to fit with Ordinal scales?
There was a problem hiding this comment.
with Ordinal scales it should work as before. the line, usually indicates a continuity on the data, using ordinal aka categorical scale the continuity is not the primary thing.
Let's stick with that for the moment, if there is the case to link discontinued points also on categorical scales we can fix that later, what do you think?
There was a problem hiding this comment.
Yeah that's fine. The fitting function logic does account for ordinal values, I was just wondering if this eliminated that functionality. Looks like it is still working as it was....
|
It looks like the lastValue was affected to pull last value for each series not necessarily the last x domain value. I think this could be really confusing if they are all from different x values.
|
nickofthyme
left a comment
nickofthyme
left a comment
There was a problem hiding this comment.
Code changes LGTM. Verified fixes issue.
# [23.0.0](v22.0.0...v23.0.0) (2020-09-30) ### Bug Fixes * render continuous line/area between non-adjacent points ([#833](#833)) ([9f9892b](9f9892b)), closes [#825](#825) ### Features * debug state flag added to chart status ([#834](#834)) ([83919ff](83919ff)) * expose datum as part of GeometryValue ([#822](#822)) ([dcd7077](dcd7077)) ### BREAKING CHANGES * when rendering non-stacked line/area charts with a continuous x scale and no fit function, the line/area between non-consecutive data points will be rendered as a continuous line/area without adding an uncertain dashed line/ semi-transparent area that connects the two, non-adjacent, points.
|
🎉 This PR is included in version 23.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [23.0.0](elastic/elastic-charts@v22.0.0...v23.0.0) (2020-09-30) ### Bug Fixes * render continuous line/area between non-adjacent points ([opensearch-project#833](elastic/elastic-charts#833)) ([5222c40](elastic/elastic-charts@5222c40)), closes [opensearch-project#825](elastic/elastic-charts#825) ### Features * debug state flag added to chart status ([opensearch-project#834](elastic/elastic-charts#834)) ([f3aba25](elastic/elastic-charts@f3aba25)) * expose datum as part of GeometryValue ([opensearch-project#822](elastic/elastic-charts#822)) ([e582bd6](elastic/elastic-charts@e582bd6)) ### BREAKING CHANGES * when rendering non-stacked line/area charts with a continuous x scale and no fit function, the line/area between non-consecutive data points will be rendered as a continuous line/area without adding an uncertain dashed line/ semi-transparent area that connects the two, non-adjacent, points.
2 participants
Summary
This fix removes the needs of filling the missing x values on line and area charts in the following
case: non-stacked line or area chart, with continuous scale and no fit function.
BREAKING CHANGE: On non-stacked line or area charts, with a continuous x scale and no fit function,
the line between consecutive points, independently from the other data series, will be a continuous
connecting line.
fix #825
Before the fix
After the fix
To Do
Checklist
Delete any items that are not applicable to this PR.
src/index.ts(and stories only import from../srcexcept for test data & storybook)