-
Notifications
You must be signed in to change notification settings - Fork 255
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
Improve text chunk component #181
Conversation
Codecov Report
@@ Coverage Diff @@
## development #181 +/- ##
===============================================
+ Coverage 57.54% 63.49% +5.94%
===============================================
Files 189 189
Lines 34434 34460 +26
===============================================
+ Hits 19815 21880 +2065
+ Misses 12072 12070 -2
+ Partials 2547 510 -2037
Continue to review full report at Codecov.
|
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.
Great. Wondering whether we need to account for line height/cap height?
} | ||
|
||
// Fit fits the chunk into the specified bounding box, cropping off the | ||
// remainder in a new chunk, if it exceeds the specified dimensions. |
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.
@adrg Can you add a note regarding the line height to the comment for Fit function. Probably the only one that needs it?
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.
Sure, a note sounds like a good idea. Yes, the Wrap method does not need the line height.
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.
LGTM
Addresses #178
This change is