Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set sass variables with css variables to allow easy overrides #4263

Merged
merged 18 commits into from
Dec 13, 2024

Conversation

frjo
Copy link
Member

@frjo frjo commented Dec 7, 2024

Fixes #4258

Also clean out unused styling items.

With new dart sass and @use instead of @import it is not possible to override sass variables in a separate file. Using css variabels make it possible again, even easy.

Test Steps

  • There should be no change is site styling.

@frjo frjo force-pushed the enhancement/css-variables branch from 9cbb6ee to 1a2f337 Compare December 7, 2024 17:31
@theskumar
Copy link
Member

With the SCSS variables the compiler would have thrown in error, I am thinking this might might make the SCSS code less reliable and prone to error. With tailwind classnames and SCSS variables, we get auto-completion and intellisense, here someone would have remember of all the variables.

My take would be to define all the variables in static_src/tailwind/base/variables.css and then create SCSS variables using those css variables.

Keeps the variables at a single place, avoids big refactor.

I think we can remove a lot these variables, but using TailwindCSS classes as well. e.g. milli ~ text-xs. But that could come in bit later.

NOTE: While the css variables are defined in the base/variables.css it's there only to allow for easier overrides. These variables are then converted to TailwindCSS tokens, then used in the tailwind generated classes.

@theskumar
Copy link
Member

theskumar commented Dec 7, 2024

In the /tailwind/base/variables.css, we can use the theme() function to pull in the font weights, dimension units, etc. https://tailwindcss.com/docs/functions-and-directives#theme

e..g

:root {
  --font-bold: theme("font.bold.weight")
  --gutters: theme("spacing.4")
}

@frjo frjo force-pushed the enhancement/css-variables branch from 8490949 to 3f8a0e3 Compare December 7, 2024 19:00
@frjo frjo force-pushed the enhancement/css-variables branch from 890c77e to 61af24e Compare December 7, 2024 21:28
@frjo frjo force-pushed the enhancement/css-variables branch from 469d74c to d30d84d Compare December 7, 2024 21:40
@frjo frjo force-pushed the enhancement/css-variables branch 2 times, most recently from c407bc7 to c441647 Compare December 7, 2024 22:13
@frjo frjo force-pushed the enhancement/css-variables branch from c441647 to 35b137e Compare December 7, 2024 22:14
@frjo
Copy link
Member Author

frjo commented Dec 8, 2024

Good points. I'm now setting the sass variables from css variables that are specified in hypha/static_src/tailwind/base/variables.css.

This will allow people to override almost all settings by setting a css variable in hypha/static_src/sass/custom/_custom.scss.

Most of the css variables are colours. I notice that we use the colours directly, e.g. --color-lightest-blue in many places. I think it would be better to set up more variables like --color-primary: var(--color-light-blue); and use these throughout Hypha.

Many other things we can do to improve styling of Hypha, this is one step.

@frjo frjo added Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Patch Mini change, used in release drafter labels Dec 8, 2024
@frjo frjo changed the title Replace use of sass variables with css variables Replace use of sass variables with css variables to allow easy overrides Dec 8, 2024
@frjo frjo changed the title Replace use of sass variables with css variables to allow easy overrides Set sass variables with css variables to allow easy overrides Dec 8, 2024
@frjo frjo added Status: Needs testing Tickets that need testing/qa Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Dec 9, 2024
@wes-otf wes-otf added Status: Tested - approved for live ✅ and removed Status: Needs testing Tickets that need testing/qa Status: Needs dev testing 🧑‍💻 Tasks that should be tested by the dev team labels Dec 13, 2024
@frjo frjo merged commit 46d201f into main Dec 13, 2024
7 checks passed
@theskumar theskumar deleted the enhancement/css-variables branch March 11, 2025 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Tested - approved for live ✅ Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Patch Mini change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert sass variables in to css variables when appropriate
3 participants