Reimplemented paragraph fit w/ binary search

[derekmiranda]

• faster than previous implementation as difference between min and
  max font size increases
• based on research from other text fitting procedures

Test results:

• Fitting "This is just a fit paragraph" or "This is a fit-clamp paragraph" within a 100x100 container

minFontSize = 6, fontSize = 50:
old implementation rerenders w/ new font size 29 times
new implementation rerenders: 14

minFontSize = 1, fontSize = 100:
old implementation rerenders: 79
new implementation rerenders: 16

However, w/ minFontSize = 1, fontSize = 10:
old implementation rerenders: 0
new implementation rerenders: 1

[andrewdowis]

I don't see a benefit to having fontSizeCheckFn() defined as an inline function (var optimalFontSize = ... on line 679). Making it a private function ensures it's already in memory and that any time there's an auto size, the function doesn't have to be defined again. Although, I guess I do understand basically purging the function when done. Still, declaring it and passing it around rather than calling a static function within the class doesn't make as much sense to me.

[derekmiranda]

I can rework it, I had similar thoughts as well and thought that I’d made the PR too early

On Mon, Oct 29, 2018 at 12:08 PM Andrew Dowis ***@***.***> wrote: ***@***.**** commented on this pull request. I don't see a benefit to having fontSizeCheckFn() defined as an inline function (var optimalFontSize = ... on line 679). Making it a private function ensures it's already in memory and that any time there's an auto size, the function doesn't have to be defined again. Although, I guess I do understand basically purging the function when done. Still, declaring it and passing it around rather than calling a static function within the class doesn't make as much sense to me. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#17 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIe4czUPrU4jQxsf61wZrwd5wBuIQDJWks5up1JDgaJpZM4X_1rm> .

-- Derek Miranda | Developer | *RED* Interactive Agency | www.weareRED.com <http://www.wearered.com/>

…

------------------------------ CONFIDENTIALITY NOTICE: This e-mail, including any previous messages and/or files attached to it, is private, confidential, and solely for the use of the intended recipient(s). Any review, retransmission, dissemination, reproduction or other use of, or taking of any action in reliance upon, this information by persons or entities other than the intended recipient(s), or by a person responsible for delivering it to the intended recipient(s), is unauthorized and strictly prohibited. ------------------------------

[kennyarehart]

I agree with Andrew that the fontSizeCheckFn() should be abstracted. Also, resizeTimes on 681 is not defined within the module. I assume this is just for testing but don't forget to remove that when finalizing. I would do a few more tests with different width/height/font/minFont etc. I'm sure it will yield the same but just to be sure.