[go: up one dir, main page]

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

✨ Update script to build scss files along css files #19

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

chtushar
Copy link
@chtushar chtushar commented Dec 11, 2021

This PR will rename scripts/build-css-modules.js to scripts/build-stylesheets.js and update the script to export scss files along with css files.

Fix

Fixes #18

Checklist:

  • Give a generic name to the script file build-css-modules.
  • Update code to generate scss files.
  • Update the build script.
  • Check if files are being build successfully.

@chtushar chtushar changed the title 🚧 Update script to build scss files along css files ✨ Update script to build scss files along css files Dec 12, 2021
@chtushar chtushar marked this pull request as ready for review December 12, 2021 09:10
Copy link
Contributor
@vladmoroz vladmoroz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been a good while since I used SCSS – are :root scoped variables a thing in SCSS? I expected they need to be under the global scope.

image

I tried this on sassmeister.com and got an error

image

@chtushar
Copy link
Author
chtushar commented Dec 13, 2021

Fixing this🔩.. It didn't appear on my vim setup.

@chtushar
Copy link
Author

@vladmoroz Fixed it. Please do have a look.

Comment on lines +25 to +27
const scaleAsScssFile = isDarkScale
? `.dark-theme {\n${scaleAsScssProperties}\n}`
: scaleAsScssProperties;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you expect to use the dark theme values with Sass? If I understand correctly, Sass outputs compiled values only, so .dark-theme scope would have the same problem that :root scope had.

With vanilla CSS variables, we provide .dark-theme class name to facilitate automatic theme changes. This way you would put .dark-theme on html or body element to toggle the theme globally.

It seems that in Sass you need to output these variables into global scope too and handle theming manually?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was just trying out Radix Colors and tried to @import the CSS files into my SCSS setup and I ran into this problem exactly - applying .dark-theme to body has no effect.

Promoting .dark-theme to :global scope in the exported SCSS files solves it for me. It behaves the same way as the CSS version afaict.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that in Sass you need to output these variables into global scope too and handle theming manually?

you can do both, continue to use CSS custom props referencing sass vars for the automatic theming and also use sass vars elsewhere

$root: ":root" !default;
$gray1: hsl(0, 0%, 99.0%) !default;
#{$root} {
  --gray1: #{$gray1};
}

and similar for the dark

$root: ".dark-theme" !default;
$gray1: hsl(0, 0%, 8.5%) !default;
#{$root} {
  --gray1: #{$gray1};
}

@eugenesvk
Copy link

I think the commit below resolves the theming issue as it allows you to import the identical-to-css data with css custom props you can continue to dynamically use based on class selector, BUT it also allows you to:

  • change the scope name from dark-theme and also add custom scope to light theme
  • use sass variables separately from the light/dark theme css custom props
  • override any specific color

For example, these imports allows overriding gray4 color and use :root.light for the light theme and :root:not(.light) for the dark one

@use "path/to/radix-ui-colors/gray"    	with ($root:":root.light"      , $gray4:red)
@use "path/to/radix-ui-colors/grayDark"	with ($root:":root:not(.light)", $gray4:red)

eugenesvk@3fded58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

✨ Feature Request: Add script for building scss/sass modules
4 participants