Skip to content
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: expose datum as part of GeometryValue #822

Merged

Conversation

markov00
Copy link
Member

Summary

This PR partially address the concerns expressed here: #808
Is currently not allow using an object as a value for X or Y coordinates, so we kindly suggest to add an additional field into your data set that allows us to uniquely identify that object with a string or a number.
As a workaround for a click handler, a way to get back the original object from the stringified/number version is proposed in this PR, exposing the original datum as part of the GeometryValue and as a consequence exposing the datum to every interaction handlers that use a GeometryValue

Checklist

  • Proper documentation or storybook story was added for features that require explanation or tutorials
  • Unit tests were updated or added to match the most common scenarios

@markov00 markov00 added enhancement New feature or request :interactions Interactions related issue :xy Bar/Line/Area chart related labels Sep 15, 2020
Copy link
Collaborator

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tested locally on chrome.

@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2020

Codecov Report

Merging #822 into master will increase coverage by 0.43%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #822      +/-   ##
==========================================
+ Coverage   74.09%   74.53%   +0.43%     
==========================================
  Files         277      292      +15     
  Lines        9440     9724     +284     
  Branches     2037     2079      +42     
==========================================
+ Hits         6995     7248     +253     
- Misses       2436     2461      +25     
- Partials        9       15       +6     
Flag Coverage Δ
#unittests 73.97% <ø> (-0.13%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/chart_types/xy_chart/rendering/rendering.ts 93.28% <ø> (ø)
src/mocks/geometries.ts 86.84% <ø> (ø)
src/utils/geometry.ts 100.00% <ø> (ø)
src/mocks/theme.ts 100.00% <0.00%> (ø)
src/mocks/series/series_identifiers.ts 100.00% <0.00%> (ø)
src/mocks/scale/scale.ts 77.77% <0.00%> (ø)
src/mocks/scale/index.ts 100.00% <0.00%> (ø)
src/mocks/specs/index.ts 100.00% <0.00%> (ø)
src/mocks/store/index.ts 100.00% <0.00%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 813aee1...d6eb483. Read the comment docs.

@markov00
Copy link
Member Author

Jenkins test this please

@markov00 markov00 merged commit dcd7077 into elastic:master Sep 29, 2020
@markov00 markov00 deleted the 2020_09_15-expose_datum_to_geometryvalue branch September 29, 2020 11:01
markov00 pushed a commit that referenced this pull request Sep 30, 2020
# [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.
@markov00
Copy link
Member Author

🎉 This PR is included in version 23.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Sep 30, 2020
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
# [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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request :interactions Interactions related issue released Issue released publicly :xy Bar/Line/Area chart related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants