nextcloud / server

☁️ Nextcloud server, a safe home for all your data
https://nextcloud.com
GNU Affero General Public License v3.0
26.63k stars 3.99k forks source link

Scoping issue when overriding CSS variables for user-themes #36303

Open mspae opened 1 year ago

mspae commented 1 year ago

To prevent confusion, here a note on the nomenclature I use: "user theme" → theme enabled by the individual user in their personal settings "nextcloud theme" → instance-wide design customization which is enabled by the administrator

TL;DR I propose changing the element the user theme data attributes are set to the root html element to solve scoping issues.

I'm developing a nextcloud theme. It should work with the inbuilt user themes (light, dark, light-highcontrast, dark-highcontrast).

In order to do this I am using a SASS build-process to reduce the amount of duplicate code and still catch all the possible edge-cases. (personal user account settings vs. inbuilt browser settings/media queries).

Example SASS mixin for theming:

@mixin dark-mode {
  @media (prefers-color-scheme: dark) {
    body:not([data-theme-light], [data-theme-light-hightcontrast]) {
      // browser says this is dark mode, nextcloud user settings do not disagree
      @content;
    }
  }
  body[data-theme-dark],
  body[data-theme-dark-highcontrast] {
    // nextcloud user settings say this should be dark mode
    @content;
  }
}

As you can see I'm using the body attribute selector to catch the user theme-preference (e.g. body[data-theme-dark]). This is sub optimal since this limits the customization to elements within the body scope. (For example the html tag has a background color, this is visible when the browser window is resized).

I propose the theme attributes are set on the html to fix this problem.

The code which would need to be adjusted lies in https://github.com/nextcloud/server/blob/master/apps/theming/src/UserThemes.vue#L240

As I have no deeper knowledge of the nextcloud code base I cannot foresee which problems/conflicts might arise from such a change.

susnux commented 1 year ago

See: https://github.com/nextcloud/server/pull/36462