Move all colours to CSS variables

[bonanitech]

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Proposed changes

I know you were not keen on getting this in for 3.0, but I had it done and thought it best to get it PR'd ready for when you are.

I changed all colors (SASS variables) to CSS variables, except for a few hard-coded colors in editor.scss.

I wish I had found a better solution for the colors in .red-ui-tabs-fade. It works, but it's not an elegant solution.

Please review the changes in this PR and let me know if there is anything else to do.

Checklist

• I have read the contribution guidelines
• For non-bugfix PRs, I have discussed this change on the forum/slack team.
• I have run grunt to verify the unit tests pass
• I have added suitable unit tests to cover the new/changed functionality

[coveralls]

Coverage remained the same at 68.282% when pulling 9b4d3ad on bonanitech:use-css-variables into 95f7ea9 on node-red:master.

[Steve-Mcl]

@bonanitech I am in the process or reviewing this PR - are there any more changes expected from you?

[bonanitech]

No more changes planned to this PR

Only if you find issues during the review 🙂

[Steve-Mcl]

Hi @bonanitech - looks good.

Regarding ...

I wish I had found a better solution for the colors in .red-ui-tabs-fade. It works, but it's not an elegant solution.

I guess you are talking about the linear-gradients where you generated additional alpha vars? to do this with pure CSS you need to do the following...

:root {
    ----red-ui-tab-background-inactive:  240, 240, 240;
}
.class-name {
    background-image: linear-gradient(to right, rgba(var(--red-ui-tab-background-inactive), 0.1), rgba(var(--red-ui-tab-background-inactive), 1));
}

however, since 240, 240, 240 cannot be used as a color var directly (it would have to be wrapped in rgb(var(--red-ui-tab-background-inactive)) so as far as I am concerned, either solution is fine.

Regarding ...

I changed all colors (SASS variables) to CSS variables, except for a few hard-coded colors in editor.scss.

These are what they are and will be corrected at some time regardless of this work (i.e. they are not really of consequence)

Lastly, I have tested a theme (forge-light) and even though it has not been changed to a simple bunch of :root var values, it still works (meaning this change to NR is backwards compatible with existing themes)

@knolleary - i am happy with a first pass and local testing.
And it is sooo nice to be able to simply tweak a CSS var and see what the theme looks like 😄

[Steve-Mcl]

Functionally, this works.
Some consolidation of variables might be good but that can be iterated.

[bonanitech]

Thanks for the feedback @Steve-Mcl

however, since 240, 240, 240 cannot be used as a color var directly (it would have to be wrapped in rgb(var(--red-ui-tab-background-inactive)) so as far as I am concerned, either solution is fine.

I tried this solution too but found it just as ugly as the one I used. Although I think I should make a small change to keep those variables kind of in sync.

$tab-background-active: $secondary-background;
- $tab-background-active-alpha: rgba($secondary-background, 0.001);
+ $tab-background-active-alpha: rgba($tab-background-active, 0.001);
...

[bonanitech]

Some consolidation of variables might be good but that can be iterated.

What do you suggest?

[Steve-Mcl]

Some consolidation of variables might be good but that can be iterated.

What do you suggest

For a later time. All good for now.

[knolleary]

There are a couple merge conflicts to resolve before we can get this in.

[bonanitech]

There are a couple merge conflicts to resolve before we can get this in.

Those conflicts are resolved. I also added a small commit to the PR. Please review it again if possible.

[Steve-Mcl]

Mauricio, I have been running this today. Functionally, it works and I don't see any graphical artefacts. So as far as I'm concerned, it passes.