-
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(legend/click): add click interations on legend titles #51
feat(legend/click): add click interations on legend titles #51
Conversation
Previously, LineGeometry and AreaGeometry did not have specId and seriesKey but rather had those properties within each of their points. This meant that there wasn't a way to use specId and seriesKey on the whole geometry, but required accessing a point within the geometry.
f7f8d93
to
34d2a46
Compare
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've just done a quick manual test and I've found the following problem on the Interactions click/hovers on legend items [line chart]
.
If you click the eye on the last line series, everything is fine, you can turn on and off that easily.
If you click one of the others, some when hiding, some when showing, the chart will crash with the following error:
Uncaught TypeError: Cannot read property 'push' of undefined
at konva.js:1436
at Array.forEach (<anonymous>)
at konva.js:1435
at Array.forEach (<anonymous>)
at createInterpolation (konva.js:1434)
at Function.create (konva.js:250)
at AnimatedInterpolation._this.updateConfig (konva.js:325)
at konva.js:842
at Array.reduce (<anonymous>)
at Controller.update (konva.js:809)
The above error occurred in the <Spring> component:
in Spring (created by LineGeometries)
in Group (created by LineGeometries)
in Group (created by LineGeometries)
in LineGeometries (created by ReactiveChart)
in Layer (created by ReactiveChart)
34d2a46
to
58f7e3d
Compare
@markov00 Regarding the line charts, the issue was with the animation of the curved line paths, not the hide/show logic itself (this is also why the last line didn't have issues–because it wasn't a curved line path). I opened up a separate issue (#89) to look at this and for now instead of animating the line path, we use opacity, so the chart at least will render and the app doesn't crash for the visibility toggling. |
# [2.1.0](v2.0.0...v2.1.0) (2019-03-06) ### Features * **legend/click:** add click interations on legend titles ([#51](#51)) ([7d6139d](7d6139d))
🎉 This PR is included in version 2.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [2.1.0](elastic/elastic-charts@v2.0.0...v2.1.0) (2019-03-06) ### Features * **legend/click:** add click interations on legend titles ([opensearch-project#51](elastic/elastic-charts#51)) ([6d756e4](elastic/elastic-charts@6d756e4))
close #23
close #25
addresses #39 (not closing this as this PR doesn't introduce the props at the spec level, but does provide some underlying architecture that we can use)
Summary
This PR attaches click listeners passed in from Settings to the legend titles and also attaches some functionality for chart internal interactions. The UI is not finalized as this will require more feedback from design, but have opened up a new issue (#79) to track that feedback (and keep separate from functionality implementation).
Update series color using color picker:
Toggle series visibility (currently on icon click):
Toggle single series visibility (currently on icon shift-click):
Open context menu with plus/minus buttons whose handlers will pass data to any attached listeners:
Selected data series resets when specs are updated:
[instead of this which was happening in the first pass of implementation]
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.