-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
adding stepped line to line and area charts #9425
Conversation
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.
We probably would need a corresponding change in the asciidocs as well.
This is a neat little addition.
FWIW, I'd add a breaking label as well since the UI is different. I'd be good to add this to the release notes, and add the deprecation of the "smooth lines" parameter.
@@ -41,6 +40,16 @@ export default function HistogramVisType(Private) { | |||
value: 'bottom', | |||
text: 'bottom', | |||
}], | |||
interpolationModes: [{ | |||
value: 'linear', | |||
text: 'normal', |
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.
consider renaming to "straight". "normal" sounds funny.
@@ -4,15 +4,17 @@ export default function ColumnHandler(Private) { | |||
|
|||
const createSeries = (cfg, series) => { | |||
const stacked = ['stacked', 'percentage', 'wiggle', 'silhouette'].includes(cfg.mode); | |||
let interpolate = cfg.interpolate; | |||
if (cfg.smoothLines) interpolate = 'cardinal'; |
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'd add a comment here that this is for backward compatibility when loading URLs or configs.
9d20880
to
041f116
Compare
041f116
to
1a285e3
Compare
@ppisljar i like this change! very neat. @thomasneirynck why do you think this is a breaking change? I don't consider a change in UI a breaking change. As long as previous dashboard continue to work, this is just a change in visual builder and is appropriate for a minor release. @epixa ^^ would you agree? |
Generally, I agree with @tbragin on this one. I haven't pulled this down - what changed in the UI other than adding this option? |
@epixa |
@ppisljar Great, thanks. Not breaking |
There are a few references to smoothLines left, that should be removed https://github.com/elastic/kibana/blob/master/src/ui/public/vislib/visualizations/point_series/area_chart.js#L14 and https://github.com/elastic/kibana/blob/master/src/ui/public/vislib/visualizations/point_series/area_chart.js#L71 |
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.
Nice job, LGTM
Backports PR #9425 **Commit 1:** adding stepped line to line and area charts * Original sha: be913a4 * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-12-09T14:59:16Z **Commit 2:** fixing based on Thomas' review * Original sha: 1a285e3 * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-12-14T08:27:50Z **Commit 3:** updating docs * Original sha: 03a0e9f * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-12-19T11:04:04Z **Commit 4:** fixing based on Brandons review * Original sha: 2f86249 * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-12-23T15:04:34Z
Backports PR #9425 **Commit 1:** adding stepped line to line and area charts * Original sha: be913a4 * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-12-09T14:59:16Z **Commit 2:** fixing based on Thomas' review * Original sha: 1a285e3 * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-12-14T08:27:50Z **Commit 3:** updating docs * Original sha: 03a0e9f * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-12-19T11:04:04Z **Commit 4:** fixing based on Brandons review * Original sha: 2f86249 * Authored by ppisljar <peter.pisljar@gmail.com> on 2016-12-23T15:04:34Z
ads stepped line interpolation option to line and area charts
closes #1365