Elkarte icon indicating copy to clipboard operation
Elkarte copied to clipboard

Theme css suggestions

Open Vekseid opened this issue 8 years ago • 17 comments

  1. Put all px values over 1px as em instead

  2. Move all non-color styling to the main files, move all color styling to _light

  • The idea here is to make it easy to make alternate colorations off of the base theme.
  1. There are still over 200 inline style declarations. About thirty of which declare hardcoded color information. We should at least get rid of those.

Vekseid avatar Oct 22 '15 21:10 Vekseid

Move all non-color styling to the main files, move all color styling to _light

Is there a lot of these? I thought the separation was already pretty much done in 1.0 (but of course I didn't look at the css that much... :angel: )

emanuele45 avatar Oct 23 '15 20:10 emanuele45

There are a few here and there. There were either were missed or got added after the separation was done. I don't think there are a lot, but there are some for sure, I noticed them when I put _light on the color diet.

Spuds avatar Oct 23 '15 22:10 Spuds

There are quite a few, plus the ~30 hardcoded style color tags.

Vekseid avatar Oct 24 '15 07:10 Vekseid

After svg embedding, there is not going to be much left to do for this. : )

Vekseid avatar Mar 03 '16 15:03 Vekseid

:+1:

Spuds avatar Mar 03 '16 15:03 Spuds

How about rem instead of em to prevent the compounding effect? Or to put it differently, do we care about IE8? http://caniuse.com/#search=rem

Frenzie avatar Apr 13 '16 17:04 Frenzie

We don't care about IE8.

I've actually been thinking of switching to calc() and viewport units instead. However, the rem vs em vs vwhatever discussion can be had after we've gotten rid of some of the uglier silliness littered throughout the code.

Vekseid avatar Apr 13 '16 18:04 Vekseid

Should this be part of a "theming guidelines" page on the wiki?

emanuele45 avatar Sep 29 '16 21:09 emanuele45

perhaps a note about using em and not px for >1 values for responsive improvements ... then maybe one to do as we say not as we do 😇

Spuds avatar Sep 29 '16 22:09 Spuds

@emanuele45 @Spuds the last inline stuff requires too much refactoring to get done in the RC phase, but maybe we could get more/all of the em conversion done?

Vekseid avatar Sep 09 '17 07:09 Vekseid

Sure.

Just a note about the timing, if at all possible, I would not hold the release for too long, let's say that once we are sure enough the upgrade works without troubles and no other big issues are present (i.e. at the moment the only blocker we have), I would kick off 1.1 and start working on 2.0. Mainly because we have been on "feature freeze" for way too long already (almost two years) and we need to move on IMO. In terms of actual time, I think no more than 1 month.

emanuele45 avatar Sep 09 '17 08:09 emanuele45

Not suggesting we wait on this at all. This would just be a small bonus.

The only thing I consider critical is the accessibility testing.

Vekseid avatar Sep 10 '17 04:09 Vekseid

Getting the accessibility testing done would be fantastic. I know we have "lots" of issue with that but would like to fix any egregious ones we have. For the em change ... I'm assuming it will be based on 1em = 16px ... so if it says 2px we instead want 2/16 or .0625em etc etc

Spuds avatar Sep 10 '17 14:09 Spuds

We want rem, not em. :-) (Dropping IE8 support and all that.)

Frenzie avatar Sep 10 '17 14:09 Frenzie

Do note that rem is not supported on IE 9 & 10 when applying it to pseudo elements

frandominguezl avatar Sep 10 '17 14:09 frandominguezl

I feel the rem change would have to wait for 2.0 as the current css sizing is relative to the font-size of its parent. Changing that now could result in a lot of fixes/adjustments to sizes and not a "simple" swap out as the px to em should be. (although rem for inputs and selects may be interesting to investigate)

Spuds avatar Sep 10 '17 16:09 Spuds

But then why swap it at all? Pixels are also a relative unit that scales along just fine. rem is much more elegant for many purposes, but in typical use in the wild the weird em for pixels thing is just a workaround for an IE<=6 behavior with the undesirable side effect of em cascading.

em cascading is even worse in IE than per spec because of a long-standing bug where it multiplies em values in generated content. (I only mention this because IE9 and 10 generated content behavior with rem was brought up as a counter-argument to rem, but the much worse generated content-related em bug #107657 persists in IE11 and probably Edge.)

I should qualify the above by saying that a combination of rem for font-size and em for margin and padding often makes a lot of sense. You may very well just want 2em of padding based on the element's font size rather than fiddling around with pointless decimal rem values. However, you seldom want em cascading for font sizes, if ever. It's only when you want something like a ~2px border at "regular" zoom/font-size that you'll want to use some silly looking value like .0625rem.

@d3vcho Pity about 10, but in that specific case the px could be kept as a fallback. Heck, I'd probably do that regardless for now. Just add all the .0625rem type stuff under the pixels and leave the decision for future removal for a few years down the line.

Frenzie avatar Sep 10 '17 18:09 Frenzie