Skip to content

fix(overrides): give brand colour overrides the dark scopes, so every dark user sees one colour - #705

Merged
rubenvdlinde merged 1 commit into
developmentfrom
fix/698-overrides-reach-explicit-themes
Sep 28, 2026
Merged

rubenvdlinde merged 1 commit into
developmentfrom
fix/698-overrides-reach-explicit-themes

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

A colour an admin set in the token editor showed for users on "System default" with a dark OS, but not for a user who picked the dark theme, so two dark users saw different colours.

The defect

CustomOverridesService wrote every override into one :root {} block with !important. For a user who chose a theme, Nextcloud serves that theme's variables scoped to [data-theme-dark] and puts that attribute on body, so body and everything in it used core's value and never inherited the one on :root; !important only wins between declarations on the same element. For "System default", core serves its dark values on :root inside a media query, where the override does win.

The tests that were red

tests/Unit/Service/CustomOverridesServiceDarkScopesTest.php, with the real DarkPaletteService: testDarkScopesWritten and testAnUnparseableColourKeepsItsValueInBothDarkScopes failed on development (no dark block at all). testLightValuesReadBack, testComponentTokensGetNoDarkCopy and testAnExportedFileReadsBackItsLightValues guard the rest.

The fix

  • For every brand-layer colour override (Nextcloud's own variables), custom-overrides.css now also carries the two dark scopes the generated dark stylesheets use: the prefers-color-scheme: dark block on the untouched body, and body[data-theme-dark], body[data-themes*=dark]. Both get the same value, so both kinds of dark user see one colour, and a user who chose the light theme is not affected.
  • That value is what the dark palette derives, through a new public DarkPaletteService::deriveDarkValue() that the generator itself now uses, so the editor and the generated stylesheets never disagree. A value that is not a colour literal keeps its light value in both scopes.
  • Component tokens get no dark copy. Both kinds of dark user already get the same body value for them from the generated dark stylesheet, and a body-level copy here would outrank the primary-lock layer, which locks them at :root.
  • The override import and the config bundle import read the :root block alone, through CssParserService::parseOverridesFile(), so the dark copies in an exported file do not overwrite the light values. ConfigBundleServiceTest::testRoundTripExportWipeImportRestoresIdenticalState caught exactly that while this was built.

A behaviour change to know about: a dark user on "System default" used to see the light override value itself; now every dark user sees its derived dark value, as authoring-token-value-types requires. The admin's own dark value per colour (the second "Dark" field and darkOverrides, tasks 2.2 and 2.3 in full) is not part of this change; this writes the derived value.

Verified

  • composer check:strict: exit 0 (its test:all skips in a bare clone, so PHPUnit ran separately).
  • PHPUnit with the server's lib/private on the autoloader: 875 tests, the same 11 errors and 7 failures as on development (missing Symfony and server classes outside a Nextcloud tree).
  • npm run lint, format, test:l10n, check:l10n-js, check:manifest, test:unit: all exit 0.
  • Hydra gates --scope-to-diff: exit 0.

Fixes #698

… dark user sees one colour

Every override sat in one :root block with !important. Nextcloud declares
a chosen dark theme's colours on body, which a :root value never reaches,
so a user who picked Dark theme kept core's colour while a user on System
default with a dark OS saw the override. custom-overrides.css now also
carries the two dark scopes of the generated dark stylesheets for every
brand-layer colour override, with the value DarkPaletteService derives
(the light value when it is not a colour literal). Component tokens are
left out: both dark users already agree on them, and a body copy would
outrank the primary-lock layer.

The override import and the config bundle import read the :root block
alone, so the dark copies in an exported file do not overwrite the light
values.

Fixes #698
@rubenvdlinde
rubenvdlinde merged commit 64a0918 into development Sep 28, 2026
32 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/thematiq @ 4a8a516

Check PHP Vue Security License Tests
lint ✅
phpcs ✅
phpmd ✅
psalm ✅
phpstan ✅
phpmetrics ✅
eslint ✅
stylelint ✅
build ✅
check-manifest ✅
test-l10n ✅
format ✅
composer ✅ ✅ 107/107
npm ✅ ✅ 2/2
app:check-code ⏭️
info.xml ✅
REUSE ✅
lockfile sync ✅
PHPUnit ✅
Newman ✅
Playwright ⏭️ deferred: E2E runs locally and on the promotion path only. This pull request targets development, so the suite is asked once per promotion into beta and main rather than once per push per open pull request. Run it on any branch from the Actions tab, or locally with npx playwright test.
Hydra gates ✅

Quality workflow — 2026-09-28 15:40 UTC

Download the full PDF report from the workflow artifacts.

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.

1 participant