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: cleanup example prop default values #173

Merged
merged 5 commits into from
Apr 16, 2019

Conversation

markov00
Copy link
Member

@markov00 markov00 commented Apr 16, 2019

Summary

Because we started having some users of the library , it's better to start cleaning up a bit the default and the examples to avoid propagate unnecessary styles to new users.
This PR can partially solve the issues described here: #121

This PR cleans up a bit the current examples:

  • configure canvas as default renderer and remove the renderer props from examples
  • as yScaleToDataExtent is default to false, I've removed the unneccessary prop from example
  • removed the basic scale type from code. It was never used and can lead to possible misunderstandings
  • updated the overview docs to reflect the current specs interfaces

Checklist

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

  • 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

Because SVG renderer is not ready yet, this commit will set the rendering to canvas as default.
BasicSeries component and in general the basic series type was a generic type never used on code.
@markov00 markov00 requested a review from emmacunningham April 16, 2019 11:29
@markov00 markov00 changed the title Cleanup defaults fix: cleanup example prop default values Apr 16, 2019
@markov00 markov00 added the :specs Chart specifications related issue label Apr 16, 2019
@codecov-io
Copy link

codecov-io commented Apr 16, 2019

Codecov Report

Merging #173 into master will increase coverage by 0.36%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #173      +/-   ##
==========================================
+ Coverage   96.62%   96.99%   +0.36%     
==========================================
  Files          36       36              
  Lines        1867     1861       -6     
  Branches      251      250       -1     
==========================================
+ Hits         1804     1805       +1     
+ Misses         54       48       -6     
+ Partials        9        8       -1
Impacted Files Coverage Δ
src/lib/series/specs.ts 100% <ø> (ø) ⬆️
src/state/utils.ts 95.23% <100%> (+3.85%) ⬆️

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 b8df1f0...6a6817a. Read the comment docs.

Copy link
Contributor

@emmacunningham emmacunningham left a comment

Choose a reason for hiding this comment

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

LGTM

@markov00
Copy link
Member Author

jenkins, test this

@markov00 markov00 merged commit ab19df0 into elastic:master Apr 16, 2019
@markov00 markov00 deleted the default-renderer branch April 16, 2019 17:54
markov00 pushed a commit that referenced this pull request Apr 16, 2019
## [3.11.2](v3.11.1...v3.11.2) (2019-04-16)

### Bug Fixes

* cleanup example prop default values ([#173](#173)) ([ab19df0](ab19df0))
@markov00
Copy link
Member Author

🎉 This PR is included in version 3.11.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Apr 16, 2019
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
released Issue released publicly :specs Chart specifications related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants