-
Notifications
You must be signed in to change notification settings - Fork 122
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
fix(rendering): fix rendering values <= 0 on log scale #114
Conversation
instead of throwing errors or warnings, if the user is using a log scale with a dataset with 0 values, we will hide them on the rendering of line and area charts. This fix also the defined area of the chart fix elastic#112, fix elastic#63
Codecov Report
@@ Coverage Diff @@
## master #114 +/- ##
==========================================
+ Coverage 90.17% 91.09% +0.91%
==========================================
Files 31 31
Lines 1405 1460 +55
Branches 150 163 +13
==========================================
+ Hits 1267 1330 +63
+ Misses 125 116 -9
- Partials 13 14 +1
Continue to review full report at Codecov.
|
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.
Tested locally on Firefox. Code LGTM just one small comment but non-blocking.
src/lib/series/rendering.ts
Outdated
@@ -263,10 +283,17 @@ export function renderArea( | |||
areaGeometry: AreaGeometry; | |||
indexedGeometries: Map<any, IndexedGeometry[]>; | |||
} { | |||
const isLogScale = yScale.type === ScaleType.Log; |
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.
Since this logic is repeated in a few places, could create a helper function isLogScale(type: ScaleType)
to ensure consistency.
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.
resolved in 790d5f7
currently passing a Scale object instead of a type as it's done for the isContinuousScale
or isOrdinalScale
🎉 This PR is included in version 3.4.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
## [3.4.2](elastic/elastic-charts@v3.4.1...v3.4.2) (2019-03-26) ### Bug Fixes * **rendering:** fix rendering values <= 0 on log scale ([opensearch-project#114](elastic/elastic-charts#114)) ([2b79877](elastic/elastic-charts@2b79877)), closes [opensearch-project#112](elastic/elastic-charts#112) [opensearch-project#63](elastic/elastic-charts#63)
Summary
Instead of throwing errors or warnings, if the user is using a log scale with a dataset with 0
values, we will hide them on the rendering of line and area charts.
This fix also the defined area of a line/area chart: if the y value is null, the area/line is depicted with an empty space between the previous non null value and the following one.
fix #112, fix #63
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.