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

Replace Sass variables with CSS custom properties #21656

Open
robintown opened this issue Apr 3, 2022 · 15 comments
Open

Replace Sass variables with CSS custom properties #21656

robintown opened this issue Apr 3, 2022 · 15 comments
Labels
A-Developer-Experience A-Performance A-Themes-Custom Custom theme variables or support A-Themes-Official Official themes (light, dark) O-Uncommon Most users are unlikely to come across this or unexpected workflow T-Enhancement Z-Labs

Comments

@robintown
Copy link
Member

Your use case

What would you like to do?

Replace all Sass variables with CSS custom properties.

Why would you like to do it?

  • Improve the performance of theme switching
  • Improve CSS compilation times
  • See actual variable names when inspecting the page instead of hardcoded values
  • Potentially simplify the custom themes implementation?

How would you like to achieve it?

Separate our theme files from the rest of the CSS so that they can be swapped out easily.

Have you considered any alternatives?

Not doing this if we have a good reason to continue using preprocessor variables.

Additional context

https://caniuse.com/css-variables

@robintown robintown added A-Performance T-Enhancement A-Themes-Custom Custom theme variables or support A-Themes-Official Official themes (light, dark) A-Developer-Experience O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Apr 3, 2022
@github-actions github-actions bot added the Z-Labs label Apr 3, 2022
@SimonBrandner
Copy link
Contributor

SimonBrandner commented Apr 3, 2022

Yes, this would make themes a lot less painful, I think. Though we might need to clean up the themes before we get to this as currently, we have a bunch of unnecessary colour variables that we would need to deal with. I am sort of in the progress of reducing the number of colour variables we use but I am stuck on design review/input...

See #19254

@EECvision
Copy link

Hi @SimonBrandner, I am an outreachy applicant. Can you guide me on how to work on this issue? I have been working on an issue that is related to the UI theme switching see #19514. I think that working on this issue would enable me to fix the UI theme switching issue.

@goelesha
Copy link

goelesha commented Apr 4, 2022

Hi @SimonBrandner @robintown. Can you elaborate more on the issue?
I am interested in working on this issue.

@germain-gg
Copy link
Contributor

@goelesha I would not particularly recommend someone new to Element to work on this as there are a lot of undocumented things around themes and how Element is built that one will face when working on this

@goelesha
Copy link

goelesha commented Apr 4, 2022

@goelesha I would not particularly recommend someone new to Element to work on this as there are a lot of undocumented things around themes and how Element is built that one will face when working on this

I get it. I will look for some other issues to work on. Can you guide me on how can I get acquainted with the functioning of themes so that I am able to take up such issues in future?

@EECvision

This comment was marked as resolved.

@robintown
Copy link
Member Author

Another reason why this would be nice: sometimes design wants select parts of the UI to always use the dark theme colors, so with CSS variables we could keep both themes loaded with minimal overhead, and then just apply a dark class to those parts of the UI.

@t3chguy
Copy link
Member

t3chguy commented Apr 12, 2022

@robintown only if we make themes all include both a light & dark variant but right now custom themes are not such and neither is the high contrast theme.

@germain-gg
Copy link
Contributor

germain-gg commented Apr 21, 2023

The future of this, will be generated by the vector-im/compound-design-tokens repo.

It outputs some CSS custom properties, and addresses most of the comment made in this conversation.
I do not anticipate we'll convert the SCSS variables, but slowly phase them out as we refactor the UI layer of Element Web

@luixxiul
Copy link

When will that Compound stuff available approximately? In the Q2 of this year, Q3 or Q4? A hint would be appreciated, whether it is official or not.

@germain-gg
Copy link
Contributor

It already exists, https://github.com/vector-im/compound-design-tokens
We are creating the semantic tokens as design decisions are made. I would not advocate to go and do a search and replace of SCSS variables to change them with the tokens listed above.

We will started using them in the Compound components https://github.com/vector-im/compound-web and then start replacing UI parts in Element with the components and organically grow their usage.

@luixxiul
Copy link

I am wondering if the design decision making process or its result would be publicly available as documentation or something, if it does not happen on vector-im's repository.

I am asking this because I am afraid that a community member's PR would be rejected because it did not follow the design spec, though the existence of the spec was not well shared due to information asymmetry.

@luixxiul
Copy link

If replacing Sass variables will not happen, let's just close the issue to clarify the situation.

@germain-gg
Copy link
Contributor

The tokens themselves are the results of design decisions.

While we're bootstrapping this project community contributions can happen but we want to also get confidence in our approach so we can promote consistent way of achieving things.
Everything will be documented as part of compound.element.io.

If replacing Sass variables will not happen, let's just close the issue to clarify the situation.

Don't get me wrong, it will happen. I just don't advocate for community contributors to go ahead and start replacing values at this stage. We don't think we're quite ready for that yet.

@luixxiul
Copy link

Don't get me wrong, it will happen.

Right, I did not notice the line below on #21656 (comment):

The future of this, will be generated by the vector-im/compound-design-tokens repo.

We don't think we're quite ready for that yet.

It would be appreciated if you or someone who is an employee would announce to the community some way when it gets ready. Updating documentation would be fine too. Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Developer-Experience A-Performance A-Themes-Custom Custom theme variables or support A-Themes-Official Official themes (light, dark) O-Uncommon Most users are unlikely to come across this or unexpected workflow T-Enhancement Z-Labs
Projects
None yet
Development

No branches or pull requests

7 participants