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

fix(theme): Example of runtime TextField theming fix #3145

Conversation

jamesmfriedman
Copy link
Contributor

This PR is intended to be more of an example than anything, but if the contributors are ok with this approach, I'd be happy to go ahead and apply this to the whole codebase.

The issue

image

This is an example of setting red as the mdc-theme-primary color at runtime, and the resulting mdc-floating-label still using the default purple theme color.

The short version

Anytime the mdc-theme-prop-value function is used in SASS, the value is being hardcoded into the CSS which means you can never override it with the runtime css variables. The issue is compounded by using the rgba function to add opacity to the color.

Proposed fixes / Ideas

  • Don't use that function
  • Handle dynamic opacity using standard opacity OR define the different percent color stops as runtime CSS Vars. Both have their trade offs.
  • A slightly crazier idea would be to just define R, G, and B as numeric variables. This would allow you to do something like this during runtime. This still results in a simple mdc-theme-primary variable consumers can use, and allow for dynamic opacity.
    image

Why don't you just recompile the SASS?

I have apps that use themes in a variety of ways

  • One app has a light and a dark mode in the UI. Using RMWC ThemeProvider I just set the new variables and everything works like magic
  • I have another app where the theme is applied dynamically based on their brand colors.
  • The runtime theming stuff is too magical to pass up. I'd love to just get it working across the board as expected.

@kfranqueiro
Copy link
Contributor

Cross-linking #3066 which is somewhat related

@kennylevinsen
Copy link

The "crazier" fix would look like:

:root {
    --mdc-theme-primary-r: 255;
    --mdc-theme-primary-g: 255;
    --mdc-theme-primary-b: 0;
}
.something-using-the-primary-theme-color {
    background-color: rgba(var(--mdc-theme-primary-r), var(--mdc-theme-primary-g), var(--mdc-theme-primary-b), 0.87);
}

This could be done considerably less verbose, while still maintaining the same level of flexibility:

:root {
    --mdc-theme-primary: 255, 255, 0;
}
.something-using-the-primary-theme-color {
    background-color: rgba(var(--mdc-theme-primary), 0.87);
}

An example codepen, showing that the alpha channel is correctly applied: https://codepen.io/anon/pen/WKRgyy

@jamesmfriedman
Copy link
Contributor Author

That’s awesome. I didn’t realize you could store arbitrary coma separated values like that in a variable!

@jamesmfriedman
Copy link
Contributor Author

I'm going to go ahead and close this out since it was never intended to be merged and was more of an idea exploration.

With that said, the proposal above seems like it would be a marked improvement to the theming framework and allow you to dynamically mix colors at runtime instead of relying on SASS to hardcode them in.

Personally, I have CSS overrides in every single project to correct the above mentioned issues (like textfield label). It's not ideal, but it works and is maintainable.

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.

3 participants