google / incremental-dom

An in-place DOM diffing library
http://google.github.io/incremental-dom/
Apache License 2.0
3.54k stars 180 forks source link

Might classList.add() make sense #312

Closed thepian closed 6 years ago

thepian commented 7 years ago

In my experience a lot of state logic affects the class attribute. Managing it with Element.classList had simplify some of the more complex components. The style attribute might be another that can become quite hairy, but other attributes should work just fine with the existing attr() call.

It might be a good addition to offer API for add/remove of classes

sparhami commented 7 years ago

Generally instead of having dynamic classes, you should use attributes instead. You can still do CSS styling with attributes and they make more sense for things that are related to state.

For example, if you have a menu in the "open" state, the classification of the menu has not changed from a "menu" to an "open menu". Contrast that with, for example, a button that can be a "plain" button or perhaps "login" button, which changes styling.

So you might do something like:

elementOpen('div', null, null,
  'class', 'menu',
  'role', 'button',
  'aria-expanded', isOpen);

and you could style it like

.menu[aria-expanded="true"] {
  /* do stuff! */
}

In general, you will likely need to have some way to communicate the state of the component to a user using a screen reader through the the right aria attribute, which you could also use to do any styling necessary.

Incremental DOM allows you to pass an object with property names and their values for style. It currently isn't smart (will rewrite the styles each time instead of diffing), but that may be fixed in the near future.

thepian commented 7 years ago

I completely agree with using attributes, but I think many concerned with browser performance still recommend using classes for getting the best performance. With some approaches(web pack css modules) it is even pretty much the only option.