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 theming: Theme specific variables must be set on the root node #36461

Closed
wants to merge 1 commit into from
Closed

Fix theming: Theme specific variables must be set on the root node #36461

wants to merge 1 commit into from

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jan 31, 2023

Summary

Currently if you use a css variable which links to an other css variable, like --color-main-background and a theme is enabled, the theme is not used for the variable. See linked issue (same e.g. on forms, see nextcloud/forms#1471 (review)).

The problem is the css variable evaluation order, e.g. consider the browser requests dark color theme but the user selected the bright nextcloud theme, then:

  • HTML tag
    • defines --color-main-background for dark color theme (as requested by the browser)
    • defines --table-color-background to be the same as --color-main-background
  • Body tag
    • redefines --color-main-background to match the theme (because data-theme-light is set on the body tag)
  • Table tag
    • Uses --table-color-background which evaluates to HTML-Tag::--color-main-background and in the context it is set to dark instead of bright.

So the solution is to set data-theme-light on the HTML tag rather than the body tag.

Screenshots

System uses dark color theme, but nextcloud is configured to use the bright theme.
You can see the NcSelect has the wrong background color:

before (currently) after (with this PR)
dark background on bright color theme, hard to read correct bright background on bright color theme, good to read

Checklist

Signed-off-by: Ferdinand Thiessen <rpm@fthiessen.de>
@susnux
Copy link
Contributor Author

susnux commented Jan 31, 2023

Superseded by #36462

@susnux susnux closed this Jan 31, 2023
@susnux susnux deleted the fix/theming-variables branch January 31, 2023 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Black table on light theme
1 participant