Refactor/merge css grids#738
Open
spike-rabbit wants to merge 2 commits into
Open
Conversation
Move the column track definition onto the `.datatable-grid` container and have the header, body, and every intermediate wrapper inherit it through `grid-template-columns: subgrid`, instead of each region re-instantiating the template from the shared CSS variable. This makes header and body cells live in a single grid so columns align on the exact same tracks. BREAKING CHANGE: All content of the header and body now reside in one single CSS grid. This can affect drag/drop interactions build around `datatable-row-def` using `@angular/cdk` or other solutions that depend on `cloneNode` to create drag previews. Enable `preserveColumnWidthsOnClone` on `datatable-row-def` to automatically copy the current column sizes into the cloned row, making independent of the table CSS grid.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the layout of the datatable components (including headers, rows, group wrappers, and body) to use CSS Grid subgrid instead of flexbox or explicit grid templates. It also introduces a preserveColumnWidthsOnClone input to DatatableRowDefComponent to measure and stamp column widths on cloned rows, which is useful for drag-and-drop previews. The reviewer suggested an improvement in DatatableRowDefComponent to use Function.prototype.call and delete instead of bind and re-assigning the bound function on cleanup, which avoids creating an extra closure and cleanly removes the overridden property.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
spike-rabbit
force-pushed
the
refactor/merge-css-grids
branch
from
fe8d2c3 to
e9092b5
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors ngx-datatable’s layout to use a single table-wide CSS grid (with subgrids) so header/body rows participate in the same column tracks, and introduces an opt-in workaround for drag preview cloning that may break due to the new shared grid approach.
Changes:
- Move the shared
grid-template-columnsdefinition to the table-level.datatable-gridand update header/body/row wrappers to usesubgridfor column alignment. - Add
preserveColumnWidthsOnCloneondatatable-row-defto stamp measured column widths onto cloned rows (e.g., CDK drag preview clones). - Update tests and Playwright snapshots to reflect the new grid behavior and empty-row spanning.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/app/drag-drop/drag-drop.component.ts | Enables the new preserveColumnWidthsOnClone option for the drag/drop example. |
| projects/ngx-datatable/src/lib/components/header/header.component.spec.ts | Updates header tests to apply grid tracks via gridTemplateColumns. |
| projects/ngx-datatable/src/lib/components/header/header.component.scss | Makes header participate in the shared grid via subgrid and explicit grid placement. |
| projects/ngx-datatable/src/lib/components/datatable.component.scss | Moves table grid columns to .datatable-grid and adds empty-row spanning rule. |
| projects/ngx-datatable/src/lib/components/datatable.component.html | Adds datatable-empty-row class to the default empty-content row. |
| projects/ngx-datatable/src/lib/components/body/summary/summary-row.component.scss | Aligns summary row to the shared grid and ensures row spans full width. |
| projects/ngx-datatable/src/lib/components/body/body.component.ts | Adds a wrapper class to support new grid styling for rendered rows. |
| projects/ngx-datatable/src/lib/components/body/body.component.scss | Converts body/scroller/render wrapper to subgrid-based layout and spans full width. |
| projects/ngx-datatable/src/lib/components/body/body-row.component.spec.ts | Updates body-row tests to provide a real grid container for subgrid usage. |
| projects/ngx-datatable/src/lib/components/body/body-row.component.scss | Switches body rows to subgrid column alignment and full-width spanning. |
| projects/ngx-datatable/src/lib/components/body/body-row-wrapper.component.scss | Converts row wrapper to subgrid layout and spans row detail across all columns. |
| projects/ngx-datatable/src/lib/components/body/body-row-def.component.ts | Adds preserveColumnWidthsOnClone and cloneNode patching; makes row-def a subgrid participant. |
| projects/ngx-datatable/src/lib/components/body/body-group-wrapper.component.scss | Converts group wrapper to subgrid and ensures group header spans all columns. |
| playwright/snapshots/e2e/summary-row.spec.ts-snapshots/summary-row-actions--summary-row-actions-chromium-linux.png | Updates snapshot for new layout. |
| playwright/snapshots/e2e/selection.spec.ts-snapshots/multi-click-and-checkbox-selection--navigation-using-tab-and-space-chromium-linux.png | Updates snapshot for new layout. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
spike-rabbit
force-pushed
the
refactor/merge-css-grids
branch
from
e9092b5 to
5f6507b
Compare
…lone We need this feature to support a table wide CSS grid layout. The `@angular/cdk` drag and drop feature is based on cloning the dragging element. Since cloning the row removes the connection to the actual grid, consumers need a way to maintain the current layout. With this change, `cloneNode` can be optionally overridden by setting `preserveColumnWidthsOnClone`. This will take a snapshot of the current column widths to create a new CSS grid on the clone.
spike-rabbit
force-pushed
the
refactor/merge-css-grids
branch
from
5f6507b to
0aacc26
Compare
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.
What kind of change does this PR introduce? (check one with "x")
What is the current behavior? (You can also link to an open issue here)
Currently each row has its own CSS grid defined.
What is the new behavior?
One table wide CSS grid, everything below uses subgrids to participate on the parent grid.
Does this PR introduce a breaking change? (check one with "x")
If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...
This can affect drag/drop interactions build around
datatable-row-defusing@angular/cdkor other solutions that depend on
cloneNodeto create drag previews.Enable
preserveColumnWidthsOnCloneondatatable-row-defto automatically copy the currentcolumn sizes into the cloned row, making independent of the table CSS grid.
Other information:
One CSS grid has a few advantages: