-
Notifications
You must be signed in to change notification settings - Fork 690
Added height fitting option (by KonradJanica) #69
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
base: master
Are you sure you want to change the base?
Conversation
| */ | ||
| private static void autofit(TextView view, TextPaint paint, float minTextSize, float maxTextSize, | ||
| int maxLines, float precision) { | ||
| int maxLines, float precision) { |
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.
nit: revert
|
This is much cleaner! The new API method names seem a bit odd to me. Instead of |
|
The proposed naming convention of |
…nested conditional for non-height-fitting modes
…o duplicate minTextSize check and setTextSize calls
|
I pushed a commit with the API refactoring. Let me know if it sounds clearer. |
|
|
||
| private boolean mEnabled; | ||
| private boolean mIsAutofitWidthEnabled; | ||
| private boolean mIsAutofitting; |
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.
Why AutofitHelper::mIsAutofitHeightEnabled is a static variable?
If I use two different AutofitTextView in my app, one with AutofitHeight enabled, and a second with AutofitHeight disabled, then the first will get AutofitHeight disabled too. Is this the expected behaviour?
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.
I believe the field is static so that it can be accessed via the private static void autofit(TextView view, TextPaint paint, float minTextSize, float maxTextSize, int maxLines, float precision) method. I'm not sure that the method needs to be static though. @grantland?
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.
I think you could leave the method autofit(...) static, but then pass mIsAutofitHeightEnabled as a parameter to it. This way, each AutofitTextView can have its own mIsAutofitHeightEnabled setting instead of sharing a global one.
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.
Good point! I pushed the appropriate change
|
Awesome, looks good ^^ Thank you for your commits. Just a small nitpick. The variable named When you use it inside a static function (like the |
|
Thank you @AranHase! |
|
Any updates on this. Have been checking in on this pull request for a while now? |
|
I made a quick fork of this with @Will5 's changes because I need to use it in my project. It's available from jitpack.io here: https://github.com/flipagram/android-autofittextview -- When this is all merged in I'll remove my fork. |
|
why has this still not been merged? |
|
Please merge this PR, we do need this fix for our project as well. Thanks! |
Took KonradJanica's code (#43) and reformatted for easier review by grantland.