refactor: convert groupedRows to signal input #524
Merged
Conversation
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
spike-rabbit
changed the title
spike-rabbit
force-pushed
the
refactor/datatable/make-rows-group
branch
2 times, most recently
from
dbdc192 to
ebb5a76
Compare
Copilot finished reviewing on behalf of
fh1ch
November 19, 2025 11:35
There was a problem hiding this comment.
Pull Request Overview
This PR refactors groupedRows and groupRowsBy from decorator-based @Input() properties to signal inputs, introducing a new computed signal _internalGroupedRows that consolidates the logic for calculating and sorting grouped rows. The refactoring moves sorting and grouping logic from imperative lifecycle methods into reactive computed signals.
Key changes:
- Converted
groupedRowsandgroupRowsByto signal inputs - Introduced
_internalGroupedRowscomputed signal to handle grouped row calculation and sorting - Removed manual sorting calls from
ngDoCheck()andonColumnSort(), as sorting is now reactive - Removed the
sortInternalRows()private method and_groupRowsByfield
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| projects/ngx-datatable/src/lib/components/datatable.component.ts | Converted inputs to signals, added _internalGroupedRows computed signal, removed imperative sorting logic |
| projects/ngx-datatable/src/lib/components/datatable.component.html | Updated bindings to use _internalGroupedRows() instead of groupedRows, removed groupRowsBy binding |
| projects/ngx-datatable/src/lib/components/body/body.component.ts | Removed unused groupRowsBy input from body component |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
projects/ngx-datatable/src/lib/components/datatable.component.ts
Outdated
Show resolved
Hide resolved
projects/ngx-datatable/src/lib/components/datatable.component.ts
Outdated
Show resolved
Hide resolved
spike-rabbit
force-pushed
the
refactor/datatable/make-rows-group
branch
from
ebb5a76 to
1adc5b9
Compare
Converts `groupedRows` and `groupRowsBy` to signal inputs. Introduces a new `_internalGroupedRows` which contains the combination of both inputs. BREAKING CHANGE: Previously, the `groupedRows` input was updated by the datatable, when `groupRowsBy` was used. This behavior was dropped. `groupedRows` will only contain application provided values. The table calculates internal the actual `groupedRows` without exposing them.
fh1ch
force-pushed
the
refactor/datatable/make-rows-group
branch
from
1adc5b9 to
731df2b
Compare
fh1ch
approved these changes
Nov 19, 2025
Member
fh1ch
left a comment
fh1ch
left a comment
There was a problem hiding this comment.
@spike-rabbit outstanding work, thanks a bunch 🙇
LGTM 👍
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)
groupedRowsandgroupRowsByare decorator based inputs.What is the new behavior?
groupedRowsandgroupRowsByare signal inputs. A new_internalGroupedRowscontains the calculated combination.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:
groupedRowswill no longer be updated, if grouped rows are calculated based ongroupRowsBy.Other information: