Themes: drop the declarations the cascade never renders - #35251
Open
EugeniyKiyashko wants to merge 4 commits into
Open
EugeniyKiyashko wants to merge 4 commits into
EugeniyKiyashko wants to merge 4 commits into
Conversation
85 declarations in base, generic, material and fluent were declared on a selector that a later rule re-declares for the same property, so no browser ever painted them. The eight base variables read only by those declarations go with them, and the blocks left empty are removed. Proof: the cascade model of all 45 bundles (at-rule context, selector, property -> winning value; 683 648 keys) is identical before and after, and the same removals applied to the design-tokens feature branch leave its 49 bundles, fluent-next included, identical too. Declarations that stay alive in fluent-next or whose winner sits in another widget were left in place.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Restore or replace the generic agenda appointment shadow reset.
Pull request overview
Removes redundant or unused SCSS declarations across DevExtreme themes and widgets.
Changes:
- Cleans obsolete sizing, spacing, color, and state rules.
- Removes redundant Scheduler, editor, popup, list, and layout styles.
- Relies on effective theme cascade replacements.
File summaries
| File | Change |
|---|---|
packages/devextreme-scss/scss/widgets/material/icons/_index.scss |
Removes redundant load-indicator styling. |
packages/devextreme-scss/scss/widgets/material/fieldset/_index.scss |
Removes unused validation spacing. |
packages/devextreme-scss/scss/widgets/generic/icons/_index.scss |
Removes redundant load-indicator styling. |
packages/devextreme-scss/scss/widgets/generic/fieldset/_index.scss |
Removes unused validation spacing. |
packages/devextreme-scss/scss/widgets/fluent/icons/_index.scss |
Removes redundant load-indicator styling. |
packages/devextreme-scss/scss/widgets/fluent/fieldset/_index.scss |
Removes unused validation spacing. |
packages/devextreme-scss/scss/widgets/base/treeView/_common.scss |
Removes unused search styling. |
packages/devextreme-scss/scss/widgets/base/toolbar/_index.scss |
Removes duplicate toolbar height. |
packages/devextreme-scss/scss/widgets/base/timeView/_index.scss |
Removes obsolete time-view sizing. |
packages/devextreme-scss/scss/widgets/base/switch/_index.scss |
Removes theme-overridden switch sizing. |
packages/devextreme-scss/scss/widgets/base/scrollable/_index.scss |
Removes redundant scrollbar sizing. |
packages/devextreme-scss/scss/widgets/base/scheduler/views/month/_index.scss |
Removes unused month-view styling. |
packages/devextreme-scss/scss/widgets/base/scheduler/views/agenda/_index.scss |
Removes theme-overridden agenda declarations. |
packages/devextreme-scss/scss/widgets/base/scheduler/views/_index.scss |
Removes redundant Scheduler dimensions and state styles. |
packages/devextreme-scss/scss/widgets/base/scheduler/appointment/agenda/_index.scss |
Removes the agenda appointment shadow reset; this requires retention or replacement. |
packages/devextreme-scss/scss/widgets/base/scheduler/_index.scss |
Removes theme-specific appointment padding. |
packages/devextreme-scss/scss/widgets/base/scheduler/_common.scss |
Removes redundant appointment collector defaults. |
packages/devextreme-scss/scss/widgets/base/pivotGrid/_index.scss |
Removes theme-overridden indicator color. |
packages/devextreme-scss/scss/widgets/base/lookup/_index.scss |
Removes redundant lookup sizing. |
packages/devextreme-scss/scss/widgets/base/list/_index.scss |
Removes obsolete list sizing and padding. |
packages/devextreme-scss/scss/widgets/base/gridBase/_index.scss |
Removes redundant revert-button margin. |
packages/devextreme-scss/scss/widgets/base/fileManager/_index.scss |
Removes theme-overridden button spacing. |
packages/devextreme-scss/scss/widgets/base/dropDownEditor/_index.scss |
Removes redundant icon height. |
packages/devextreme-scss/scss/widgets/base/diagram/_index.scss |
Removes redundant toolbox width. |
packages/devextreme-scss/scss/widgets/base/dataGrid/_common.scss |
Removes theme-provided group-panel styling. |
packages/devextreme-scss/scss/widgets/base/colorBox/_index.scss |
Removes redundant overlay padding. |
packages/devextreme-scss/scss/widgets/base/_toast.scss |
Removes theme-provided toast styling. |
packages/devextreme-scss/scss/widgets/base/_tileView.scss |
Removes theme-overridden tile alignment. |
packages/devextreme-scss/scss/widgets/base/_slider.scss |
Removes theme-provided range height. |
packages/devextreme-scss/scss/widgets/base/_popup.scss |
Removes theme-provided popup defaults. |
packages/devextreme-scss/scss/widgets/base/_pagination.scss |
Removes theme-specific pagination sizing. |
packages/devextreme-scss/scss/widgets/base/_overlay.scss |
Removes theme-provided shader color. |
packages/devextreme-scss/scss/widgets/base/_numberBox.scss |
Removes redundant number-box defaults. |
packages/devextreme-scss/scss/widgets/base/_loadPanel.scss |
Removes declarations superseded by the effective cascade. |
packages/devextreme-scss/scss/widgets/base/_htmlEditor.scss |
Removes declarations superseded by the effective cascade. |
packages/devextreme-scss/scss/widgets/base/_gallery.scss |
Removes declarations superseded by the effective cascade. |
packages/devextreme-scss/scss/widgets/base/_form.scss |
Removes declarations superseded by the effective cascade. |
packages/devextreme-scss/scss/widgets/base/_fieldset.scss |
Removes declarations superseded by the effective cascade. |
packages/devextreme-scss/scss/widgets/base/_dropDownList.scss |
Removes declarations superseded by the effective cascade. |
packages/devextreme-scss/scss/widgets/base/_dateBox.scss |
Removes declarations superseded by the effective cascade. |
packages/devextreme-scss/scss/widgets/base/_box.scss |
Removes declarations superseded by the effective cascade. |
packages/devextreme-scss/scss/widgets/base/_accordion.scss |
Removes declarations superseded by the effective cascade. |
Review details
Suppressed comments (1)
packages/devextreme-scss/scss/widgets/base/scheduler/appointment/agenda/_index.scss:13
- Removing this reset leaves generic agenda appointments with the shadows emitted by
base/scheduler/appointment/regular/_index.scss(box-shadowfor normal, hover, and active states). Unlike Material and Fluent, the generic scheduler styles do not override those shadows, so agenda appointments will render with unintended vertical-appointment shadows. Keep the agenda-specificbox-shadow: nonereset (including the state variants if needed) or add an equivalent generic-theme override.
background-color: transparent;
- Files reviewed: 42/42 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
tests/dead-declarations.test.ts compiles the 44 theme bundles with source maps and fails on any SCSS declaration that loses for every selector of its rule in every bundle, naming the line and the rules that repaint it. The 23 known cases stay in dead-declarations.baseline.json with a reason each: 21 base values fluent-next still renders on the design-tokens branch, and 2 dataGrid outlines whose winner lives in treeList.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Baseline entries must require a non-empty reason so the regression test enforces its stated requirement.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 44/44 changed files
- Comments generated: 1
- Review effort level: Lite
…one block A missing comma in the dropDownEditor of fluent and material compiled into seven descendant chains no markup produces; the scheduler of fluent styled `.dx-scheduler-header-panel-cell-date343`, a class the widget never sets. Both go, unfixed: adding the comma or the right class name would change what renders, and this change does not. Four blocks declared the same property twice with the same value, gallery in generic kept a background only to repaint it transparent on the next line (the variable behind it goes too), and two parse-time fallbacks - `display: -webkit-box` and `cursor: pointer` before `grabbing` - are dead for every browser in browserslist. The gate now counts duplicates inside one block as well. Cascade model of the 45 bundles: no winning value changed; the 17 keys that disappear are exactly the removed selectors.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Address the WebKit fallback and enforce non-blank baseline reasons.
Review details
Suppressed comments (2)
packages/devextreme-scss/scss/widgets/base/_form.scss:23
- This removes the explicit
-webkit-boxfallback even though the followingdisplay: flexis not understood by older WebKit engines. The inline stylelint suppression confirms this was an intentional compatibility declaration; keeping only the unprefixed value can make.dx-label-h-align.dx-flex-layoutlose its layout in those supported browsers.
&.dx-flex-layout {
display: flex;
packages/devextreme-scss/tests/dead-declarations.test.ts:182
- The baseline is treated as an allowlist using only
file,property, andselector;reasonis never checked. Consequently, adding a new dead declaration with an empty or missing reason to the JSON makes this test pass, despite its stated contract requiring every baseline entry to have a reason. Filter the allowlist to entries with a non-blank reason (or add an explicit validation assertion).
- Files reviewed: 54/54 changed files
- Comments generated: 0 new
- Review effort level: Lite
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The dead-declaration test misses overlapping selectors with differing specificity.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
packages/devextreme-scss/tests/dead-declarations.test.ts:112
- The key only contains the normalized selector text, so
byKeycompares declarations only when the selectors are textually identical. A later more-specific selector (for example.dx-widget .dx-popup-contentoverriding.dx-popup-content) can make an earlier declaration dead for every matching element, but this test will never detect it; the suite therefore does not enforce the stated “cascade never renders” invariant for the selectors in this PR. Compare declarations whose selector match sets overlap and apply specificity/order (or narrow the test name and contract to exact-selector duplicates).
- Files reviewed: 54/54 changed files
- Comments generated: 0 new
- Review effort level: Lite
aleksei-semikozov
approved these changes
Sep 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.