Added height fitting option (by KonradJanica) #69
Conversation
| @@ -89,7 +92,7 @@ public static AutofitHelper create(TextView view, AttributeSet attrs, int defSty | |||
| * Re-sizes the textSize of the TextView so that the text fits within the bounds of the View. | |||
| */ | |||
| private static void autofit(TextView view, TextPaint paint, float minTextSize, float maxTextSize, | |||
| int maxLines, float precision) { | |||
| int maxLines, float precision) { | |||
|
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 mIsAutofitting; | ||
| private static boolean mIsAutofitHeightEnabled; |
There was a problem hiding this comment.
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.
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.
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.
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.