Improvement: use css custom properties instead of scss variables for themeing

[RikuVan]

Presently Attactions requires a dependency on Sass for overriding variables. Personally I have found this troublesome since I am not really interested in using SASS otherwise. I recently tried to change the build and this was immediately a blocking issue. If Attractions used custom css properties (variables) for overrides (which is compatible with SASS), this would remove some friction for users. More info here: https://sass-lang.com/documentation/breaking-changes/css-vars.

[illright]

There are several reasons why Attractions is currently using SCSS:

1. I personally had doubts about bringing in CSS variables because this would cut the browser compatibility. Admittedly, this is not a large cut at all, moreover, I suspect that in some cases there might be other things that render the library incompatible with older browsers such as new JS syntax that might be untranspiled or something like that.

2. Attractions also makes use of some very handy Sass features such as style nesting and color computation. Switching to pure CSS would mean messier stylesheets and just plainly lack of some features like proper hover colors and so on.

The first point is not a deal-breaker, I suppose we could switch the library to use CSS variables and still be highly supported by browsers. The second point, however, is a present matter of discussion in the Svelte community, especially with desirable things like TypeScript. Currently there is no sensible way of shipping preprocessed Svelte components, however, there are claims that it should be taken care of in the upcoming SvelteKit. They say that the SvelteKit isn't too far out, so perhaps our best bet would be to wait for its release and then take the steps to ship preprocessed Svelte source.

Until then, however, I suppose we have to force users to use SCSS. Can you elaborate more on why you think it's troublesome? I understand it's an extra step in installation and a worthless devDependency, but other than that, is there anything we could do in the library that would make installing SCSS less painful? Open to any suggestions

[aabounegm]

About the very last sentence, I suppose we could make it a direct dependency instead of a dev one, but that only saves users the trouble of installing it manually, not of configuring it (which is more inconvenient, imo). Perhaps we can also depend on svelte-preprocess and expose an object with the preprocessing configuration already written for those who don't want to customize the variables?

[RikuVan]

I realize my original request was not clear. There are at least three possibilities (probably more): 1. removing sass completely, 2. removing the scss from components and adding a build step so only pure css is delivered with components 3. retaining sass but allowing users to override style with css variables.

I realize choice #1 is unreasonable--a big refactor. I have used sass for years and I understand it still has a big following and some libraries may depend on it (I stopped using it a few years ago). Choice #2 is probably onerous too since it would require a less major but still big refactor and a new build step. But option #3 seems reasonable to me. So internally the library relies on scss and the svelte plugin is also required. But you can have scss variables which are also css variables and can be overridden in pure css. I have done this recently in a work project for flexibility in how the style tokens are consumed. When trying to upgrade the build it was the _attractions-theme.scss dependency that gave me problems. When I tried to upgrade to a new version of rollup and other plugins, the scss build stopped working and I have yet to resolve the problem. Since I am not otherwise using scss in this project, it would be just so much easier if I could define my own :root with any variables I need to override. This would also allow me to reuse these same variables throughout my project without having to adopt Sass. It is true css vars do not work in IE11 if that is really an aim (although there are plugins to deal with this) but otherwise they are broadly supported and I use in them in public projects in Finland.

You mentioned Typescript and this I think is a similar issue. I use Typescript whenever possible but I understand why you would not want to force it on users--until there is a better story for extracting types from Svelte components. I think the Sass is a bit like Typescript case; it is a dependency some may not have wanted and while it is fine if it used internally it is less fun when it leaks out into the consuming application code.

Finally, this is just an idea and I understand there are some good reasons why the suggestion might be wrong for you.

[RikuVan]

btw the original reason I started thinking about this is that the advice for how to include _attractions-theme.scss worked at the outside but I started getting a _attractions-theme.scss not found error when upgrading rollup and the svelte preprocess plugin. Have you guys used the same technique for the latest versions of these libraries?

[aabounegm]

Your suggestions are highly appreciated and will be taken into account, but I want to direct your attention to the point mentioned earlier by @illright: we use Sass functionality that would otherwise cause the stylesheet to be too complicated and convoluted, such as the color functions.
The reason your 3rd suggestion will not work is that Sass and CSS variables cannot work together for reasons outlined in Sass's variables docs (check the "Heads up!" notice). Put shortly, Sass variables are resolved in compile-time, while CSS ones are in runtime.
Our main problem now, as @illright explained earlier, is that the Svelte convention is to ship the unprocessed source code. Compare that to Vue's approach, which allows you to write whatever you want in the source code and ship the compiled version instead. Hopefully, this gets resolved with SvelteKit.
I should also mention that you can always use the compiled version of the library and not have to install Sass or PostCSS, but this approach will limit the customizability.

[aabounegm]

Actually, now that I think about it again, I don't think this can be resolved with the Vue style of shipping components, since the Sass variables will still be pre-compiled and hence not customizable.
What I am thinking now is that we can rewrite the Sass in such a way that all usages of variables can be replaced with CSS variables that are given a default value from Sass.
To illustrate what I mean, consider the following line:

attractions/attractions/button/button.scss

Line 62 in babfe1e

background: linear-gradient(0deg, $main 0%, lighten($main, 5%) 100%);

In Sass, you can just change $main and it will change this background color accordingly. My suggestion is to rewrite it like this:

$main: #4300b0;

:root {
  --main: #{$main};
  --main-background-gradient: #{linear-gradient(0deg, $main 0%, lighten($main, 5%) 100%)};
}

and replace the line in the button's scss with

    background: var(--main-background-gradient);

If you use Sass, you will just override the $main variable and everything will work as expected. If you want to use just CSS, you will need to be aware that other variables may need to be changed if they depend on the one you are replacing.
I see this as the only sensible workaround that leaves Sass users with what they expected before from the library, and gives CSS users the ability to use the compiled version without having to install Sass/PostCSS with just the added difficulty of having to change more variables.

What do you think?

[RikuVan]

Yes, this is just what I was suggesting!

…

On Mon, Dec 14, 2020 at 1:04 PM Abdelrahman Aly Abounegm < ***@***.***> wrote: Actually, now that I think about it again, I don't think this can be resolved with the Vue style of shipping components, since the Sass variables will still be pre-compiled and hence not customizable. What I am thinking now is that we can rewrite the Sass in such a way that all usages of variables can be replaced with CSS variables that are given a default value from Sass. To illustrate what I mean, consider the following line: https://github.com/illright/attractions/blob/babfe1eb4ede72c0d8687bcc039743a106243caa/attractions/button/button.scss#L62 In Sass, you can just change $main and it will change this background color accordingly. My suggestion is to rewrite it like this: $main: #4300b0; :root { --main: #{$main}; --main-background-gradient: #{linear-gradient(0deg, $main 0%, lighten($main, 5%) 100%)}; } and replace the line in the button's scss with background: var(--main-background-gradient); If you use Sass, you will just override the $main variable and everything will work as expected. If you want to use just CSS, you will need to be aware that other variables may need to be changed if they depend on the one you are replacing. I see this as the only sensible workaround that leaves Sass users with what they expected before from the library, and gives CSS users the ability to use the compiled version without having to install Sass/PostCSS with just the added difficulty of having to change more variables. What do you think? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#181 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABX2SC2DBY7MQH3EGLYR7PTSUXWL3ANCNFSM4UXHQJDQ> .

[RikuVan]

The only issue, even with the change you suggest, is that ithe scss variables are still imported into each component now e.g. @import ' _attractions-theme.scss', So you will still get an error if you don't somehow include this file in your project. Is it possible to make it unnecessar to provide this file? On Mon, Dec 14, 2020 at 1:32 PM Richard Van Camp <richard.vancamp@gmail.com> wrote:

…

Yes, this is just what I was suggesting! On Mon, Dec 14, 2020 at 1:04 PM Abdelrahman Aly Abounegm < ***@***.***> wrote: > Actually, now that I think about it again, I don't think this can be > resolved with the Vue style of shipping components, since the Sass > variables will still be pre-compiled and hence not customizable. > What I am thinking now is that we can rewrite the Sass in such a way that > all usages of variables can be replaced with CSS variables that are given a > default value from Sass. > To illustrate what I mean, consider the following line: > > https://github.com/illright/attractions/blob/babfe1eb4ede72c0d8687bcc039743a106243caa/attractions/button/button.scss#L62 > In Sass, you can just change $main and it will change this background > color accordingly. My suggestion is to rewrite it like this: > > $main: #4300b0; > :root { > --main: #{$main}; > --main-background-gradient: #{linear-gradient(0deg, $main 0%, lighten($main, 5%) 100%)}; > } > > and replace the line in the button's scss with > > background: var(--main-background-gradient); > > If you use Sass, you will just override the $main variable and > everything will work as expected. If you want to use just CSS, you will > need to be aware that other variables may need to be changed if they depend > on the one you are replacing. > I see this as the only sensible workaround that leaves Sass users with > what they expected before from the library, and gives CSS users the ability > to use the compiled version without having to install Sass/PostCSS with > just the added difficulty of having to change more variables. > > What do you think? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#181 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/ABX2SC2DBY7MQH3EGLYR7PTSUXWL3ANCNFSM4UXHQJDQ> > . >

[aabounegm]

Unfortunately, Sass doesn't support optional imports (afaik), and this import is needed for overriding the default values. What you can do instead is use

autoPreprocess({
  scss: {
    includePaths: ['node_modules/attractions'],
  },
})

in your rollup config, as we have an empty _attractions-theme.scss there to overcome this issue while building the compiled versions (but I can't guarantee that this will always remain the case).

[illright]

Finally I get to catch up with this thread.

@RikuVan you mentioned you had build issues after upgrading Rollup and svelte-preprocess.
We have a project which uses Attractions, that project has Rollup 2.34.1 (but works with the latest 2.35.1, just checked) and svelte-preprocess 4.6.1 (latest), it builds just fine. What sort of issue are you having exactly?

@aabounegm making svelte-preprocess and the like a direct dependency is a rather questionable decision since it's not required if you're using a compiled bundle of Attractions outside of Svelte. I understand it's not a common usecase, but installing a few devDeps is also not that much pressure on users. As for exposing the configuration object, this is something I'm working on right now (due to the new system of Sass modules that we should migrate to, but I can't get it to work just yet).

@RikuVan @aabounegm I like the idea of using Sass to populate the values for CSS variables so that they could be overriden both at build time and at run time. Moreover, this allows for restyling the library in specific parts of the website, so making the switch to CSS variables is rather attractive (pun intended).

@RikuVan yes, Sass doesn't support optional imports which is a big sad in my opinion, especially with this new module system that they introduced that will "revolutionize" the world. However, digging around it more, I've discovered that Sass supports custom import resolvers, which is enough. So at the cost of writing a couple more lines of code in rollup.config.js we free ourselves from the shackles of _attractions-theme.scss. Of course we could write these lines in the docs for our users to copy-paste, but I'm thinking we can simply expose an object with SCSS properties to pass to svelte-preprocess. But that will probably only come when I figure out the Sass modules and put them in place. Stay tunezz.

[RikuVan]

Finally I get to catch up with this thread.

@RikuVan you mentioned you had build issues after upgrading Rollup and svelte-preprocess.
We have a project which uses Attractions, that project has Rollup 2.34.1 (but works with the latest 2.35.1, just checked) and svelte-preprocess 4.6.1 (latest), it builds just fine. What sort of issue are you having exactly?

I will have another go at this and get back to you with a better report on the errors--should be possible to find my way around it. Alternatively I might give snowpack a try just to confuse things further :).

[RikuVan]

Version 3 looks like a nice improvement in regard to Sass and overriding variables! Thanks. Will have a go at upgrading in the next week or two.

[RikuVan]

I updated and everything works. The reason I ran into issues the first time I did this is that rollup-plugin-svelte version 7 had been released and I didn't immediately get that there had been a major change regarding css, so I was getting errors. Before the svelte plugin would write the css to a file for you. But now you probably need to use rollup-plugin-scss or something similar, to write the css to a file. Dunno, might be helpful to have this information in the attraction docs as well.