HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Responsive text by JavaScript

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
responsivetextjavascript

Problem

I'm working on a JavaScript plugin that makes font-sizes flexible (for a responsive layout). The code works well so far, but I'm sure that it's capable of improvement (especially if you look at the global variable).

(function () {
    var extend = function (obj, ext) {
        for (var key in ext) {
            if (ext.hasOwnProperty(key)) {
                obj[key] = ext[key];
            }
        }
        return obj;
    };

    window.txt = function (el, k, c) {
        var s = extend({
            n: Number.NEGATIVE_INFINITY,
            x: Number.POSITIVE_INFINITY
        }, c);

        var fit = function () {
            var a = k;

            var i = function () {
                el.style.fontSize = Math.max(Math.min(el.clientWidth / a, parseFloat(s.x)), parseFloat(s.n)) + "px";
            };
            i();
            window.addEventListener("resize", i);
            window.addEventListener("orientationchange", i);
        };

        if (el.length) {
            for (var i = 0; i < el.length; i++) {
                fit(el[i]);
            }
        } else {
            fit(el);
        }

        return el;
    };
})();


DEMO

Meaning of the used variables:

  • n = minFontSize



  • x = maxFontSize



  • s = settings



  • c = config



  • el = element



  • i → running the function



  • a or k = fontRatio/compressor



window.txt(document.getElementById("title"), 10);

↑ executes the JavaScript Plugin on an element.

Solution

Thoughts:

-
Variable names: I hate typing unnecessary text as much as the next dev, but variable names are not a good place to skimp. It's one thing when using i as an index variable, for example, but for others, please, please, please use meaningful names. If you have to provide a key after the fact, this should be a good clue that you've not adequately done so.

-
Don't pollute the global space. If the caller needs access to the function, return it instead. Or use modules (module.exports or the like).

-
minFontSize less than zero doesn't make much sense. I'd cap it at zero rather than –∞.

-
maxFontSize being capped at +∞ probably doesn't make sense either.

-
Your code will re-parse the minimum and maximum font sizes each time the handler is called. Cache these instead.

-
var a = k: Why? In this case, just use k instead.

-
i() does not do what you think it will: if el is an array, el.style is going to be undefined, and el.style.fontSize will error.

-
When iterating arrays using for (;;;), cache the length, like so:

for (let i=0, l=arr.length;i<l;i++) { ... } // for ES5 use "var"


-
If k = 0, what happens? Division by zero...

This is a very quick rendition of your code (in ES6), with meaningful variable names, and implementing caching and such, but it might be useful:

function fitText(el, ratio, {minFontSize, maxFontSize}) {
  el.style.fontSize = Math.max(Math.min(el.clientWidth / ratio, maxFontSize), minFontSize) + "px";
}

function attachListeners(el, ratio, {minFontSize, maxFontSize} = {}) {
  const boundFitText = fitText.bind(undefined, el, ratio, {minFontSize, maxFontSize}),
        addEventListener = window.addEventListener;
  addEventListener("resize", boundFitText, false);
  addEventListener("orientationchange", boundFitText, false);
}

export function adjustTextSize(el = [], ratio = 1, 
  { 
    minFontSize=0, 
    maxFontSize=Number.POSITIVE_INFINITY 
  } = {}) 
{
  if (ratio === 0) {
    return el; // prevent division by zero later; throwing an exception might be better here, though
  }

  const parsedOptions = {
    minFontSize: parseFloat(minFontSize),
    maxFontSize: parseFloat(maxFontSize)
  }

  const els = (el instanceof Array) ? el : [el];
  for (let i=0, l=els.length; i<l; i++) {
    let actualEl = els[i];
    fitText(actualEl, ratio, parsedOptions);
    attachListeners(actualEl, ratio, parsedOptions);
  }
  return el;
};

Code Snippets

for (let i=0, l=arr.length;i<l;i++) { ... } // for ES5 use "var"
function fitText(el, ratio, {minFontSize, maxFontSize}) {
  el.style.fontSize = Math.max(Math.min(el.clientWidth / ratio, maxFontSize), minFontSize) + "px";
}

function attachListeners(el, ratio, {minFontSize, maxFontSize} = {}) {
  const boundFitText = fitText.bind(undefined, el, ratio, {minFontSize, maxFontSize}),
        addEventListener = window.addEventListener;
  addEventListener("resize", boundFitText, false);
  addEventListener("orientationchange", boundFitText, false);
}

export function adjustTextSize(el = [], ratio = 1, 
  { 
    minFontSize=0, 
    maxFontSize=Number.POSITIVE_INFINITY 
  } = {}) 
{
  if (ratio === 0) {
    return el; // prevent division by zero later; throwing an exception might be better here, though
  }

  const parsedOptions = {
    minFontSize: parseFloat(minFontSize),
    maxFontSize: parseFloat(maxFontSize)
  }

  const els = (el instanceof Array) ? el : [el];
  for (let i=0, l=els.length; i<l; i++) {
    let actualEl = els[i];
    fitText(actualEl, ratio, parsedOptions);
    attachListeners(actualEl, ratio, parsedOptions);
  }
  return el;
};

Context

StackExchange Code Review Q#83425, answer score: 8

Revisions (0)

No revisions yet.