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

chore: preparation for spec rendering order #516

Merged
merged 3 commits into from
Feb 13, 2020

Conversation

markov00
Copy link
Member

@markov00 markov00 commented Jan 15, 2020

Summary

This PR is the first one to that allow us to implement a valid and clear method to sort the splitted series and allow custom rendering order of bars/area/lines in either chart, legend and tooltips.

This PR in particular clean up few small items:

  • include the parsing action that that is useful to distinguish in which state we are when updating a spec. This will also ensure that the default setting component is always part of the specs array
  • split the tooltipValues array into an header and an array with only the metric values.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

  • Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
  • This was checked for cross-browser compatibility, including a check against IE11
  • 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
  • Each commit follows the convention

@codecov-io
Copy link

codecov-io commented Jan 15, 2020

Codecov Report

Merging #516 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #516   +/-   ##
=======================================
  Coverage   71.49%   71.49%           
=======================================
  Files         208      208           
  Lines        6118     6118           
  Branches     1137     1169   +32     
=======================================
  Hits         4374     4374           
  Misses       1727     1727           
  Partials       17       17

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 8dede8c...c434a08. Read the comment docs.

@markov00 markov00 force-pushed the 2020-01-15_keep_spec_sort_order branch from ca33298 to 8dede8c Compare February 12, 2020 17:53
@markov00 markov00 changed the title fix: align sorting order of specs in legend, tooltip and renderer chore: preparation for spec rendering order Feb 12, 2020
@markov00 markov00 marked this pull request as ready for review February 12, 2020 18:12
@markov00 markov00 requested review from nickofthyme, rshen91 and 111andre111 and removed request for 111andre111 February 12, 2020 18:12
@markov00 markov00 added the chore label Feb 12, 2020
Copy link
Contributor

@rshen91 rshen91 left a comment

Choose a reason for hiding this comment

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

These changes LGTM - thanks for updating all the tests and including chart_state.specs.test.ts. I ran this storybook locally and all the stories appeared to be rendering correctly!

@markov00 markov00 merged commit f056b77 into elastic:master Feb 13, 2020
@markov00 markov00 deleted the 2020-01-15_keep_spec_sort_order branch February 13, 2020 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants