fix: ensure custom sort icons become applied#704
Conversation
![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 DataTableHeaderCellComponent to return a space-separated string for sortClass instead of an array of strings, and adds unit tests to verify custom sort icons. A review comment points out that accessing this.cellContext() in calcSortClass introduces an unnecessary dependency on the cellContext computed signal, which can trigger redundant re-evaluations when row selection changes. It is recommended to directly access this.column().sortable instead.
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.
There was a problem hiding this comment.
Pull request overview
This PR fixes sort icon class application in DataTableHeaderCellComponent so consumers can provide multiple CSS classes for a given sort direction (e.g., "icon up") and have them applied correctly to the sort indicator element.
Changes:
- Change
sortClass/calcSortClassto return a space-delimitedstringsuitable for[class]binding (instead of astring[]). - Add unit tests verifying that custom multi-class sort icon values are applied for ascending/descending toggles.
- Update spec TS config typings to include Vitest browser typings used by the new tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| projects/ngx-datatable/tsconfig.spec.json | Adds Vitest browser typings needed by browser-context test utilities. |
| projects/ngx-datatable/src/lib/components/header/header-cell.component.ts | Fixes sort indicator class binding by emitting a single class string. |
| projects/ngx-datatable/src/lib/components/header/header-cell.component.spec.ts | Adds coverage to ensure multi-class custom sort icon inputs are applied. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Ensure consumers can provide multiple classes for a single sort direction.
spike-rabbit
force-pushed
the
fix/header-column-sort-icons
branch
from
54f8c49 to
eb705a5
Compare
ensure consumers can provide multiple classes for a single sort direction
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)
What is the new behavior?
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: ...
Other information: