DataGrid/TreeList: Split basic cell styles#32694
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the base Grid (DataGrid/TreeList) SCSS by extracting “basic cell styles” from gridBase/_index.scss into a new gridBase/layout/cell.scss mixin, then including that mixin from grid-base($widget-name).
Changes:
- Added
grid-base-cell($widget-name)mixin inlayout/cell.scssand moved core cell-related rules into it. - Updated
gridBase/_index.scssto@usethe new layout module and@include grid-base-cell($widget-name). - Removed large inlined cell-style blocks from
gridBase/_index.scssand adjusted a few selectors accordingly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
packages/devextreme-scss/scss/widgets/base/gridBase/layout/cell.scss |
New extracted mixin with core cell styles (positioning, overflow, sizing, borders, padding, etc.). |
packages/devextreme-scss/scss/widgets/base/gridBase/_index.scss |
Imports and includes the new mixin; deletes/rewires the previously inlined rules. |
Comments suppressed due to low confidence (1)
packages/devextreme-scss/scss/widgets/base/gridBase/_index.scss:75
grid-base-cellalready defines the “last-row-border” behavior for cells (layout/cell.scsssets… > td { border-bottom-width: 0; }). Keeping this additional rule here is redundant, and as written it targets.dx-data-rowrather than thetds (which is typically where the border is applied). Consider removing this block from_index.scssand keeping the single> tdrule inlayout/cell.scss.
packages/devextreme-scss/scss/widgets/base/gridBase/layout/cell.scss
Outdated
Show resolved
Hide resolved
packages/devextreme-scss/scss/widgets/base/gridBase/layout/cell.scss
Outdated
Show resolved
Hide resolved
packages/devextreme-scss/scss/widgets/base/gridBase/layout/cell.scss
Outdated
Show resolved
Hide resolved
975ba39 to
da15869
Compare
| @@ -0,0 +1,341 @@ | |||
| /* stylelint-disable no-duplicate-selectors */ | |||
There was a problem hiding this comment.
The file disables no-duplicate-selectors for the entire stylesheet. This makes it easy for future changes to introduce accidental duplicates (including duplicates with conflicting properties) without lint feedback. Consider either consolidating the repeated selectors into single blocks (e.g. .dx-header-row > td, .dx-... .dx-command-expand) or scoping the suppression to the minimum necessary range (disable-next-line / disable + enable around specific blocks).
| /* stylelint-disable no-duplicate-selectors */ |
packages/devextreme-scss/scss/widgets/base/gridBase/layout/cell.scss
Outdated
Show resolved
Hide resolved
1571277 to
99429ea
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/devextreme-scss/scss/widgets/base/gridBase/_index.scss:63
border-bottom-width: 0;is now applied to the.dx-data-rowelement instead of its cells. In this codebase the row borders are applied to> td, so this selector likely no longer removes the last-row border (and it also duplicates the correct> tdrule that already exists inlayout/cell.scss). Update this block to target> td(or remove it entirely and rely on thelayout/cell.scssimplementation).
packages/devextreme-scss/scss/widgets/generic/treeList/layout/cell.scss
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/devextreme-scss/scss/widgets/base/gridBase/_index.scss:63
- The selector under
.dx-last-row-bordernow setsborder-bottom-width: 0on.dx-data-row(the<tr>). In this codebase the last-row-border fix targets table cells (> td) (now implemented inlayout/cell.scss), so keeping the<tr>rule here is redundant and likely has no effect. Consider removing this rule from_index.scssand relying on the> tdrule inlayout/cell.scssto avoid confusion/duplicate maintenance.
packages/devextreme-scss/scss/widgets/material/dataGrid/layout/cell.scss
Outdated
Show resolved
Hide resolved
packages/devextreme-scss/scss/widgets/material/gridBase/layout/cell.scss
Show resolved
Hide resolved
| .dx-rtl .dx-datagrid .dx-datagrid-sticky-columns .dx-datagrid-content .dx-datagrid-table .dx-row.dx-column-lines > td.dx-datagrid-first-header.dx-datagrid-sticky-column-border-left { | ||
| border-left: 2px solid; | ||
| border-left-color: $datagrid-border-color; | ||
| } |
There was a problem hiding this comment.
Why we do not use scss? These rules can be refactored, now they are too heavy. There is the style duplication everywhere:
.dx-rtl .dx-datagrid .dx-datagrid-sticky-columns .dx-datagrid-content .dx-datagrid-table .dx-row.dx-column-lines...
There was a problem hiding this comment.
Why we do not use scss? These rules can be refactored, now they are too heavy. There is the style duplication everywhere:
.dx-rtl .dx-datagrid .dx-datagrid-sticky-columns .dx-datagrid-content .dx-datagrid-table .dx-row.dx-column-lines...
I agree that we can combine styles into shared selectors in some places. But let's do that later. I’ve added it to my to-do list.
No description provided.