Feedback / suggestions

[joshribakoff]

Great plugin. I must have gone through several dozen plugins claiming to adjust font size before I found this one.

Line Heights & Corner case bugs

FYI at really small font sizes it breaks. If you set a div to 5px tall by 500px wide, the line-height of the div is never modified, this pushes the span to a new line, like this JSFiddle I made to show the issue: https://jsfiddle.net/zhtt7t8p/5/

You can see there the span doesn't line up. I fixed it in my code by forking your code & setting the line-heights. For the "parent", I do this in the binary search [probably crap code but shows the fix]:

$(innerSpan).parent().css('line-height', '0px'); // set parent's line height to 0
$(innerSpan).css('line-height', mid + 'px'); // set child line height to same as font size

This was required to ensure that the inner span doesn't overflow it's parent. I also modified the part after the binary search:

// Sub 1 at the very end, this is closer to what we wanted.
$(innerSpan).css('line-height', (mid-1) + 'px');
innerSpan.style.fontSize = (mid - 1) + 'px';

Callback & "responsive" resizing

Since your plugin runs the sizing logic once when invoked, and doesn't bind to any resize events, I wrote my own plugin that runs yours when invoked, and again when the elements are resized. However your algorithm causes more resizes to fire, which causes an infinite loop. I worked around this by setting overflow:hidden to prevent unwanted resize events. Since your algorithm is asynchronous (it takes time to try the different font sizes until it is done), the resize events can fire while the algorithm is still running, creating race conditions where different instances of the algorithm are fighting over the same piece of text simultaneously. To workaround this my plugin simply wraps your plugin in Underscore's _.throttle() method to ensure it won't be called more frequently than once a second.

If your plugin simply had a callback for when it was done sizing the text, I as a user could unbind the resize event while your algorithm is "working" to only rebind it when your algorithm is "done". This would fix a lot of race conditions & make it easier to bind the plugin to resize events of it's parent element.

Browser Bugs

I'm still working on this one but sometimes if the parent has some width (let's say 500px), and you put the span of text there inside it & start iterating through font sizes, sometimes I've seen Chrome set the span's width to 501px (1px overflowing its parent). I think it depends on some font settings or letter spacing. Still debugging.

The problem this causes is that your algorithm responds to this by continuing to decrease the font size. For some reason, independent of font size, Chrome is always rendering the inner span at 501px until the font-size is decreased enough for all text to fit on one line.

The only workarounds I can think of are

1) In the binary search when it's checking if the width/height are acceptable, compare it to the parent's size +3px to give it some "wiggle room" (this can make the fonts too big in 99% of cases).

2) I noticed another thing we can use to detect this "situation" (browser bug). If the size of the inner span (501px in our case) has not changed between multiple iterations of the binary search (and that size is within some acceptable threshold of the target size), we can conclude that the font size is acceptable, and assume that the 1px overflow is due to a browser bug that will not be affected by any further font-size decreases (at least not without making the text so undesirably small that it defeats the purpose of the plugin)

[STRML]

Hey @joshribakoff,

Thanks for the feedback.

I don't want to fix the corner case with line-heights because the plugin is intended to preserve line-heights. Unless I am misunderstanding, the fix would obliterate any present line-height.

As for running on resize, if you bind to window resize (window.addEventListener('resize', function() { ... })) you won't have any trouble with resize being fired by the plugin. I believe the 'resize' event you're catching is actually a jQuery addition and doesn't actually exist in the DOM.

This library actually isn't asynchronous. A completion callback wouldn't make sense.

As for that browser bug, I've seen it before (and put it in Chromium's bug tracker). I'll give it some more thought. Do you have a jsfiddle that is broken by this that you could show me?

[joshribakoff]

the fix would obliterate any present line-height.

The whole point of textFit is that I want to "obliterate" my font-size (and line-height). Checkout the screenshot I emailed you. I attribute the aesthetic text to modifying line-height. It depends on your use case. The plugin could support both use cases with a setting to disable it. If you wanted to make it really extensible you could have a strategy method allowing the user to define a strategy (modify other stuff besides just font-size) or define their own "test" (other than the "innerSpan.offsetWidth <= originalWidth"....).

Edit - ok it seems here the issue is I didn't know about setting line-height to a percentage value, let me play around some more.

As for running on resize, if you bind to window resize (window.addEventListener('resize', function() { ... })) you won't have any trouble with resize being fired by the plugin. I believe the 'resize' event you're catching is actually a jQuery addition and doesn't actually exist in the DOM.

I'm binding to the resize event of the "parent" div. textFit increases the inner span height which will increase the size of it's parent div (unless it is set to overflow:hidden, the div's height increases to accommodate the larger span - or in the case of overflow:scroll; the scrollbar's toggling enabled/disabled state appears to fire resize events).

In simpler terms, the "trying of different font sizes" sometimes causes "resize" events on the parent div. I need to bind to the parent div, and not the window. The reason is the page reflows for other reasons than resizing the window. Maybe I'm toggling a jQuery sidebar, or I have it in an Angular app where a column has ng-show/ng-hide. In any case, other elements on the page can cause reflows without necessarily firing a "window" resize. My concern then is how do I respond to these reflows & notify "textFit" to redraw my text?

This library actually isn't asynchronous. A completion callback wouldn't make sense.

Technically your demo is since it uses setInterval(). But you're right the real version is synchronous. But I'm right that it still takes time, and I wish to call a function that takes time from an asynchronous event.

Can you show me then an example of how to bind it to the "resize" event without any race conditions, and without having to use lodash's "Throtttle" method to workaround the fact the plugin takes time? It needs a callback to do so I think. This way I can unbind the event until the callback completes, then rebind the event. My concern with the "throttle" approach is even if I set it to let's say 100ms, someone could run the app on a slow computer where drawing the text takes longer.

As for that browser bug, I've seen it before (and put it in Chromium's bug tracker). I'll give it some more thought. Do you have a jsfiddle that is broken by this that you could show me?

Your JSFiddle is very helpful, I was having issues coming up with my own JSFiddle. I'll try to play around with some ideas on workarounds for the issue & report back with results.

[joshribakoff]

To simulate the browser bug in a consistent manner I hardcoded a "+1" in the "test" where it checks if the text is within bounds. I then modified the code so that fitted the text properly in the presence of the bug.

I also removed my line-height hacks & set line-height: 100% in the CSS which scales the line-height correctly as the fonts change.

I found that as long as I use the version of your script with the while loop instead of setInterval it behaves in a synchronous manner & can be called without throttling.. I think I was testing with the "slow" version that you modified to behave asynchronously when I saw those race conditions. So that is solved now.

Also in my screenshot I sent you you maybe saw I was working at micro font sizes. As a part of my fix, I modified the script to have an "adjustment Amount" that I set to 0.1 instead of 1.0 that you had. This makes a world of difference & fixes issues where up to 25% of my "height" goes unused because it doesn't try out fractional sizes.

Here's the part of the code I modified, the binary search:

 // Binary search for best fit
            var previousValueOfInnerSpanOffsetWidth = false;
            var previousValueOfInnerSpanOffsetHeight = false;
            var adjustmentAmount = 0.1;

            while ( low <= high) {
                mid = parseFloat((low + high) / 2, 10);
                innerSpan.style.fontSize = mid + 'px';

                var withinHeightBounds = innerSpan.offsetHeight <= originalHeight;
                var withinWidthBounds = innerSpan.offsetWidth <= originalWidth;
                var withinBounds = withinWidthBounds && (settings.widthOnly || withinHeightBounds);

                // if there was a previous w/h, and the width has stayed the same & the height has decreased, and we're stil inside the bounds
                if(previousValueOfInnerSpanOffsetWidth !== false && previousValueOfInnerSpanOffsetHeight !== false &&
                    innerSpan.offsetHeight <= previousValueOfInnerSpanOffsetHeight &&
                    previousValueOfInnerSpanOffsetWidth === innerSpan.offsetWidth && withinHeightBounds
                ) {
                    console.info('chromium issue #289511 detected, treating as within bounds instead of outside');
                    low = mid + adjustmentAmount;
                } else if(withinBounds){
                    low = mid + adjustmentAmount;
                } else {
                    high = mid - adjustmentAmount;
                }

                previousValueOfInnerSpanOffsetWidth = innerSpan.offsetWidth;
                previousValueOfInnerSpanOffsetHeight = innerSpan.offsetHeight;
            }
            // Sub 1 at the very end, this is closer to what we wanted.
            //innerSpan.style.fontSize = (mid - 1) + 'px';

As you see I also commented out the part where it subtracts 1px at the end. This was part of the issue with it leaving up to 25% unused height when your div is small like mine.

My "new feature" (sub-pixel fonts) could be improved by adjusting the adjustment amount. It could start with an adjustment amount of let's say 1px. Once it detects that low & high are within 1px of eachother, it decreases the adjustment amount by a "meta adjustment amount" & continues, until low & high are within some configurable threshold of each other. (Eg. it makes adjustments of 1px until its detected the best fit to a precision of 1px, then it starts making 0.1px adjustments until it's within 0.1px of the best fit, it could then continue with 0.01px adjusments, depending on a user-defined precision. For now its sufficient for my purposes so I'm not going to modify any more.

[joshribakoff]

@STRML Hopefully I'm not annoying you with my "feedback" ;)

I lied when I said I was done hackin on it. It still has rounding issues where it makes bad use of height, here's a demonstration... even after increasing it's precision by an order of magnitude as shown above, it leaves a "blank last line"... Here's my parent div that I need my text to fill up (div.theText). This is the element I call the plugin on:

After the plugin sizes up the text, I can see the span is not utilizing the full height:

Margin of error: ~50px

Here's a possible solution I'm playing around with:

// Binary search for best fit
            var previousValueOfInnerSpanOffsetWidth = false;
            var previousValueOfInnerSpanOffsetHeight = false;
            var adjustmentAmount = 0.00000000000001;

            innerSpan.style.opacity = 0;
            while ( low <= high) {
                mid = parseFloat((low + high) / 2, 10);
                innerSpan.style.fontSize = mid + 'px';

                var innerSpanOffsetHeight = innerSpan.offsetHeight;
                var innerSpanOffsetWidth = innerSpan.offsetWidth;
                var withinHeightBounds =  innerSpanOffsetHeight  <= originalHeight;
                var withinWidthBounds = innerSpanOffsetWidth <= originalWidth;
                var withinBounds = withinWidthBounds && (settings.widthOnly || withinHeightBounds);

                var totalAreaOutsideBounds =  (innerSpanOffsetHeight - originalHeight) *
                    (innerSpanOffsetWidth <= originalWidth);

                // shouldn't the algorithm stop here?!
                if(totalAreaOutsideBounds >= -1  && totalAreaOutsideBounds <= 3 && Math.round(high-low) === 0) {
                    break;
                }

                // if there was a previous w/h, and the width has stayed the same & the height has decreased, and we're stil inside the bounds
                if(previousValueOfInnerSpanOffsetWidth !== false && previousValueOfInnerSpanOffsetHeight !== false &&
                    innerSpanOffsetHeight <= previousValueOfInnerSpanOffsetHeight &&
                    previousValueOfInnerSpanOffsetWidth === innerSpanOffsetWidth && withinHeightBounds
                ) {
                    //console.info('chromium issue #289511 detected, treating as within bounds instead of outside');
                    low = mid + adjustmentAmount;
                } else if(withinBounds){
                    low = mid + adjustmentAmount;
                } else {
                    high = mid - adjustmentAmount;
                }

                previousValueOfInnerSpanOffsetWidth = innerSpanOffsetWidth;
                previousValueOfInnerSpanOffsetHeight = innerSpanOffsetHeight;
            }
            // Sub 1 at the very end, this is closer to what we wanted.
            innerSpan.style.fontSize = (mid + adjustmentAmount) + 'px';
            innerSpan.style.opacity = 1;

Basically, I changed the adjustment amount to 1-e15, which is the most precision that I can see inside Chrome's debugger. I also hide the text while we're sizing it around by setting it's opacity to 0.

When playing around in the debugger now, and setting breakpoints on the 3 lines that can either increment or decrement mid, I see that the binary search works well until we get to smaller precisions. The "test" of doing low <= high does not work well for small fine tunings. It continues oscillating between going 1px out of bounds, and ~ -30px within bounds, and seems to unpredictably settle on -30px within bounds before it gets to 0px exactly in bounds. I don't think we have enough floating point precision for the binary search to always find the perfect 0px within bounds match every time, due to rounding errors & such.

I added a piece of logic that short circuits the binary search if the "total area within bounds" is within some acceptable threshold of an "exact match". This stops the binary search early as soon as it finds it something that's within a given threshold. This allows the algorithm to stop searching when it finds a "suitable fit" rather than the "perfect fit" which may not be found before we round out of floating point precision. (eg. in the case where the search is oscillating between 1px too big, and -50px too small, it'll short circuit on the acceptable margin of error, and prevent the chance of the binary search stopping before it ever finds the best fit & risking a 50/50 chance of getting the larger margin of error)

After these changes, the text always fills up my div for a perfect "visual fit". I even found that some of the fonts include extra space for letters like "g" that dip below line-height. Since I allow the 3px overflow on the height, it actually cancels this out & gives "the perfect fit" every time (knock on wood before I go down another 12hr debugging session lol!):

Here my div is 117px tall

My short circuit stopped the search as soon as it found something 3px 'too tall' (120px) which is actually "visually perfect" & more performant than trying to find the mathematically "perfect" fit.

Margin of error: 3px

Edit: FYI the 1-e15 adjustment amount is not needed most likely. The key is setting this small enough to prevent the binary search from ending too early (0.0001 is likely fine), but then have a short circuit that stops the binary search from oscillating too long & running out of hairs to split due to rounding. With the right tuning of these parameters I believe you'll achieve the true "perfect fit" that we're all after.

[joshribakoff]

Still finding more corner cases with the "last line" being blank. I think this time I finally fixed it!

Basically I set the binary search back to a precision of 1px. Then after that is done, I have a system of 10 passes that make subsequently smaller adjustments to the font-size (+/- 1px, +/- 0.1px, +/- 0.01px, etc..) Here's the progress so far below. Since the container will not be divisible by a font-size that gives a perfect fit, I'm going to try to use translateY() to stretch it into place as a final step of the algorithm.

Updated Code:

// Binary search for best fit
            var previousValueOfInnerSpanOffsetWidth = false;
            var previousValueOfInnerSpanOffsetHeight = false;
            var adjustmentAmount = 1;

            var innerSpanOffsetHeight;
            var innerSpanOffsetWidth;
            var withinHeightBounds;
            var withinWidthBounds;
            var withinBounds;
            var totalAreaOutsideBounds;
            var totalPercentageOutsideBounds;

            //innerSpan.style.opacity = 0;
            while ( low <= high || (Math.round(high-low)==0 && totalPercentageOutsideBounds < 0.1 )) {
                mid = parseFloat((low + high) / 2, 10);
                innerSpan.style.fontSize = mid + 'px';

                innerSpanOffsetHeight = innerSpan.offsetHeight;
                innerSpanOffsetWidth = innerSpan.offsetWidth;
                withinHeightBounds =  innerSpanOffsetHeight  <= originalHeight;
                withinWidthBounds = innerSpanOffsetWidth <= originalWidth;
                withinBounds = withinWidthBounds && (settings.widthOnly || withinHeightBounds);

                totalAreaOutsideBounds =  (innerSpanOffsetHeight - originalHeight) *
                    (innerSpanOffsetWidth <= originalWidth);

                totalPercentageOutsideBounds = totalAreaOutsideBounds / (originalHeight*originalWidth);

                // if there was a previous w/h, and the width has stayed the same & the height has decreased, and we're stil inside the bounds
                if(previousValueOfInnerSpanOffsetWidth !== false && previousValueOfInnerSpanOffsetHeight !== false &&
                    innerSpanOffsetHeight <= previousValueOfInnerSpanOffsetHeight &&
                    previousValueOfInnerSpanOffsetWidth === innerSpanOffsetWidth && withinHeightBounds
                ) {
                    //console.info('chromium issue #289511 detected, treating as within bounds instead of outside');
                    low = mid + adjustmentAmount;
                } else if(withinBounds){
                    low = mid + adjustmentAmount;
                } else {
                    high = mid - adjustmentAmount;
                }

                previousValueOfInnerSpanOffsetWidth = innerSpanOffsetWidth;
                previousValueOfInnerSpanOffsetHeight = innerSpanOffsetHeight;
            }
            console.log('Binary search finished @ font-size',mid, totalAreaOutsideBounds, 'px overflow is present');

            var fontMicroStepSize = 1;
            var passNum = 0;
            var lastFontSize = false;

            while(passNum <= 10 && Math.abs(totalAreaOutsideBounds) > 1) {
                passNum++;
                fontMicroStepSize /= 10;
                while (totalAreaOutsideBounds > 0) {
                    mid -= fontMicroStepSize;
                    innerSpan.style.fontSize = mid + 'px';

                    innerSpanOffsetHeight = innerSpan.offsetHeight;
                    innerSpanOffsetWidth = innerSpan.offsetWidth;
                    withinHeightBounds = innerSpanOffsetHeight <= originalHeight;
                    withinWidthBounds = innerSpanOffsetWidth <= originalWidth;
                    withinBounds = withinWidthBounds && (settings.widthOnly || withinHeightBounds);

                    totalAreaOutsideBounds = (innerSpanOffsetHeight - originalHeight) *
                    (innerSpanOffsetWidth <= originalWidth);

                    console.log('adjust -=', fontMicroStepSize, 'to ', mid, 'with overflow', totalAreaOutsideBounds);
                }
                while (totalAreaOutsideBounds < 0) {
                    mid += fontMicroStepSize;
                    innerSpan.style.fontSize = mid + 'px';

                    innerSpanOffsetHeight = innerSpan.offsetHeight;
                    innerSpanOffsetWidth = innerSpan.offsetWidth;
                    withinHeightBounds = innerSpanOffsetHeight <= originalHeight;
                    withinWidthBounds = innerSpanOffsetWidth <= originalWidth;
                    withinBounds = withinWidthBounds && (settings.widthOnly || withinHeightBounds);

                    totalAreaOutsideBounds = (innerSpanOffsetHeight - originalHeight) *
                    (innerSpanOffsetWidth <= originalWidth);

                    console.log('adjust +=', fontMicroStepSize, 'to ', mid, 'with overflow', totalAreaOutsideBounds);
                }
                console.log('Micro adjust passNum', passNum, ' w/ precision',fontMicroStepSize,' finished @ font-size',mid, totalAreaOutsideBounds, 'px overflow is present');
                console.log('delta for pass was ', mid-lastFontSize);
                lastFontSize = mid;
            }
            console.log('Micro adjust done & finished @ font-size',mid, totalAreaOutsideBounds, 'px overflow is present');

            innerSpan.style.opacity = 1;

[joshribakoff]

This is the new code that goes at the end of that script, stretches it into place with CSS to workaround containers that have no ideal font-size that is divisible for a perfect fit:

var totalHeightOutsideBounds = totalAreaOutsideBounds; // @todo fix this...

            var stretchY = originalHeight/innerSpanOffsetHeight;
            var offsetY = -1*(totalHeightOutsideBounds/2);

            //console.log(stretchY, offsetY);

            innerSpan.style.opacity = 1;
            innerSpan.style.transform = 'matrix(1.000, -0.000, 0.000, '+stretchY+', 0.000, '+offsetY+')';