Migrate to bulma-css-variables and introduce dark mode #954

Merged
konrad merged 72 commits from adrinux/frontend:bulma-css-variables into main 2021-11-22 21:12:55 +00:00
Showing only changes of commit a258b938fa - Show all commits

View File

@ -1,6 +1,6 @@
:root {
/* Vikunja colors as CSS custom properties */
/* Vikunja colors as CSS custom properties */
--grey-50: hsl(210, 20%, 98%);
dpschen marked this conversation as resolved Outdated

Any specific reason why for the switch to hsl?

Any specific reason why for the switch to hsl?

Well:

  • It's the way bulma-css-variables is built. See the source from that line on.
  • hsl is not compatible with the sass colour manipulation mixins but does allow for easy and more understandable specification of variants in itself.
  • It's a superior colour declaration format to hex - though I look forward to lch() being widely supported by browsers ;)
Well: - It's the way bulma-css-variables is built. See the [source](https://github.com/dino4udo/bulma-css-variables/blob/a4c5c4b3730746ae9b0bfb8af754ebfc8ebb88dc/css/bulma.css#L12643) from that line on. - hsl is not compatible with the sass colour manipulation mixins but does allow for easy and more understandable specification of variants in itself. - It's a superior colour declaration format to hex - though I look forward to lch() being widely supported by browsers ;)

It's the way bulma-css-variables is built. See the source from that line on.

That makes sense =)

It's a superior colour declaration format to hex - though I look forward to lch() being widely supported by browsers ;)

LCH is awesome! I always have to force myself to read it corectly because it looks like the German 'me' which is the word 'ich' (at the beginning of a sentence 'Ich' which looks really similar).

> It's the way bulma-css-variables is built. See the source from that line on. That makes sense =) > It's a superior colour declaration format to hex - though I look forward to lch() being widely supported by browsers ;) LCH is awesome! I always have to force myself to read it corectly because it looks like the German 'me' which is the word 'ich' (at the beginning of a sentence 'Ich' which looks really similar).
--grey-100: hsl(220, 14.3%, 95.9%);
--grey-200: hsl(220, 13%, 91%);
@ -16,7 +16,7 @@
--light-background: var(--grey-100);
konrad marked this conversation as resolved Outdated

We should call this just --background-color or --site-background-color.

We should call this just `--background-color` or `--site-background-color`.

I've renamed the --light-background variable to --site-background in a45c6cdf6b

I've renamed the --light-background variable to --site-background in https://kolaente.dev/adrinux/frontend/commit/a45c6cdf6bb001b1a2ff18547f7c3d72eb6f9d8f
--scheme-main: var(--white);
// Vikunja overrides of Bulma defaults
// Vikunja overrides of Bulma defaults
// --black-bis: #121212;
// --black-ter: #242424;
--grey-darker: var(--grey-700);
@ -33,7 +33,7 @@
--white-l: 100%;
--white-a: 1;
--white: hsla(var(--white-h), var(--white-s), var(--white-l), var(--white-a));
--white-translucent: hsla(var(--white-h), var(--white-s), var(--white-l), 0.75);
--white-translucent: hsla(var(--white-h), var(--white-s), var(--white-l), 0.75);
adrinux marked this conversation as resolved Outdated

🔥

🔥

Not clear whether it's the use of variables (we're overriding the ones bulma-css-variables has set) or the fact that vscode seems to have formatted to tabs on paste :(

Fixed it in sublime text but that now formats to tabs on save too :(

I think we need a specific entry for scss files in editorconfig

[*.{scss,css}]
indent_style = space
indent_size = 2

Works for me...but do I include it in this PR?

Not clear whether it's the use of variables (we're overriding the ones bulma-css-variables has set) or the fact that vscode seems to have formatted to tabs on paste :( Fixed it in sublime text but that now formats to tabs on save too :( I think we need a specific entry for scss files in editorconfig ``` [*.{scss,css}] indent_style = space indent_size = 2 ``` Works for me...but do I include it in this PR?

Nahh all good! The 🔥 was in regard to the nice splitting mechanism of the individual color components / channels (not sure what it's called in hsl ?).
That's hot!

Regarding the formatting: we are looking into using prettier either way so I'm ignoring minor formatting stuff right now =)

Nahh all good! The 🔥 was in regard to the nice splitting mechanism of the individual color components / channels (not sure what it's called in hsl ?). That's hot! Regarding the formatting: we are [looking into using prettier](https://kolaente.dev/vikunja/frontend/pulls/930#issue-5468) either way so I'm ignoring minor formatting stuff right now =)

Regarding .editorconfig: maybe create a new separate issue =)

Regarding `.editorconfig`: maybe create a new separate issue =)

Regarding .editorconfig: maybe create a new separate issue =)

Work work work ;)

> Regarding `.editorconfig`: maybe create a new separate issue =) Work work work ;)

--white-translucent has a different opacity than --primary-translucent. I think it should either be the same kind of translucent or there should be variants of it like --white-opacity-75 or similar. On the other hand, since we already defined the hsl parts for it we could probably just use those in combination with a different opacity value directly where it's required.

Then the other thing is that a variable name like white implies that it contains some kind of white. In a dark color scheme that would probably contain something else entirely so white wouldn't actually contain white. Maybe it's a better idea to use a variable name that reflects what this is used for?

The same applies to the greys but I don't really have a good idea for those. I noticed that you did this already for red/green/orange, I think that's the way to go.

`--white-translucent` has a different opacity than `--primary-translucent`. I think it should either be the same kind of translucent or there should be variants of it like `--white-opacity-75` or similar. On the other hand, since we already defined the hsl parts for it we could probably just use those in combination with a different opacity value directly where it's required. Then the other thing is that a variable name like `white` implies that it contains some kind of white. In a dark color scheme that would probably contain something else entirely so `white` wouldn't actually contain white. Maybe it's a better idea to use a variable name that reflects what this is used for? The same applies to the greys but I don't really have a good idea for those. I noticed that you did this already for red/green/orange, I think that's the way to go.

On the other hand, since we already defined the hsl parts for it we could probably just use those in combination with a different opacity value directly where it's required.

That makes most sense to me. There are very few instances of translucent colours at present.

Then the other thing is that a variable name like white implies that it contains some kind of white. In a dark color scheme that would probably contain something else entirely so white wouldn't actually contain white. Maybe it's a better idea to use a variable name that reflects what this is used for?

Unfortunely we inherit those from bulma/bulma-css-variables, so I think we're stuck with them for now. I know it seems odd, but you get used to it.

The same applies to the greys but I don't really have a good idea for those.

I think you already have the solution just think of --grey-100 as 10% dark in light mode and 10% light in dark mode.

I noticed that you did this already for red/green/orange, I think that's the way to go.

An easy change because bulma/bulma-css-variables already defines those danger/success/warning variables. Bulma Colors docs.

> On the other hand, since we already defined the hsl parts for it we could probably just use those in combination with a different opacity value directly where it's required. That makes most sense to me. There are very few instances of translucent colours at present. > Then the other thing is that a variable name like `white` implies that it contains some kind of white. In a dark color scheme that would probably contain something else entirely so `white` wouldn't actually contain white. Maybe it's a better idea to use a variable name that reflects what this is used for? Unfortunely we inherit those from bulma/bulma-css-variables, so I think we're stuck with them for now. I know it seems odd, but you get used to it. > The same applies to the greys but I don't really have a good idea for those. I think you already have the solution just think of --grey-100 as 10% dark in light mode and 10% light in dark mode. > I noticed that you did this already for red/green/orange, I think that's the way to go. An easy change because bulma/bulma-css-variables already defines those danger/success/warning variables. [Bulma Colors docs](https://bulma.io/documentation/overview/colors/).
--black-h: 0deg;
--black-s: 0%;
@ -63,16 +63,15 @@
--primary-h: 216.5deg !important;
--primary-s: 100% !important;
--primary-l: 54.9% !important;
--primary-light-l: 73%;
--primary-a: 1 !important;
--primary: hsla(var(--primary-h), var(--primary-s), var(--primary-l), var(--primary-a));
// simulate sass lighten($primary, 30) by increasing lightness 30% to 73%
adrinux marked this conversation as resolved Outdated

Is it possible to use calc() here?

Is it possible to use `calc()` here?

Ahh I saw the !important now

Ahh I saw the `!important` now

Is it possible to use calc() here?

I did initially but really I think it just obfuscates the actual value, though I don't much like the this either.
So:

calc(var(--primary-l) * 1.3)

Works to give a 30% increase?

--primary-light is only used in src/views/tasks/TaskDetailView.vue and I've no idea what end result it has. Can't see what different values do. Any idea?

If we stick with hard coding a value I wonder if using the pattern bulma-css-variables uses would be clearer:

 --primary-h: 216.5deg !important;
 --primary-s: 100% !important;
 --primary-l: 54.9% !important;
 --primary-light-l: 73%;
 --primary-a: 1 !important;
 --primary: hsla(var(--primary-h), var(--primary-s), var(--primary-l), var(--primary-a));

  // simulate sass lighten($primary, 30) by increasing lightness 30% to 73%
  --primary-light: hsla(var(--primary-h), var(--primary-s), var(--primary-light-l), var(--primary-a));
  --primary-translucent: hsla(var(--primary-h), var(--primary-s), var(--primary-l), 0.5);

Thoughts? EDIT: have actually just changed it to the latter.

> Is it possible to use `calc()` here? I did initially but really I think it just obfuscates the actual value, though I don't much like the this either. So: ``` calc(var(--primary-l) * 1.3) ``` Works to give a 30% increase? --primary-light is only used in src/views/tasks/TaskDetailView.vue and I've no idea what end result it has. Can't see what different values do. Any idea? If we stick with hard coding a value I wonder if using the pattern bulma-css-variables uses would be clearer: ``` --primary-h: 216.5deg !important; --primary-s: 100% !important; --primary-l: 54.9% !important; --primary-light-l: 73%; --primary-a: 1 !important; --primary: hsla(var(--primary-h), var(--primary-s), var(--primary-l), var(--primary-a)); // simulate sass lighten($primary, 30) by increasing lightness 30% to 73% --primary-light: hsla(var(--primary-h), var(--primary-s), var(--primary-light-l), var(--primary-a)); --primary-translucent: hsla(var(--primary-h), var(--primary-s), var(--primary-l), 0.5); ``` Thoughts? EDIT: have actually just changed it to the latter.
--primary-light: hsla(var(--primary-h), var(--primary-s), 73%, var(--primary-a));
--primary-translucent: hsla(var(--primary-h), var(--primary-s), var(--primary-l), 0.5);
}
--primary-light: hsla(var(--primary-h), var(--primary-s), var(--primary-light-l), var(--primary-a));
--primary-translucent: hsla(var(--primary-h), var(--primary-s), var(--primary-l), 0.5);
@media (prefers-color-scheme: dark) {
:root {
@media (prefers-color-scheme: dark) {
adrinux marked this conversation as resolved Outdated

Nest the @media so we don't need to repeat :root.

Nest the `@media` so we don't need to repeat `:root`.

Good catch. Done a258b938fa

Good catch. Done https://kolaente.dev/adrinux/frontend/commit/a258b938fabdb69b30615897b157c37a020b4059

Maybe it would make sense to group all dark scheme adjustments (and the base light versio) in separate stylesheets instead of seperating by color or shadows?

Maybe it would make sense to group all dark scheme adjustments (and the base light versio) in separate stylesheets instead of seperating by color or shadows?

I think this is fine. Because we might need adjustments deep in components for dark mode either way.
I think it would be wrong to separate those styles from the components.

I think this is fine. Because we might need adjustments deep in components for dark mode either way. I think it would be wrong to separate those styles from the components.

That's something I didn't think about. Makes sense.

That's something I didn't think about. Makes sense.
/* Light mode colours reversed for dark mode */
--grey-900: hsl(210, 20%, 98%);
--grey-800: hsl(220, 14.3%, 95.9%);
@ -96,4 +95,5 @@
--text-light: var(--grey-300) !important;
--text-strong: var(--grey-900) !important;
}
}