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: remove series with undefined splitSeriesAccessor values #627

Conversation

markov00
Copy link
Member

Summary

This PR removes every series generated from a partial datum where some or all the values accessed by the splitSeriesAccessors array are null or undefined.

fix #594

Checklist

Delete any items that are not applicable to this PR.

  • Unit tests were updated or added to match the most common scenarios

@markov00 markov00 added bug Something isn't working :specs Chart specifications related issue labels Apr 10, 2020
@markov00 markov00 requested a review from nickofthyme April 10, 2020 14:06
@codecov-io
Copy link

codecov-io commented Apr 10, 2020

Codecov Report

Merging #627 into master will increase coverage by 0.00%.
The diff coverage is 29.93%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #627   +/-   ##
=======================================
  Coverage   67.78%   67.79%           
=======================================
  Files         236      236           
  Lines        6914     6916    +2     
  Branches     1275     1276    +1     
=======================================
+ Hits         4687     4689    +2     
  Misses       2208     2208           
  Partials       19       19           
Impacted Files Coverage Δ
src/chart_types/index.ts 100.00% <ø> (ø)
src/mocks/specs/specs.ts 76.19% <ø> (ø)
...art_types/goal_chart/layout/viewmodel/viewmodel.ts 3.12% <3.12%> (ø)
...pes/goal_chart/renderer/canvas/canvas_renderers.ts 9.58% <9.58%> (ø)
...al_chart/state/selectors/on_element_over_caller.ts 18.42% <18.42%> (ø)
..._types/goal_chart/state/selectors/picked_shapes.ts 23.52% <23.52%> (ø)
...l_chart/state/selectors/on_element_click_caller.ts 29.62% <29.62%> (ø)
...oal_chart/state/selectors/on_element_out_caller.ts 31.81% <31.81%> (ø)
src/state/chart_state.ts 84.21% <33.33%> (ø)
...goal_chart/renderer/canvas/connected_component.tsx 34.42% <34.42%> (ø)
... and 14 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 5669178...50c20d1. Read the comment docs.

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.

Code changes look good. Tested storybook and looks good.

Would we ever want in the future to enable this functionality under other if a prop were passed, say includeMissingSplitGroups or something?

nickofthyme and others added 2 commits April 10, 2020 18:37
- change logic to exclude only if no splits are found but accessor is defined
- remove all unnecessary split series accessors, per the new constraint above
- add tooltip vrt for multple split and y accessors
- remove tooltip visibility on 2y2g story
- update necessary screenshots
@markov00 markov00 merged this pull request into elastic:master Apr 14, 2020
markov00 added a commit that referenced this pull request Apr 14, 2020
This commit removes every series generated from a partial datum where all the values accessed by the splitSeriesAccessors array are null or undefined.

Co-authored-by: nickofthyme <nick.ryan.partridge@gmail.com>
markov00 pushed a commit that referenced this pull request Apr 15, 2020
# [18.3.0](v18.2.2...v18.3.0) (2020-04-15)

### Bug Fixes

* remove series with undefined splitSeriesAccessor values ([#627](#627)) ([59f0f6e](59f0f6e))

### Features

* gauge, goal and bullet graph (alpha) ([#614](#614)) ([5669178](5669178))
* **partition:** add legend and highlighters ([#616](#616)) ([6a4247e](6a4247e)), closes [#486](#486) [#532](#532)
@markov00 markov00 deleted the 2020_04_10-fix_undefined_split_series_accessors branch November 25, 2020 11:47
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working :specs Chart specifications related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ignore series created with undefined/null splitAccessors
3 participants