Add a dark mode based on bulma-css-variables #1826
Labels
No Label
dependencies
duplicate
help wanted
invalid
kind/bug
kind/feature
needs reproduction
question
security
wontfix
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vikunja/vikunja#1826
Loading…
Reference in New Issue
No description provided.
Delete Branch "%!s(<nil>)"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
With most browsers now supporting the CSS media query
@prefers-color-scheme: darkit should be relatively simple to add a dark mode to Vikunja by defining the colours as CSS custom properties (aka CSS variables).However as per previous disscussion about themes (see issue vikunja/frontend#484 and this forum post.) it's currently blocked by Bulma, the CSS framework on which Vikunja is built.
In short, Bulma uses SASS variables but pre-processes them to hard coded colours, so by time they reach the browser they're no longer variable!
I tried redefining colours used in Vikunja as CSS custom properties, which got me 80% of the way but it became clear that doing that last 20% would require rewriting large parts of Vikunja frontend CSS to no longer be based on Bulma - not something I have time for. You can see that work on my css-vars-dark branch https://kolaente.dev/adrinux/frontend/src/branch/css-vars-dark
However I found a fork of Bulma with it's variables redefined as CSS custom properties Bulma-css-variables and by swapping Bulma for Bulma-css-variables I've been got 95% of the way to a decent dark mode.
Bulma-css-variables seems to be an active project and has been maintained though several releases of Bulma, so it seems viable to adopt it. But this would be a big descision, and it's not mine to make :)
There several issues with my conversion at this point:
That said you can check out my https://kolaente.dev/adrinux/frontend/src/branch/bulma-css-variables branch, do
yarn run serveopen it up in your browser and point it to your own Vikunja backend to get a look at it.Next steps:
/essay-mode-off
Well, I've figured out why I had to use !important so much. When the custom color styles are included inline they're included first, before the base from bulma-css-variables.
This makes sense in terms of SASS variables I presume, where you need the overrides in place before preprocessing all the CSS, but with CSS custom properties you need the overrides added last in the inline styles.
That's why. No clue as to how to reorder the inline styles though.
👋 @adrinux,
Nice work!
I'm today short in time, but funnily I was yesterday working on a moving the styles together with the components. I'm currently working on moving vikunja to vue3 but was stuck a bit and needed something else to work on that wouldn't create too many merge issues. Styles seemed like a sure thing 😆 .
It's all really work in progress, that's why I didn't create even a WIP pull request. yet — I just commited the status and will probably force-push later and untangle some stuff (e.g. remove the new pagination component from that branch).
A lot of stuff is currently broken, but in general having scoped styles per component is IMHO the way to go and helps a lot removing that deep nested complexity that the current style depends on. With all that said, here's that branch:
https://kolaente.dev/dpschen/frontend/src/branch/feature/wip-move-styles-to-components
Regarding bulma: I didn't take a deep look but bulma-css-variables variables looks interesting. As you say it's something that should be thought of in detail before making a decision to switch. Generally I have a bit bad feeling that bulma seems so poorly supported recently with so many open issues and pull requests (or was it always like this?).
I didn't take a look at your code yet. I might have time this evening. Maybe it's somehow possible to combine these efforts 🤷♂️
One note to
@prefers-color-scheme: dark: I think if a dark mode is added that should go along with a user option to ignore the systems color mode and force either dark or light mode as wished.Bulma-css-variables is really just a drop in replacement that allowed me to get this working with minimal effort. It's not something that you'd necessarily want if you start majorly refactoring the frontend code, in that case you'd probably be better not building on Bulma at all.
I'd never touched Bulma (or any other CSS framework) before this. I also ditched SASS in favour of PostCSS and css-next (now css-preset-env) years ago, so even the scss makes my head hurt a bit :)
I think Bulma's main problem is that it was built, like SASS to fix missing features in CSS which have now arrived in CSS. It's now a pile of technical debt. The main Bulma dev sems to want to add CSS custom properties whilst still maintaining backward compatibility - laudable but I think it'd be better to just do a major version break instead.
I'm familiar with the idea of components (but never personally worked with them), will take a look at your code later. But if Vikunja is ever to support themes, including dark mode for each of them then I think you'd at least want all the colours centralized and defined with css custom properties then just refered to in the components. Then swap out the color.css with theme switch (I'm sure you know all this).
So I'm not sure a combination is really the way to go, I more see this work I've done as a temporary fix to get a dark mode right now - I've a feeling it could be finished and end up in a release within a couple of weeks - whereas more major refactoring like you seem to be doing should probably be used as an oportunity to do things better than Bulma. And that I can see myself contributing too from the CSS side.
I think OS level dark mode can be overriden at the browser level? Either way a user level switch would require something more like themeing and with a UI to configure it - not what I was aiming for initially.
Yeah I agree. My target was mostly to solve the weird issues that come along with the global styles:
!importantneeded.My target was mostly to merge the scss with the template and js as it's usual in vue.
In the current codebase the styles often have this form:
I think what was preventing the combination of styles with their template until now was that the variables where not imported globally (aka prefixed before the styles in every component).
I love post-css-preset-env. Sadly it's not actively updated right now which prevents it from supporting PostCSS 8 (is an issue e.g. in Nuxt). Right now I usually help myself out by manually importing what I need. But it's hard to keep track of that.
What I do still like with SASS is that it's stable and many (still) know how to write it.
BTW: with Autoprefixer Vikunja is using PostCSS already, so shouldn't be a problem to add other functionality. SASS and PostCSS don't exclude each other :)
Totally agree with the new major version here. But maybe he doesn't want to invest to much work and support 2 versions he doesn't have time for :D
On the other hand it's maybe good that bulma is a purely class based system. This way it's possible to use it just where wanted. This should work even better when using scoping. If wanted you could remove it later peace by peace.
I really love scoped styles! I couldn't work without it anymore. Right now you will just see the
scopedattribute in the styles block in the components. I hardly optimized the css for it. But what it allows you is to e.g. use the same class multiple times in different components in your project, let's say.buttonwithout any conflicts. The VueLoader does this by addding data attributes to the elements and styles to make the selectors unique (looks something like that:.button[e2z4f1].I think it's the reason why vue never had the so many other solutions for styles then e.g. react because the scoping just works™️ 😛. This is probably also what causes the issues that are still there: in cases where the styles were intended to be used by multiple components (not easy to find out from the source) I might have moved the CSS to one of the multiple and in conclusion destroyed some styling.
Sidefact: some kind of scoped styling seems to be coming to native html / css, see https://css.oddbird.net/scope/
Agree. All colors (vars) should be defined just at one location.
This is where I see things a bit different: I hope it won't take that long, since I don't want to refactor styles for now, but just move the styles together with the template they are written for. Doing things better then Bulma can be done later :)
Would love that 👍 and love to get some knowledge of the experience you have with PostCSS. Except post-css-preset-env plus some other modules I didn't dig too deep there yet, since SCSS serves me usally well.
That's true, but usually not per page! So by introducing a dark mode without the option to opt-out you might make some people angry. Because of this I think there is question that some kind of option has to come hand in hand with the dark mode.
Out of the reasons above I think some UI is necessary. To make things simple it could be a dropdown with three options (system-default, light, dark) that get saved in local storage. Maybe local storage isn't even the worst choice since it would allow a user to use different settings based on the end device he is using.
Thanks for that heads up, hadn't actually realised - my CSS kept working I suppose, makes me wonder how much :)
But have taken a closer look - lurking on their discord channel right now - it seems there is currently plenty of activity. It's not visible in postcss-preset-env because the blockers, and the work to resolve them, are all in the various modules it depends on and those are all in their own github projects.
Oh that was my point. You just say Bulma 9.4 is final. Here is the more modern Bulma 10 and you'll have to migrate your sites to 10 or just freeze them at 9.4. No need to complicate things.
In short, components are less DRY but more readable/discoverable/maintainable :)
I checked out your code, I get the utility of what your doing, but...
There is no halfway house with the SASS variables to CSS custom properties transition though, it can't be done incrementally. Frontend wont compile, fix that and some colours are still quite broken.
But it's easy to transition things, once I switched Bulma to Bulma-css-variables it was really just a case of find/replace of, for example $white to var(--white) on the whole code base.
It'd be easy to do it before moving all the styles to components and easy to do after - even if you make the bulma switch in the middle it's still very easy to do a quick find and replace on your component styles and then cary on migrating styles to components.
But you can't really get away with changing one instance of $var at a time as you migrate to a component, you'll never know if its the change to the component or the colour variable that has caused breakage.
I can see the utility of that, I'd assumed it would be an option at some point, but I still don't see why it has to be there on day one. But again, not my choice :)
Nice! Hopefully they release something soon :)
Not sure I understand this. In my experience putting the style in components is much more DRY. Depends probably on the way you see it.
I put the styles next to the template that uses the styles. By doing that the selectors can be really flat (almost no nesting necessary).
In my experience reusing classes globally is a bad think to do (#fail bulma) since it's hard to have an overview where which classes are actually used.
It tends to make code more complex since you might need deep nested selectors and don't remember that some class was used somewhere else for a different use-case and break the styling by modifing it.
Global utilities on the other hand are fine! Regardless of using PostCSS, SCSS mixins or Tailwind (via the the @apply selector). Using tailwinds CSS classes of course is a topic on its own :D / they solve this issue by using PurgeCSS.
Yes, for sure! I misunderstood you there.
So that dark mode isn't enabled for someone that just wanted his system UI dark, not his task manager, without a way to opt-out.
I meant it quite literally Dont Repeat Yourself - put the styles in a central place and let them cascade over multiple components, you only need to write the styles once.
Put the styles in the component and scope them and you may have to write them multiple times, as you said a couple of comments ago "I might have moved the CSS to one of the multiple and in conclusion destroyed some styling", in that sense using components can be less DRY.
I'm not disagreeing with you, I'm aware of the advantages components bring but they are an example where being less DRY is better.
Surely it's the scoping that lets you keep it flat?
I think we are on the same level here :)
Regarding:
This doesn't have to be true. Here are two ways I can think of to prevent this:
Mixins
You can define the styles global as a scss mixin (or PostCSS).
So you will have duplicate classes. Maybe with different names, because they are scoped and in different components and you forgot the name that you used to apply the styles in the mixin to in the other component. But that doesn't matter since they are scoped. Also it makes sure that you know where your styles came from.
Using components to scope styles
If there is another occasion where you need the same style most of the time what you actually want is to reuse some component including its style. Maybe it doesn't exist yet, because you created too large components.
Either way, with...
... you might have misunderstood me. The reason for this was that the styles were written in a way that they affected multiple components from the beginning.
If you write styles in a way that they always just affect the local component you'll never run into that problem.
I might be able to push an update to the styles branch later, maybe I find a better example there :)
Correct :)
Sorry for the late response! Looks like you two have already been quite busy discussing this.
I think using bulma-css-variables is a way to go. Especially since I'd like to have customizable theming in the long-term.
There is a way to figure out if the current browser wants a dark color scheme with js which then can be used to set a css class to enable the dark theme. Tailwind has a nice example of this in their docs.
I agree the current frontend could use improvement. In the long-term I'd like to refactor that, maybe even moving away from bulma entierly. Having everything scoped in components would probably help with that. That would be a whole heap of work with little short term benefit - It would make it more maintainable in the long-term though. I don't see it happening anytime soon so I think we should focus on improving what we currently have. That includes moving more styles to components and using bulm-css-variables. Thoughts?
In my day job I've come to love tailwind. We've recently migrated a big less codebase over to tailwind which took us something like 4 months (in combination with a layout modernization though). It was bigger than what Vikunja currently is, but has grown substantially over the years and was very much unmaintainable. I'd like to avoid that for Vikunja.
Not the answer I was expecting :)
But glad to hear it. In that case I will find time next week to iron out the remaining missing parts and bugs.
Some good hints there. I've been putting the colour custom properties in ':root' by default but should be able to scope them to .dark instead.
One other point to note is Browser compatibility though I suspect Vikunja was already beyond the point of supporting IE...
Sounds great!
That is correct. There's other JS-Features already that are not supported in Vikunja. Microsoft has announced the EOL of IE so I see no point in continuing to support it.
There's also this: https://twitter.com/jgthms/status/1372241503423184898
But since there doesn't seem happen anything around that in bulma I'm not sure how reliable that is.
Btw: a few minutes ago postcss-preset-env with postcss 8 support came out =)
I got the impression the bulma-css-variables was forked from a css variables branch in the bulma repo, but as you say, not much activity on css variables in the main repo.
Also FYI I noticed some missing comma's in bulma-css-variables
6a95eee3abemailed dino4udo (Daniil Chumachenko) and he said he'd fix it ASAP (last wednesday). Tried to send him a PR but github just wants to PR to upstream Bulma...I think you have to do that in that repo manually.
Might be good to integrate your changes before merge. I was aware that there is not much activity, but I saw this as a way to get to the dark mode as quick as possible, so I guess we are fine =)
I'm not aware of any visible problems caused by the handful of broken rules - I don't think vikunja uses those bulma vars.
They do cause browser devtools to complain about rejecting those syntax violating rules though. Minor developer annoyance. Hopefully Daniil will apply a diff and do a new release soon...
As far as I understand it that file is using thing we don't have in Vikunja so I guess this is not blocking #954?
Exactly, they're all variations on .file-cta class, not something Vikunja appears to use at the moment.
I created a pull-request: vikunja/frontend#1022
I guess this can be closed?
I think with the merging of #954 in
46fa43d67fthis is closed.