vikunja/frontend
Archived
6
39
Fork 48
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

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
Conversation 24 Commits 72 Files Changed 73 +3 -3
2 changed files with 3 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 6c59567ebf - Show all commits

4
src/styles/theme/theme.scss
View File

@ -6,7 +6,7 @@
}
:focus {
box-shadow: 0 0 0 2px var(--primary-translucent);
box-shadow: 0 0 0 2px hsla(var(--primary-hsl), 0.5);
adrinux marked this conversation as resolved Outdated
konrad commented 2021-11-07 13:56:47 +00:00
Outdated
Review
Copy Link

Doesn't rgba(var(--primary), 0.5) work?

Doesn't `rgba(var(--primary), 0.5)` work?
dpschen commented 2021-11-07 14:19:37 +00:00
Outdated
Review
Copy Link

No, since the var already includes hsla() so resolved by the browser it would be double wrapped:

rgba(hsla(var(--primary-h), var(--primary-s), var(--primary-l), var(--primary-a)), 0.5)

See: https://kolaente.dev/vikunja/frontend/pulls/954/files#issuecomment-18553

rgba() also just accepts r, g, b, a values. SASS would render this as as the css rgba() and not the SCSS rgba() helper since it includes a custom-property which can not be resolved by SASS at build time.

No, since the var already includes `hsla()` so resolved by the browser it would be double wrapped: ```css rgba(hsla(var(--primary-h), var(--primary-s), var(--primary-l), var(--primary-a)), 0.5) ``` See: https://kolaente.dev/vikunja/frontend/pulls/954/files#issuecomment-18553 `rgba()` also just accepts r, g, b, a values. SASS would render this as as the css [rgba()](https://developer.mozilla.org/en-US/docs/Web/CSS/color_value/rgba()) and not the SCSS [`rgba()` helper](https://sass-lang.com/documentation/modules#rgb) since it includes a custom-property which can not be resolved by SASS at build time.
adrinux commented 2021-11-08 15:51:33 +00:00
Outdated
Review
Copy Link

Given that --primary-translucent is only used twice (both in theme.scss, lines 8 & 18), it might make more sense to use the existing primary-* vars to build those 2 instances.

box-shadow: 0 0 0 2px hsla(var(--primary-h), var(--primary-s), var(--primary-l), 0.5;

or do what I've done with some of the greys and define a 'naked' --primary-hsl that we can drop in:

box-shadow: 0 0 0 2px hsla(var(--primary-hsl), 0.5;

I think the latter is most readable, and doesn't make the alpha value hard to see.

Thoughts @konrad @dpschen ?

Given that --primary-translucent is only used twice (both in theme.scss, lines 8 & 18), it might make more sense to use the existing primary-* vars to build those 2 instances. ``` box-shadow: 0 0 0 2px hsla(var(--primary-h), var(--primary-s), var(--primary-l), 0.5; ``` or do what I've done with some of the greys and define a 'naked' --primary-hsl that we can drop in: ``` box-shadow: 0 0 0 2px hsla(var(--primary-hsl), 0.5; ``` I think the latter is most readable, and doesn't make the alpha value hard to see. Thoughts @konrad @dpschen ?
konrad commented 2021-11-08 15:53:44 +00:00
Outdated
Review
Copy Link

I think the second one makes most sense.

I think the second one makes most sense.
dpschen commented 2021-11-08 16:16:56 +00:00
Outdated
Review
Copy Link

I just realize that we have to define that --primary-hsl just for that usecase. Then we could also just keep with the --primary-translucent.

Because of that I would prefer the first solution. 🤷‍♂️

I just realize that we have to define that `--primary-hsl` just for that usecase. Then we could also just keep with the `--primary-translucent`. Because of that I would prefer the first solution. 🤷‍♂️
adrinux commented 2021-11-10 15:08:48 +00:00
Outdated
Review
Copy Link

I created --primary-hsl as described above 6c59567ebf

I created --primary-hsl as described above https://kolaente.dev/adrinux/frontend/commit/6c59567ebf1bd1e706554ec33326c886706fb45a
👍 1
}
:focus:not(:focus-visible) {
@ -15,7 +15,7 @@
:focus-visible,
:-moz-focusring {
box-shadow: 0 0 0 2px var(--primary-translucent);
box-shadow: 0 0 0 2px hsla(var(--primary-hsl), 0.5);
}
body {

2
src/styles/variables/colors.scss
View File

@ -65,11 +65,11 @@
--primary-l: 54.9%;
--primary-light-l: 73%;
--primary-a: 1;
--primary-hsl: 216.5deg, 100%, 54.9%;
--primary: hsla(var(--primary-h), var(--primary-s), var(--primary-l), var(--primary-a));
adrinux marked this conversation as resolved Outdated
dpschen commented 2021-11-04 20:47:06 +00:00
Outdated
Review
Copy Link

Is it possible to use calc() here?

Is it possible to use `calc()` here?
dpschen commented 2021-11-04 21:03:38 +00:00
Outdated
Review
Copy Link

Ahh I saw the !important now

Ahh I saw the `!important` now
adrinux commented 2021-11-05 12:43:49 +00:00
Outdated
Review
Copy Link

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.
// 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);
@media (prefers-color-scheme: dark) {
adrinux marked this conversation as resolved Outdated
dpschen commented 2021-11-04 20:47:44 +00:00
Outdated
Review
Copy Link

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

Nest the `@media` so we don't need to repeat `:root`.
adrinux commented 2021-11-05 12:59:32 +00:00
Outdated
Review
Copy Link

Good catch. Done a258b938fa

Good catch. Done https://kolaente.dev/adrinux/frontend/commit/a258b938fabdb69b30615897b157c37a020b4059
👍 1
konrad commented 2021-11-07 14:09:50 +00:00
Outdated
Review
Copy Link

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?
dpschen commented 2021-11-07 14:25:38 +00:00
Outdated
Review
Copy Link

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.
👍 1
konrad commented 2021-11-07 14:29:01 +00:00
Outdated
Review
Copy Link

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 */