joernweissenborn/lcars

Some quasi-buttons do not exhibit the color change behavior when active

Closed this issue · 5 comments

Let "quasi-button" mean a div with the button class. The intended behavior is for the button to change color (by receiving the :active pseudo-class) when clicked. I observe some lcars-elements with button class do and some do not change color on click. Particularly, those that are in a bar and/or header.

Upon closer examination, it looks like some elements background color are marked !important, which is overriding the :active rules.

Experimentally, it seems like one solution is to add the !important specifier to the rule for .lcars-element.button:active (found at line 644 of css/lcars.css). This, of course, ought to be translated into the base Stylus notation if accepted as a solution.

I have input on this issue too.

frameworks should avoid !important
Although I can see why in the case of general color classes if feels necessary [everything selects on class names, you can't know identifiers in advance, class selector specificity has to be overridden somehow].

Like this issue points out though, once the background colors are !important, you end up needing pseudo-class attributes to be !important. And any other color modifying class also has to deploy with !important so it can compete with the classes of framework colors being used...

But, "when everything is important, nothing is."

So I have a suggestion to slightly boost the color selector specificity, instead of using !important.

kinda-important
If we generate the color class selectors as body .lcars-<color> it increases the CSS specificity from matching only the class to also matching an element type (the element has to be inside the tag, so the downside is the only place the color classes won't work is on the body tag itself -- but come'on)

This makes the lcars-colors selectors slightly more specific than the default tan color selector, or any css selector that only specify and match on same-element class names. But the important part is it doesn't resort to !important. It makes the color designation just a bit more important and fits right in with the psuedo classes on the same element.

//Creates dynamic color classes
create-colors(prop, suffix, colors)  
for c, v in colors  
  body .{prefix}{c}-{suffix}  
    {prop} v

I believe this works in all major browsers.

@xenziffen thank you for the explanation. Your solution sounds much more correct, clean, and future-safe. Would you be willing to create a fresh branch with your superior fix? If a little testing came out good, I would then just kill my pull request in favor of yours.
I'm really glad to have your strong skills at the table.

@jrwarwick Yes, I will create a branch for this.

I guess I can ask your thoughts on some options available.

We could use:

1. body .lcars-color-name
Matches for HTML elements that exists within the <body>, slightly more specific than just matching on classname, so gets the job done.

2. div .lcars-color-name
Matches elements contained in a <div> at some level. This alternative has the same specificity as option 1, but in some ways feels more like what a framework user might expect, rather than referring to top-level tags. Elements that use color classes being inside a div will almost always happen because of the column/row pattern.

I like the first option, if we're not trying to over-think it. There's some merit to option 2 if having body in the selectors feels weird.