-
Notifications
You must be signed in to change notification settings - Fork 120
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(canvas_text_bbox_calculator): increase font scaling factor #93
Conversation
Codecov Report
@@ Coverage Diff @@
## master #93 +/- ##
==========================================
+ Coverage 88.07% 88.51% +0.44%
==========================================
Files 28 28
Lines 1132 1132
Branches 118 118
==========================================
+ Hits 997 1002 +5
+ Misses 125 120 -5
Partials 10 10
Continue to review full report at Codecov.
|
@markov00 Codecov was blocking merge because we didn't have any tests written for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, Code looks good to me and tested locally on chrome works fine.
I've also see that you already added the canvas mock and solved the testing so ok to merge
## [3.0.1](v3.0.0...v3.0.1) (2019-03-08) ### Bug Fixes * **canvas_text_bbox_calculator:** increase font scaling factor ([#93](#93)) ([f6a1f1d](f6a1f1d))
🎉 This PR is included in version 3.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
## [3.0.1](elastic/elastic-charts@v3.0.0...v3.0.1) (2019-03-08) ### Bug Fixes * **canvas_text_bbox_calculator:** increase font scaling factor ([opensearch-project#93](elastic/elastic-charts#93)) ([f3080a7](elastic/elastic-charts@f3080a7))
Summary
close #90
This PR fixes the truncation issues with formatted labels on Chrome by adding a 1px padding amount to cover any undercalculation. This also means that we can now significantly reduce the amount that we are scaling fonts by. It does not completely remove the need for a bit of scaling (tested just with offsetting with padding and there was still some truncation).
On Mac OS X Chrome:
Also, see #94 for enhancing this by allowing users to define more padding.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] 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