patternjavascriptMinor
Responsive text by JavaScript
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).
DEMO
Meaning of the used variables:
↑ executes the JavaScript Plugin on an element.
(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
aork= 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 (
-
-
-
Your code will re-parse the minimum and maximum font sizes each time the handler is called. Cache these instead.
-
-
-
When iterating arrays using
-
If
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:
-
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.