Skip to content

Custom Accent support - #35096

Open
Raushen wants to merge 1 commit into
DevExpress:feature/26_2_new_fluent_theme_with_design_tokensfrom
Raushen:feature/accent_color
Open

Custom Accent support#35096
Raushen wants to merge 1 commit into
DevExpress:feature/26_2_new_fluent_theme_with_design_tokensfrom
Raushen:feature/accent_color

Conversation

@Raushen

@Raushen Raushen commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@Raushen
Raushen requested a review from a team September 8, 2026 09:34
@Raushen Raushen self-assigned this Sep 8, 2026
@Raushen Raushen added the 26_2 label Sep 8, 2026
@Raushen Raushen closed this Sep 8, 2026
@Raushen Raushen reopened this Sep 8, 2026

@EugeniyKiyashko EugeniyKiyashko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseded by the review below (same content, reworded); inline comments removed.

@EugeniyKiyashko EugeniyKiyashko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Superseded by the review below (same content, reworded); inline comments removed.

@EugeniyKiyashko EugeniyKiyashko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I built the theme from this branch and opened it in Chromium 148, Chrome 149, Chrome 152 and Firefox 155, and I ran the colour formula against all 11 designed palettes. No objections to the approach: it is the same as in Blazor (every accent stylesheet wraps each primary step in a fallback, the computed steps live on the root element, the same three settings), and it works. With no accent set the theme looks exactly as designed, a set colour recolours everything, and it degrades safely in browsers without relative colour syntax. It is actually safer than Blazor's version, which has neither the browser-support check nor the intermediate -source variable.

Cost per bundle (blue.light): 1 207 065 -> 1 211 954 bytes (+4.9 KB), gzipped 140 790 -> 141 226 (+0.4 KB). The calc() budget goes 359 -> 393 (17 computed steps, two expressions each).

The inline comments cover: the two edge steps that one browser build gets wrong, tuning the three numbers to our palette, the generator check, how the naming test should treat the new variables, and the unrelated project.json change. Three things not tied to a single line:

  1. Nothing in the repository ever sets --dx-accent-color. No unit test, no screenshot test, no playground page. Every green job ran with the accent switched off, which is exactly why the edge-step problem was not noticed. One screenshot of a widget with the accent set, in a fluent-next job, plus one unit test that checks the generated palette has 18 wrapped steps, would make the feature visible to CI.

  2. A way to set the colour from code. In Blazor the colour goes through SetCustomAccentColor(), so it is validated before it reaches CSS. Here the user writes the variable by hand, and a bad value (--dx-accent-color: foo) makes every primary-coloured surface transparent: the fallbacks only cover a missing variable, not a wrong one. A small helper in themes, say setAccentColor(value), that checks the value is a real colour and then sets it on the root element, closes this cheaply and gives the documentation one canonical recipe.

  3. Things the documentation should say (all measured on the built theme): the value must be a valid, opaque colour, because transparency is copied into all 18 steps; the variable only works on the root element, setting it on a nested block does nothing; a colour of medium lightness and reasonable saturation works best, very light, very dark or grey colours collapse one end of the palette; charts keep the default blue accent for now. Blazor's documentation says none of this, so ours would be the first.

Suggested priority. Before merge: a test that sets the accent, the two edge steps, the naming-test category instead of a bigger snapshot, and the project.json change moved out. Good to have: tuned numbers, the themes helper, the exact-count check in the generator. Documentation: the list above.

The red quarantine job is the popup drag test that also failed in quarantine on the base branch on 04.09 - unrelated to this PR.

--dx-accent-color-150: oklch(from var(--dx-accent-color-source) calc(l - 5 * (max(l, var(--dx-accent-lightness-min)) - var(--dx-accent-lightness-min)) / 8) calc(c - 5 * (max(c, var(--dx-accent-chroma-min)) - var(--dx-accent-chroma-min)) / 8) h);
--dx-accent-color-160: oklch(from var(--dx-accent-color-source) calc(l - 6 * (max(l, var(--dx-accent-lightness-min)) - var(--dx-accent-lightness-min)) / 8) calc(c - 6 * (max(c, var(--dx-accent-chroma-min)) - var(--dx-accent-chroma-min)) / 8) h);
--dx-accent-color-170: oklch(from var(--dx-accent-color-source) calc(l - 7 * (max(l, var(--dx-accent-lightness-min)) - var(--dx-accent-lightness-min)) / 8) calc(c - 7 * (max(c, var(--dx-accent-chroma-min)) - var(--dx-accent-chroma-min)) / 8) h);
--dx-accent-color-180: oklch(from var(--dx-accent-color-source) calc(l - 8 * (max(l, var(--dx-accent-lightness-min)) - var(--dx-accent-lightness-min)) / 8) calc(c - 8 * (max(c, var(--dx-accent-chroma-min)) - var(--dx-accent-chroma-min)) / 8) h);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lightest and the darkest steps come out wrong in one browser build. In Chromium 148, with --dx-accent-color: #a703ff:

  • step 180 becomes pure black (lightness 0 instead of 0.15);
  • step 10 loses its tint and becomes a neutral grey-white (chroma 0 instead of 0.04).

Steps 20 to 170 are exact. Chrome 149, Chrome 152 and Firefox 155 compute everything correctly, so this is a browser bug that has already been fixed upstream, but embedded browsers and WebViews on older Chromium builds still have it.

What triggers it is the shape of the expression: "multiply by 8, then divide by 8" (and "by 9 / by 9" for step 10). When the multiplier equals the divisor, that browser simplifies the whole thing to l - l. The middle steps use different numbers and are fine.

The fix is free: the two edge steps are, mathematically, just a minimum and a maximum, so they can be written that way and need no arithmetic at all:

--dx-accent-color-10:  oklch(from var(--dx-accent-color-source) max(l, var(--dx-accent-lightness-max)) min(c, var(--dx-accent-chroma-min)) h);
--dx-accent-color-180: oklch(from var(--dx-accent-color-source) min(l, var(--dx-accent-lightness-min)) min(c, var(--dx-accent-chroma-min)) h);

I checked this form in all four browsers: 0.95 / 0.04 / 0.15 everywhere.

Good news: the theme itself never uses steps 10 and 180 (light mode uses 20 to 140, dark mode uses 70 to 160), so no widget renders wrong. It only matters for people who read --dxds-primary-10 or --dxds-primary-180 directly, and those two are part of the public set of variables. For reference, Blazor's stylesheet has the same shape for its two edge steps, so this came along with the formula.

@supports (color: oklch(from red l c h)) {
:root {
--dx-accent-color-source: var(--dx-accent-color);
--dx-accent-lightness-max: 0.95;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three numbers are Blazor's, and they fit Blazor's palette: there the lightest step really has lightness 0.95. Our palette is different. When the design tokens moved to 262.6.0, a new, even lighter step was added on top (our step 10 is #f4f8fc, lightness 0.977; our step 20 is what Blazor calls step 10). With Blazor's numbers the light end of our computed palette comes out darker and more saturated than what the designers drew.

I ran the formula from each of the 11 designed palettes and compared the result with the designed steps (perceptual colour difference, where 5 is clearly visible):

settings: lightest / darkest / minimum saturation average difference steps that differ noticeably (out of 198)
.95 / .15 / .04 (as in the PR) 2.1 15, all on the light end, steps 20 to 60
.975 / .15 / .02 1.6 2
.975 / .15 / .01 1.8 3

The most visible case: a user who picks "the same blue" #0f6cbd gets a step 20 that differs from the designed #e7eff9 by 5.6. Step 20 is the light background of selected and hovered items, so people will see it.

Suggestion: tune the three numbers to our palette instead of copying them. The final numbers are for design to decide; the mechanics do not change.

},
);

if (wrapped === 0) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check only fires when nothing at all was wrapped. If one day a token gets a description, Style Dictionary prints it as a comment after the semicolon, the pattern stops matching that one line, and 17 of 18 steps get wrapped while the check stays silent. Safer to require the exact number: 18, or however many primary steps the dictionary contains.

'--dx-scheduler-animation-top',
]);

const DECLARED_BY_APPLICATION = new Set([

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list only excuses reading --dx-accent-color. The 22 variables declared in _accent-color.scss are still counted by the three older checks (publicSurfaceUnused, publicSurfaceDifferences, publicTierManualDeclarations). Those checks compare the theme against a saved snapshot that, by the rule written at the top of this file, is only ever allowed to get smaller. In this PR the snapshot grows by 22 entries in each list.

The rule exists for a reason: a --dx-* variable that only one theme declares silently breaks an application stylesheet when the user switches themes. That check was brought down to zero on 27.08.

These 22 names are neither old surface nor component variables. They are settings an application writes plus the formula's own parameters, so they deserve their own small category in this test, the same way the variables set by JavaScript already have one (RUNTIME_CONTRACT). Then the snapshot does not need to grow. They also do not follow the naming rules in NAMING.md (accent is not a component and not a registered system concern), so that category needs a one-line rule saying what these names are.

"publicSurfaceUndeclared": [],
"publicSurfaceDifferences": [],
"publicSurfaceDifferences": [
"--dx-accent-chroma-min: only in fluent-next",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the note in the test itself: these 22 entries undo a check that was at zero. If the accent variables get their own category in the test, this list becomes empty again and the snapshot does not need to be regenerated.

@@ -13,8 +13,7 @@
"targetDirectory": "./artifacts",
"excludePatterns": [
"./artifacts/css",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change has nothing to do with the accent: devextreme-dist is no longer protected from the artifact clean-up, the same exclusion exists in upstream/main, and it affects the build commands of every package. There is no explanation in the PR. My guess is that stale accent stylesheets in devextreme-dist survived a clean during local testing. If that is the reason, could it go into a separate PR with that explanation, or be dropped from this one?

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.

2 participants