-
Notifications
You must be signed in to change notification settings - Fork 120
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
feat(series): set custom series colors through spec prop #95
feat(series): set custom series colors through spec prop #95
Conversation
Codecov Report
@@ Coverage Diff @@
## master #95 +/- ##
==========================================
+ Coverage 88.07% 88.63% +0.56%
==========================================
Files 28 28
Lines 1132 1144 +12
Branches 118 118
==========================================
+ Hits 997 1014 +17
+ Misses 125 120 -5
Partials 10 10
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.
Code LGTM. Tested locally and works fine
One minor thing to notice: if, in the future we want to to have a sort of JSON schema for the spec, how do you think we can serialize Map<DataSeriesColorsValues, string>
@markov00 for serializing the
This still allows the user to define in terms of DataSeriesColorValues (instead of needing to know more about the chart internals and forming the string series key themselves) while providing a bit of type safety to how they are defining these so that we at least get both a specId and colorValues to uniquely identify the series. We would need to then change the way that updating the props works; because currently it is a map, we can just set the customSeriesColors map using the key when there is an update. Using this new data structure, we'd need to iterate through the array for the spec and then find if a config with a matching DataSeriesColorValues exists. Another option would be to have this prop be an object (one advantage is that the updating logic would remain about the same, unlike the option above):
where the seriesKey property is the stringified form of the DataSeriesColorValues and the value is string representing color. The drawback to this is that the user would need to be able to form the seriesKey, though we do have a function that they could call on DataSeriesColorValues to generate this string. |
# [3.1.0](v3.0.1...v3.1.0) (2019-03-11) ### Features * **series:** set custom series colors through spec prop ([#95](#95)) ([fb09dc9](fb09dc9))
🎉 This PR is included in version 3.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [3.1.0](elastic/elastic-charts@v3.0.1...v3.1.0) (2019-03-11) ### Features * **series:** set custom series colors through spec prop ([opensearch-project#95](elastic/elastic-charts#95)) ([581c97e](elastic/elastic-charts@581c97e))
Summary
close #39
This PR adds the ability to specify a custom series colors through a prop on the series spec.
Each series can now receive an optional prop
customSeriesColor: Map<DataSeriesColorValues, string>
, where the key is used to identify the series and the string is a representation of the color. Under theStylings
section of Storyboard, there is an example of this in action.The color picker in the legend is also still usable to customize a series color; on color picker update, the
customSeriesColor
prop for the corresponding series is updated.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.