1

I have CSS style properties "sprinkled" through out my JavaScript code.

Here is an example line:

label_element.style.opacity = 0;

I was planning on moving all CSS out of the code except className changes.

For example if I need to dynamically change some CSS dynamically with JavaScript, I plan to consolidate the changes to a className change as such:

.new_class{
    label_element.style.opacity = 0;
}

Therefore to dynamically change CSS all changes are made with class changes as such:

div_el.className = "new_class";

All CSS remains in the CSS file this way. JavaScript only has access to class names.

I wanted to verify/validate this is a good way to do it.

Marc Audet
  • 46,011
  • 11
  • 63
  • 83
  • 1
    That's a completely acceptable method of doing it - using classes to change the look of HTML elements instead of dynamically adding CSS properties. – Qantas 94 Heavy Jun 26 '13 at 14:17
  • 1
    that is definitely something you should do – cptnk Jun 26 '13 at 14:18
  • Yes, Its a good idea. – Rajeev Bera Jun 26 '13 at 14:18
  • should this even be done for one liners? –  Jun 26 '13 at 14:19
  • The question is good... I just think it's more suitable to [Code Review](http://codereview.stackexchange.com/) than to SO. Not downvoting, just voting to move. – Geeky Guy Jun 26 '13 at 14:19
  • that way your designer would not have to look trough all your .js files to change something so... yeah I would advice you do this even for 1 liners. – cptnk Jun 26 '13 at 14:20
  • 1
    @the_web_situation that's because the Stack is segregated by very specific topics. There is a site for code review, a site for computer science, a site for programmers' life, one for super users, one for server configuration issues etc. SO is the one where you go to fix small blocks of code. – Geeky Guy Jun 26 '13 at 14:55
  • ...okay...related question ... http://stackoverflow.com/questions/195951/change-an-elements-css-class-with-javascript/196038#196038 –  Jun 26 '13 at 17:42

5 Answers5

2

I would like to suggest using some simple helper functions to add/remove classes instead of appending a classname:

Element.prototype.hasClass = function(name){
    return RegExp('(\\s|^)' + name, 'g').test(this.className);
};
Element.prototype.addClass = function(name){
    if(!this.hasClass(name))
        this.className += (this.className ? ' ' : '') + name;
};
Element.prototype.removeClass = function(name){
    this.className = name ? this.className.replace(RegExp('(\\s|^)' + name, 'g'), '') : '';
};

Demo here: http://jsfiddle.net/EdAEB/

nderscore
  • 4,182
  • 22
  • 29
  • http://caniuse.com/#search=classList –  Jun 26 '13 at 15:31
  • I'm toggling between 3 different classes...only 1 should be applied...unfortunately there is no native solution for this. I'm going to just use a prefix to set groups like this toggle_namespace_className and write some code similar to what you have posted. –  Jun 26 '13 at 16:22
  • I need to implement toggleNS here ... http://jsfiddle.net/bDuPY/1/ –  Jun 26 '13 at 17:47
  • You're trying to toggle between three classes? Try this: http://jsfiddle.net/3DfkZ/3/ – nderscore Jun 26 '13 at 18:39
  • Here it is on code review. Thanks for the start ... http://codereview.stackexchange.com/questions/27822/addclass-removeclass-extension-to-html5-api –  Jun 26 '13 at 22:17
1

Yes, as you probably get from the comments, it's the preferred thing to do. One slight remark, though:

elem.className = 'foobar';

Might not prove ideal when working with multiple classes, best play it safe and go for:

elem.className += ' foobar';

Of course, in cases like this:

window.addEventListener('click', function(e)
{
    e = e || window.event;
    var target = e.target || e.srcElement;
    if (target.tagName.toLowerCase() === 'p' && target.className.test(/\btoggle\b/))
    {
        target.style.display = target.style.display === 'hidden' ? 'block' : 'hidden';
    }
}, false);

You could use a class for that, but I get the feeling that toggles will be doable in CSS3, and you could just leave code like this as is, to support older browsers, for now.

Elias Van Ootegem
  • 74,482
  • 9
  • 111
  • 149
  • @the_web_situation: While that works with smaller pages with few scripts interacting, it requires to some degree that they all know about each other. Past a certain point, you'll want to have each script only making its own changes to an element...and *adding* a CSS class rather than *setting* one makes the scripts a lot more independent of each other. – cHao Jun 26 '13 at 14:33
  • @the_web_situation: fair enough, but sometimes, you might not know for sure if an element already has one (or more) classes set... that's why I suggested the concatenation – Elias Van Ootegem Jun 26 '13 at 14:33
  • ...just use the inspector of whatever browser to see what classes an element has. –  Jun 26 '13 at 14:41
  • 1
    @the_web_situation but, your code can't use the inspector. – nderscore Jun 26 '13 at 14:43
  • @the_web_situation: That's not the point, if you're setting classes as you go in your code, you can test, and test until you're ready to retire, someone will probably manage to interact with your site in that one way you didn't foresee, and your layout will fall to pieces. Something you can easily avoid by adding a `+` to the assignment operator, and prepending a space to the actual class. 2 chars isn't really worth the hours of torture that debugging this would otherwise cause – Elias Van Ootegem Jun 26 '13 at 14:45
  • there are so many better options now ... see ... http://caniuse.com/#search=classList ... I would remove the += all together. –  Jun 26 '13 at 15:40
1

About 6 months ago, I was using the following process:

  1. Declare an array called classes
  2. Use a function to add, remove items from the array. Each item of the array is a css class
  3. Then join the elements of the array with an space to get an string like: "class1 class2 class3 class4".
  4. Apply directly or indirectly, the class string to the className attribute of an element, like this:

document.getElementById('anyElementID').className = classesArray.join(" ");

Now, you don't have to do this, you can simply use the almost recently added DOM classList API, which has the add, remove, toggle and contain methods.

Here you go: http://davidwalsh.name/classlist

Have a nice day!

  • I'm toggling between 3 different classes...only 1 should be applied...unfortunately there is no native solution for this. I'm going to just use a prefix to set groups like this `toggle_namespace_className` and write some code similar to nderscore. –  Jun 26 '13 at 16:22
  • Why don't you use the toggle method of the classList API? Do you need backwards compatibility? – Raziel Ravenheart Jun 26 '13 at 16:38
0

I need to go against the grain here..

One of the major disadvantages of having all style in the CSS file, is in situations where style values need to be calculated. One real world example we encountered, was that of a resizable dialog box. As we changed the size of the browser window, then style values (width, and height), need to be calculated. There are many other situations we encountered where style values needed to be calculated, so in the end we had a situation where some CSS was in the CSS files, and some was in the Javascript. This is NOT pretty.

On a more personable level, i find that storing all the style in the CSS file breaks the OO principle of encapsulation. If I have specific styles for a specific object, then those specific styles should be kept with that specific object. As our projects have gotten bigger our CSS files have gotten horrendously ugly.

Oliver Watkins
  • 12,575
  • 33
  • 119
  • 225
-1

I can't agree...

It depends what javascript has to do. Sometimes it is better to have a class, but not every time. Complicated layout can spam your css file. You are trying to clean your javascript code, but think about it a little. Efect can be also inverted. For example widgets and plugins. Some plugins require theirs own css file. Some require more then one file. It can make a lot of mess in file structure. And when more people work on one project then it is harder to maintain, modify or repair bugs, because of too complicated file structure and too many class references ...

I solve this seperatly for each style change that I do in javascript. It is more important for me to make code easy to read and maintain then be strict.

Entity Black
  • 3,401
  • 2
  • 23
  • 38