1
\$\begingroup\$

I have a dedicated stylesheet <style id="dynamicStyles"></style> for my page's dynamic styles. They are controlled by this JavaScript:

function style(className, sty) {
    var sheet = document.getElementById('dynamicStyles').sheet;
    sheet.insertRule('.' + className + ' {' + sty + '}');
}

// No validation, do not pass tainted content.
function unstyle(className, sty) {
    var sheet = document.getElementById('dynamicStyles').sheet;
    var rules = sheet.cssRules;
    var regex = new RegExp('\\.' + className);
    for (var i = 0; i < rules.length; i++) {
        if (rules[i].cssText.match(regex)) {
            sheet.deleteRule(i);
            return;
        }
    }
}

function styleRule(className, that, sty) {
    if (that.checked) {
        style(className, sty);
    } else {
        unstyle(className);
    }
}

This is called, for example, like this:

<input type=checkbox onClick="styleRule('someClass', this, 'border: 1px #a00 dashed')">

There are maybe 10 or 20 inputs with similar code. The number of elements to be styled is large, hundreds or thousands, so I don't want to re-style each of them.

In addition to a general style review, I'm looking to see if there is a more elegant way to do the lookup in unstyle.

\$\endgroup\$
2
  • \$\begingroup\$ Why did you not use event listeners? Add class to each input, for example, js-input. Add change event (change is more suitable than click in this case). And write all logic in the handler. There is no need to add / remove styles. It is more elegant to add / remove classes. However, I can not help much while I do not see all you html. It is difficult to understand the purpose of your code for now. \$\endgroup\$ – FreeLightman Feb 17 '16 at 16:06
  • \$\begingroup\$ @FreeLightman: I have about a thousand elements with the classes, and I don't want to change all of them each time the input is toggled. There's not much to see in the HTML, just a huge number of elements (350 kB). \$\endgroup\$ – Charles Feb 17 '16 at 17:11
3
\$\begingroup\$

Moar built-ins... MOAR (and a look-up table)!

It is great that you are using the built-in insertRule method for style sheets here:

sheet.insertRule('.' + className + ' {' + sty + '}');

It is not great that you aren't using the built in deleteRule for style sheets here:

function unstyle(className, sty) {

Reading the docs, it says that the deleteRule method accepts an index. This index comes from the second parameter of insertRule which you are currently not using. That means all you need to do is start keeping track of the indexes.

In order to keep track of the indexes, class names, and CSS rules, simply create a look up table and use a counter for the indexes:

var ruleToIndexTable = {};
var indexCounter = 0;

Now, when you are styling a rule, simply add it to the table:

indexCounter++;
ruleToIndexTable[rule] = indexCounter;

Note that this will require you to compute the rule somehow, which are technically already doing in the earlier part of the style function:

sheet.insertRule('.' + className + ' {' + sty + '}');

We'll be using this logic again later, so it'll be best to extract this into it's own function computeRule(className, sty) and have both your style and unstyle functions take the single rule as a parameter.

Then later, in your unstyle function, you can simply use the lookup table to find out which table to delete:

function unstyle(rule) {
    sheet.removeRule(ruleToIndexTable[rule]);
}

This assumes that unstyle can access sheet, too, which it now should be able to so you don't have to get the sheet element over and over again (which is inefficient).

With these changes, your code is much more efficient now. With the addition of this look-up table and the new use of the removeRule function, your code now runs in \$O(1)\$ time (I think).

\$\endgroup\$
2
\$\begingroup\$

It would be much easier just to toggle a class on a common parent element (such as body).

Let's say the elements you want to style have the class my-item:

.my-item {
  border: 1px solid blue; /* style them anyway you like */
}

Use a descendant selector, to restyle those elements, when a different class is set on a parent element:

.someClass .my-item {
  border: 1px dashed red;
}

Toggle a class on body:

<input type="checkbox" onchange="document.body.classList.toggle('someClass', this.checked);" />

(Uses classList. For support see https://developer.mozilla.org/de/docs/Web/API/Element/classList or use jQuery's .toggle().)

\$\endgroup\$
1
  • \$\begingroup\$ I considered the common element method you suggest -- maybe I will use this. Thanks for bringing up toggle, I didn't know about that! \$\endgroup\$ – Charles Feb 18 '16 at 13:35
1
\$\begingroup\$

I think you over-complicate the thing with toggling css rules. The whole thing can be done in 3 easy steps:

  1. Add classes for selectors (e.g. tgl-dash-border) or do not add them if you want some css property to be toggled equally for all elements of a type (e.g. input)
  2. Add listeners that would toggle corresponding class (e.g. dash-border)
  3. Add css rules like .dash-border {border: 1px #a00 dashed;}.

This would do the work. And no need to toss stylesheets on the fly.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.