refactor: color scheme #1245

[kkbt0]

for issue #1245

Change the theme color switching scheme to switch color variables. This is beneficial for removing or adding color schemes at a very small cost.
This may bring compatibility issues, but most browsers now support CSS variables.

https://developer.mozilla.org/zh-CN/docs/Web/CSS/var

Or licensed for use https://github.com/jhildenbiddle/css-vars-ponyfill Compatible with old browsers.

Most styles should be the same, with known differences being code block themes. reference

• https://gohugo.io/content-management/syntax-highlighting/
• https://xyproto.github.io/splash/docs/all.html

The code block uses GitHub and OneDark themes instead of three very long variables.

[vercel]

@kkbt0 is attempting to deploy a commit to the pcloud's projects Team on Vercel.

A member of the Team first needs to authorize it.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
do-it	❌ Failed (Inspect)		💬 Add feedback	Apr 26, 2024 0:01am

[HEIGE-PCloud]

Hi @kkbt0, thanks for you contribution! If I am understanding you correctly, your work is a useful refactor on the current color system. However, I fail to see how it relates to the original issue.

My intention for #1245 is to (visually) adjust the current dark color theme and remove the black theme altogether. DoIt is also in the process of migrating from SCSS to Tailwind CSS for better maintainability. Hence my original plan for this is to

• Decide a new set of colors to be used for the dark theme,
• Define them inside Tailwind config https://tailwindcss.com/docs/customizing-colors#using-custom-colors (Specifically with CSS variables for better customisability https://stackoverflow.com/questions/72117668/tailwind-colors-based-on-dark-mode),
• Add color classes to the HTML elements and remove them from SCSS.

You might notice this PR may not fit very well with the plan, and I am deeply sorry that I failed to explain my intentions more clearly in the original issue.

Here are some context behind the original issue: #1, #654, #1176.

I'd like to hear your thoughts on this.

Many thanks
PCloud

[kkbt0]

Well . I also don't kown the best method. I only used default and dark in TailwindCSS .

Should it be like this? > https://play.tailwindcss.com/KDVQG5ULlM (This example overlooks .)

However, I anticipate that the class length for Hugo's {{ .Content }} typography prose class will be quite lengthy. It'll be a big project.

[HEIGE-PCloud]

Should it be like this? > https://play.tailwindcss.com/KDVQG5ULlM (This example overlooks .)

I was thinking about something like

@tailwind base;
@tailwind components;
@tailwind utilities;

@layer base {

  :root {
    --color-primary: 247 147 34;
    --color-text: 33 33 33;
    --color-success: 0 200 81;
    --color-info: 51 181 229;
    --color-warn: 255 187 51;
    --color-error: 254 78 78;
  }

  :root[class~="dark"] {
    --color-primary: 247 147 34;
    --color-text: 33 33 33;
    --color-success: 0 200 81;
    --color-info: 51 181 229;
    --color-warn: 255 187 51;
    --color-error: 254 78 78;
  }
}

/** @type {import('tailwindcss').Config} */
module.exports = {
  darkMode: "class",
  theme: {
    colors: {
      primary: "rgb(var(--color-primary) / <alpha-value>)",
      text: "rgb(var(--color-text) / <alpha-value>)",
      success: "rgb(var(--color-success) / <alpha-value>)",
      info: "rgb(var(--color-info) / <alpha-value>)",
      warn: "rgb(var(--color-warn) / <alpha-value>)",
      error: "rgb(var(--color-error) / <alpha-value>)",
      transparent: "transparent",
      current: "currentColor",
    },
}

(Reference: https://stackoverflow.com/questions/72117668/tailwind-colors-based-on-dark-mode)

And the user can override any of the CSS variables inside the override.scss

DoIt/assets/css/_override.scss

Line 3 in 0b8fa13

// 覆盖变量

I am not planning to support multiple color themes for now since we are even struggling to coming up with a single color palette for the dark theme. I believe ad hoc customisations for individual color elements will be more useful for the users and easier for us to maintain.

However, I anticipate that the class length for Hugo's {{ .Content }} typography prose class will be quite lengthy. It'll be a big project.

The prose class will indeed be lengthy if we eventually migrate completely to Tailwind, and I am not even sure that's a good idea. But fortunately Tailwind CSS has very good suppport for progressive migration, and it seems to be working quite well integrating with the existing SCSS setup. I am planning to migrate piece by piece lazily rather than all in one go.

Specifically, I want to style all new features using Tailwind, and it worked quite well in the past (See https://github.com/HEIGE-PCloud/DoIt/pull/1214/files#diff-b15932692c619f9a334ecb156ac167557c3af214a8a3df8990c09232ac8a5baa). I also want to migrate existing styles to Tailwind when we are refactoring, so I see both #1236 and #1245 as good opportunities for doing so.

[kkbt0]

Okay, in the example of stackhoverflow, defining the appearance through CSS variables can also easily increase or decrease the theme. In fact, I like these three themes without considering the CSS file size and the large number of duplicates in the two dark modes.(Especially with the addition of code color parameters, the three theme color variables seem to exceed Github's approximately 1000 lines of colors variables .)

So next step we should define all need color variables in a file and use them in tailwind.config.js . Then, step by step, replace the definition of font and background color with class in html .

---

Regarding the issue of Tailwindcss Class being too long, I have tried Unocss Shortcuts . This will put all Tailwindcss classes into a self named class. Just be like https://daisyui.com/

[HEIGE-PCloud]

So next step we should define all need color variables in a file and use them in tailwind.config.js . Then, step by step, replace the definition of font and background color with class in html .

Nice, sound good!

Regarding the issue of Tailwindcss Class being too long, I have tried Unocss Shortcuts . This will put all Tailwindcss classes into a self named class. Just be like https://daisyui.com/

I am not very sure on this one. I don't necessarily see "classes being too long" as an issue, if an element requires a lot of styling, then the class name can be long - that's what it takes to express such information. Utilities like Unocss Shortcuts does not really reduce the classes, it just moves it from HTML to the config file, which in my view defeats the purpose of atomic CSS. The whole idea is that when the style and the element is co-located, it minimises the coupling of the styles between different elements - I no longer need to worry whether changing the style of one element might potentially break the style of another. I think the right way to reduce duplication in styles is to use partial templates to extract elements as reusable "components" but keep the class names as is.

Also, I don't really think it's necessary to completely move to Tailwind, if using prose to style Content is a pain, we can just keep the current SCSS setup and we can have the best of both worlds.

Reference https://tailwindcss.com/docs/reusing-styles#extracting-components-and-partials

[kkbt0]

In some extreme cases, the number of classes can reach 20-30. for example a button in https://catalyst.tailwindui.com/ .
A container that is not the longest in catalyst demo:

<div class="relative h-full w-full rounded-xl bg-white 
shadow-[0px_0px_0px_1px_rgba(9,9,11,0.07),0px_2px_2px_0px_rgba(9,9,11,0.05)] 
dark:bg-zinc-900 dark:shadow-[0px_0px_0px_1px_rgba(255,255,255,0.1)] 
dark:before:pointer-events-none dark:before:absolute 
dark:before:-inset-px dark:before:rounded-xl 
dark:before:shadow-[0px_2px_8px_0px_rgba(0,_0,_0,_0.20),_0px_1px_0px_0px_rgba(255,_255,_255,_0.06)_inset] 
forced-colors:outline">

Actually, there are a lot of class when I am writing a grid element . This is indeed easy to write , but I'm not sure if this is highly readable and easy to maintain code.

<div class="bg-white dark:bg-dark
w-full h-full overflow-hidden rounded-lg 
border border-transparent 
flex flex-col justify-center 
@container
col-span-2 row-span-4 aspect-1/2 
md:col-span-1 md:row-span-2 md:aspect-1/2 
card-transition">

Or create a template partia.After writing blog info card component, it should be like this. https://github.com/saicaca/fuwari/blob/main/src/components/PostCard.astro
Unocss Shortcuts corresponds to Tailwindcss @apply . Although Tailwind think that create a template partial or JavaScript component is better then use @apply.

[HEIGE-PCloud]

I understand your idea, instead of writing

<div class="<a lot of classes>">

You write

<div class="custom-div">

and

.custom-div {
  @apply <a lot of classes>
}

My point is that this does not reduce the amount of classes we write, it merely moves it from one place to another. Now when we want to change the style for the div, we still need to deal with a very long list of classes - just in a different file. But even worse, now we don't know whether someone else is using the custom-div class and whether our changes will break other elements. I don't see how "a long list of class names" is less maintainable than "a large CSS file", if we have a lot of styles, we need to write a lot of code to describe that style, any attempt to hide it or add indirections will just make it worse.

---

One potential use case is that you might use that same div twice on the same page

<div class="<a lot of classes>"></div>
<div class="<a lot of classes>"></div>

Now when we want to adjust its style, we need to make sure we have adjusted every single div that uses the same style. In which case we can just extract that div into a partial as Tailwind's docs suggested. Or even better, we can leverage Hugo to work with variables like:

{{- $style := "<a lot of classes>" -}}

<div class="{{- $style -}}"></div>
<div class="{{- $style -}}"></div>

[kkbt0]

In fact , my code likely be like this

<div class="m-c-basic fc-jc g-2-4">

.m-c-basic {
  @apply bg-white dark:bg-dark w-full h-full overflow-hidden rounded-lg border border-transparent card-transition shadow-[0px_0px_0px_1px_rgba(9,9,11,0.07),0px_2px_2px_0px_rgba(9,9,11,0.05)] dark:bg-zinc-900 dark:shadow-[0px_0px_0px_1px_rgba(255,255,255,0.1)] @container 
}

.fc-jc {
 @apply  flex flex-col justify-center 
}

.g-2-4 {
 @apply col-span-2 row-span-4 aspect-1/2 
}

.g-md-1-2 {
 @apply md:col-span-1 md:row-span-2 md:aspect-1/2 
}

CSS will be like

.m-c-basic {
    width: 100%;
    height: 100%;
    display: flex;
    border-width: 1px;
    border-color: transparent;
    border-radius: 0.5rem;
    container-type: inline-size;
    ....
}

Because this element needs to appear over ten times in the grid. So I abstracted and reused a portion of the code.Perhaps when changing shadows, there is no need to search and replace them in bulk. But I use @apply just to make things look “cleaner” .This may violate the principles in the tailwindcss document.

<div class="m-c-basic fc-jc g-2-4">
<div class="bg-white dark:bg-dark
w-full h-full overflow-hidden rounded-lg 
border border-transparent 
flex flex-col justify-center 
@container
col-span-2 row-span-4 aspect-1/2 
md:col-span-1 md:row-span-2 md:aspect-1/2 
shadow-[0px_0px_0px_1px_rgba(9,9,11,0.07),0px_2px_2px_0px_rgba(9,9,11,0.05)] dark:bg-zinc-900 dark:shadow-[0px_0px_0px_1px_rgba(255,255,255,0.1)] 
card-transition">

Which one is better ?

[HEIGE-PCloud]

Ok, now let's say i want to change rounded-lg to rounded-full, if we are using m-c-basic, what do we do? Do we change the definition of m-c-basic? In which case we need to check every place that uses m-c-basic and make sure our changes are doing what we want. Do we add it to the back of the class list and try to override the rounded-lg provided by m-c-basic? Does it always have a higher specificity?

.m-c-basic {
  @apply bg-white dark:bg-dark w-full h-full overflow-hidden rounded-lg border border-transparent card-transition shadow-[0px_0px_0px_1px_rgba(9,9,11,0.07),0px_2px_2px_0px_rgba(9,9,11,0.05)] dark:bg-zinc-900 dark:shadow-[0px_0px_0px_1px_rgba(255,255,255,0.1)] @container 
}

Is it really "clean"? We still have this long and "unclean" m-c-basic in our config file, we just moved it from the markup to another place.
Is it really "more maintainable"? Before I roughly know what the div will look like when I am reading the code, now I only know it's a m-c-basic and I have no idea what m-c-basic is. Imagine there is a CSS bug around this area, in which case can we track down the culprit faster? If someone else submits a PR that changes m-c-basic, as a reviewer, do you need to check every usage as well? If someone else submits a PR that uses m-c-basic, do you allow it or reject it?

Here is what I would probably do, I would define a custom shadow in the config to make the shadow less error prone. I would also set up the bg-color with CSS variable so I don't need to worry about trivial dark variants. If the div only appears once in the markup - maybe you are generating them with a range loop, than that's fine, that's it. If it appears multiple times, I would either use a partial - if it does not have configurable children, or I would define a Go variable to store these class names right above them.

[kkbt0]

.m-c-basic means my-card-basic . It is a my-cards-baisc child element.
Aa a reviewer, maybe you need to check this base config file is right . Every change to a configuration file usually requires caution.

In fact, this is probably an imitation of Bootstrap. The bootstrap code style is roughly like this.

<div class="card shadow position-relative h-100 rounded-home-item"></div>

Or this is a molecule composed of several atoms. Just like a transition in Tailwindcss . When we refactor this old code, similar molecule code should be very common.

[HEIGE-PCloud]

.m-c-basic means my-card-basic . It is a my-cards-baisc child element.

my-card-basic also does not give me any idea how the div is styled, I still need to search for it in a separate config file and read that long @apply. It just adds one layer of indirection.

Or this is a molecule composed of several atoms. Just like a transition in Tailwindcss . When we refactor this old code, similar molecule code should be very common.

The good thing about transition in Tailwind is that it's immutable - unlike m-c-basic, no one can modify it, so we don't have to worry about whether changing it might break stuff elsewhere, no one can modify it, we either apply it to an element or not.

Such molecules can be useful from time to time, but I view it in a case by case way, and we shall definitely make them immutable if we decide to create our own.

I agree abstractions like

.g-1-2 {
 @apply col-span-1 row-span-2 aspect-1/2 
}

.g-2-4 {
 @apply col-span-2 row-span-4 aspect-1/2 
}

can be useful, but m-c-basic is a bit too much.

[kkbt0]

That is what i want to say. Sometimes we need molecules . The specific amount of aggregation depends on the project requirements.

[HEIGE-PCloud]

Why do we need to hard code all the darken colors here?

Can something like this work?

.style {
  --color: #000;
  color: darken(var(--color), 3%);
}

[kkbt0]

darken is a scss function . color: darken(var(--color), 3%);

ERROR TOCSS: failed to transform "/css/style.scss" (text/x-scss): "/root/DoIt/assets/css/_variables.scss:107:34": argument $color of darken($color, $amount) must be a color
Built in 890 ms
Error: error building site: TOCSS: failed to transform "/css/style.scss" (text/x-scss): "/root/DoIt/assets/css/_variables.scss:107:34": argument $color of darken($color, $amount) must be a color

[kkbt0]

Mabey we should use hsl
https://developer.mozilla.org/en-US/docs/Web/CSS/color_value/hsl

[HEIGE-PCloud]

We are not using darken-8 and darken-10 anywhere right? I can look into each individual usage point and pick an explicit color for each of them.

[kkbt0]

DoIt/assets/css/_partial/_single/_code.scss

Line 79 in a3970ec

background: darken($code-background-color, 8%);

DoIt/assets/css/_partial/_single/_code.scss

Line 130 in a3970ec

background-color: darken($code-background-color, 10%);

so /root/DoIt/assets/css/tailwind.css L129

--code-background-color-code-header: var(--code-background-color-darken-8);
--code-background-color-code-hl: gainsboro; 

should be

--code-background-color-code-hl: var(--code-background-color-darken-10);

It is a err . And dark black theme don't use them ( 8 and 10 ).

[HEIGE-PCloud]

I have been thinking about this, at this point do we really need the mapping from CSS variables to SCSS variables? Is there anything stopping us from just writing

.style {
  color: var(--color);
}

[kkbt0]

I did this at first, but then I kept a mapping. Manage all colors in one SCSS file. I think it's easy to advance and retreat.
This would be useful for refactoring to reduce the same color variables, and to modify them using a format similar to "-- primary".