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

fix: render orphan data points on lines and areas #900

Merged
merged 3 commits into from
Nov 12, 2020

Conversation

markov00
Copy link
Member

@markov00 markov00 commented Nov 11, 2020

Summary

This PR adds the ability to render orhpan data points displayed on a line or area chart.
An orphan data point is a datapoint that doesn't have a next or a previous data point to connect to.
The possible cases are:

  • the first data-point on a series with a null, undefined consecutive datapoint
  • the last data-point of a series with a null, undefined previous datapoint
  • a data-point of a series with both previous and next points null.

If the series style is configured to show the point, it will not change the current rendering behaviour if, instead, the series style hides the point (point:{visible: false}) on the rendering phase we filter and render only the ophan data points.

Screenshot 2020-11-11 at 13 19 01

fix #783
close #63

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

@codecov-io
Copy link

codecov-io commented Nov 11, 2020

Codecov Report

Merging #900 (2ef5fb9) into master (9f8e66f) will increase coverage by 0.26%.
The diff coverage is 77.27%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #900      +/-   ##
==========================================
+ Coverage   69.65%   69.91%   +0.26%     
==========================================
  Files         340      356      +16     
  Lines       10950    10611     -339     
  Branches     2276     2147     -129     
==========================================
- Hits         7627     7419     -208     
+ Misses       3309     3110     -199     
- Partials       14       82      +68     
Flag Coverage Δ
unittests ?

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

Impacted Files Coverage Δ
src/chart_types/xy_chart/renderer/canvas/areas.ts 34.28% <0.00%> (+3.51%) ⬆️
src/chart_types/xy_chart/renderer/canvas/lines.ts 40.00% <0.00%> (ø)
src/mocks/geometries.ts 92.10% <ø> (ø)
src/utils/geometry.ts 100.00% <ø> (ø)
src/chart_types/xy_chart/renderer/canvas/bars.ts 95.23% <50.00%> (-4.77%) ⬇️
src/chart_types/xy_chart/rendering/points.ts 90.12% <95.00%> (+0.93%) ⬆️
...rc/chart_types/xy_chart/renderer/canvas/bubbles.ts 61.11% <100.00%> (-7.64%) ⬇️
src/chart_types/xy_chart/rendering/area.ts 91.17% <100.00%> (ø)
src/chart_types/xy_chart/rendering/line.ts 94.44% <100.00%> (-0.30%) ⬇️
src/chart_types/xy_chart/rendering/utils.ts 92.94% <100.00%> (-1.45%) ⬇️
... and 205 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 9f8e66f...2ef5fb9. Read the comment docs.

Copy link
Contributor

@monfera monfera left a comment

Choose a reason for hiding this comment

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

For aesthetic reasons and to reinforce the meaning of the marker circles, as well as emphasize the section starts and ends, maybe the section starts/ends could also have the marker.

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, just a few minor suggestions. 👍

src/chart_types/xy_chart/renderer/canvas/lines.ts Outdated Show resolved Hide resolved
src/chart_types/xy_chart/rendering/utils.ts Outdated Show resolved Hide resolved
src/chart_types/xy_chart/rendering/utils.ts Outdated Show resolved Hide resolved
src/chart_types/xy_chart/rendering/points.ts Show resolved Hide resolved
src/chart_types/xy_chart/rendering/points.ts Outdated Show resolved Hide resolved
src/chart_types/xy_chart/rendering/points.ts Outdated Show resolved Hide resolved
src/chart_types/xy_chart/rendering/utils.ts Outdated Show resolved Hide resolved
src/chart_types/xy_chart/rendering/points.ts Outdated Show resolved Hide resolved
@monfera monfera self-requested a review November 12, 2020 09:56
Copy link
Contributor

@monfera monfera left a comment

Choose a reason for hiding this comment

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

LGTM, achieves the objective, and the section start/end markers are beyond the scope of this PR

@markov00 markov00 added :chart Chart element related issue :xy Bar/Line/Area chart related enhancement New feature or request labels Nov 12, 2020
@markov00 markov00 changed the title feat: render orphan data points on lines and areas fix: render orphan data points on lines and areas Nov 12, 2020
@markov00 markov00 merged commit 0be282b into elastic:master Nov 12, 2020
@markov00 markov00 deleted the 2020_11_11-fix_orphan_data_points branch November 12, 2020 14:16
markov00 pushed a commit that referenced this pull request Nov 24, 2020
# [24.1.0](v24.0.0...v24.1.0) (2020-11-24)

### Bug Fixes

* **area_charts:** correctly represent baseline with negative data points ([#896](#896)) ([d1243f1](d1243f1))
* **legend:** legend sizes with ordinal data ([#867](#867)) ([7559e0d](7559e0d)), closes [#811](#811)
* render orphan data points on lines and areas ([#900](#900)) ([0be282b](0be282b)), closes [#783](#783)
* specs swaps correctly reflected in state ([#901](#901)) ([7fba882](7fba882))

### Features

* **legend:** allow legend text to be copyable ([#877](#877)) ([9cd3459](9cd3459)), closes [#710](#710)
* allow clearing series colors from memory ([#899](#899)) ([ab1af38](ab1af38))
* merge series domain with the domain of another group ([#912](#912)) ([325b013](325b013))
* small multiples for XY charts (alpha) ([#793](#793)) ([d288208](d288208)), closes [#500](#500) [#500](#500)
@markov00
Copy link
Member Author

🎉 This PR is included in version 24.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Nov 24, 2020
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
# [24.1.0](elastic/elastic-charts@v24.0.0...v24.1.0) (2020-11-24)

### Bug Fixes

* **area_charts:** correctly represent baseline with negative data points ([opensearch-project#896](elastic/elastic-charts#896)) ([b622fda](elastic/elastic-charts@b622fda))
* **legend:** legend sizes with ordinal data ([opensearch-project#867](elastic/elastic-charts#867)) ([74bcbad](elastic/elastic-charts@74bcbad)), closes [opensearch-project#811](elastic/elastic-charts#811)
* render orphan data points on lines and areas ([opensearch-project#900](elastic/elastic-charts#900)) ([3e2c739](elastic/elastic-charts@3e2c739)), closes [opensearch-project#783](elastic/elastic-charts#783)
* specs swaps correctly reflected in state ([opensearch-project#901](elastic/elastic-charts#901)) ([a94347f](elastic/elastic-charts@a94347f))

### Features

* **legend:** allow legend text to be copyable ([opensearch-project#877](elastic/elastic-charts#877)) ([21a96d3](elastic/elastic-charts@21a96d3)), closes [opensearch-project#710](elastic/elastic-charts#710)
* allow clearing series colors from memory ([opensearch-project#899](elastic/elastic-charts#899)) ([e97f4ab](elastic/elastic-charts@e97f4ab))
* merge series domain with the domain of another group ([opensearch-project#912](elastic/elastic-charts#912)) ([716ad5a](elastic/elastic-charts@716ad5a))
* small multiples for XY charts (alpha) ([opensearch-project#793](elastic/elastic-charts#793)) ([3b88e1c](elastic/elastic-charts@3b88e1c)), closes [opensearch-project#500](elastic/elastic-charts#500) [opensearch-project#500](elastic/elastic-charts#500)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:chart Chart element related issue enhancement New feature or request released Issue released publicly :xy Bar/Line/Area chart related
Projects
None yet
4 participants