intercellular / cell

A self-driving web app framework
https://www.celljs.org
MIT License
1.5k stars 93 forks source link

Style attribute and the element's style property are conflicted in Cell elements #136

Closed tphecca closed 7 years ago

tphecca commented 7 years ago

Currently, specifying a style for an element within Cell is usually done like this:

el = {
  style: "background-color: red;",
  $text: "Hello, world!"
}

While it is convenient to define the element's style attribute within Cell, it overwrites the element's style property with a string (that you defined for the attribute). However, when I access a non-Cell HTMLElement's style property (or if the property was not overwritten by Cell) I will get a CSSStyleDeclaration object as described here: https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/style

It works fine to apply styles to the element, but when you need to change it within the script, you must change the string (theoretically, I have not tested that) instead of changing the CSSStyleDeclaration as you would with a non-Cell element. This leads to unpredictable behavior when dealing with the styling of Cell elements in JavaScript:

el = {
  style: "background-color: red;",
  $text: "Hello, world!",
  onhover() {
    // What you should do with an HTMLElement:
    this.style.backgroundColor = "blue"; // this.style is a string, so that doesn't do anything!
  }
}

The current solution is to not define a style attribute, and instead use the given CSSStyleDeclaration in the element's $init function to add styling (or just use a CSS file, which can be inconvenient in some situations).

To retain the convenience and flexibility of styling within the element's Cell definition, I think a $style special keyword should be added, or at least the style attribute should be handled properly and not overwrite the style property of the element. If there were a $style keyword, its value could be an object that has keys corresponding to the CSSStyleDeclaration properties you want to change:

el = {
  $style: { backgroundColor: "red" },
  $text: "Hello, world!", 
  onhover() {
    this.style.backgroundColor = "blue"; // It works!
    // (this effect is for demonstrating Cell; the :hover pseudo selector should be used in CSS)
  }
}

If a styling is not important enough to be its own special keyword, developers should still be informed to set their styles within the $init function of an element if they want to use a proper style property in their scripts, or resolve the conflict between Cell's style attribute and JavaScript's style CSSStyleDeclaration.

By the way, Cell is the reason I enjoy web dev again after months of being burned out. Thanks for making this awesome script and sharing it to the world!

mdings commented 7 years ago

By the way, Cell is the reason I enjoy web dev again after months of being burned out. Thanks for making this awesome script and sharing it to the world!

Sorry for being offtopic but I had the same feeling! Nothing to setup, no module loaders to config, no outdated dependencies to update. Just load in the damn lib and get going. Awesome!

gliechtenstein commented 7 years ago

@mdings Thanks! Any off-topic comment that says nice things about the library is welcome 😂

@tphecca thanks for the thorough description of the problem. I'm on it! Will update when done.

gliechtenstein commented 7 years ago

Alright, now it's supported! https://github.com/intercellular/cell/pull/138

Please test it out and let me know if something doesn't work right!

gliechtenstein commented 7 years ago

Assuming it's all working fine, closing :) Please reopen if it doesn't.