Themes: drop the declarations the cascade never renders - #35251
Open
EugeniyKiyashko wants to merge 2 commits into
Open
EugeniyKiyashko wants to merge 2 commits into
EugeniyKiyashko wants to merge 2 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
| }); | ||
|
|
||
| test('every dead declaration is removed or listed in the baseline with a reason', () => { | ||
| const known = new Set(baseline.map(signature)); |
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.