fix: ensure custom sort icons become applied#703
Conversation
ensure consumers can provide multiple classes for a single sort direction
![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 updates the DataTableHeaderCellComponent to return sortClass as a space-separated string instead of an array, and adds a new test suite to verify custom sort icons using Vitest's browser context. A review comment correctly identifies a compilation issue where inputBinding and outputBinding are incorrectly imported from @angular/core instead of @angular/core/testing.
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.
| import { | ||
| AfterViewInit, | ||
| Component, | ||
| inputBinding, | ||
| outputBinding, | ||
| signal, | ||
| TemplateRef, | ||
| viewChild, | ||
| WritableSignal | ||
| } from '@angular/core'; | ||
| import { ComponentFixture, TestBed } from '@angular/core/testing'; |
There was a problem hiding this comment.
In Angular 18.2+, inputBinding and outputBinding are exported from @angular/core/testing, not @angular/core. Importing them from @angular/core will cause a compilation error. Please move these imports to @angular/core/testing.
import {
AfterViewInit,
Component,
signal,
TemplateRef,
viewChild,
WritableSignal
} from '@angular/core';
import { ComponentFixture, TestBed, inputBinding, outputBinding } from '@angular/core/testing';
There was a problem hiding this comment.
Pull request overview
Fixes sort icon class application in DataTableHeaderCellComponent so consumers can supply multiple CSS classes (space-separated) for custom sort icons without Angular treating them as a single invalid class token.
Changes:
- Convert computed
sortClassfromstring[]to a space-delimitedstringfor correct[class]binding. - Add unit tests covering custom ascending/descending sort icon class application.
- Update spec tsconfig types 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 1 comment.
| File | Description |
|---|---|
| projects/ngx-datatable/tsconfig.spec.json | Adds vitest/browser typings to support browser-mode test utilities/types. |
| projects/ngx-datatable/src/lib/components/header/header-cell.component.ts | Fixes [class] binding by joining sort class arrays into a single space-delimited string. |
| projects/ngx-datatable/src/lib/components/header/header-cell.component.spec.ts | Adds regression tests ensuring multi-class custom sort icon strings are applied correctly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
| await fixture.whenStable(); | ||
| }); |
| protected readonly sortClass = computed<string[] | undefined>(() => { | ||
| return this.calcSortClass(this.sortDir()); | ||
| protected readonly sortClass = computed<string | undefined>(() => { | ||
| return this.calcSortClass(this.sortDir())?.join(' '); |
There was a problem hiding this comment.
please move the join inside calcSortClass and please either inline the function then or at least convert to an arrow function without {}
|
closing in favor of #704 |
ensure consumers can provide multiple classes for a single sort direction
class binding support array but doesn't allow spaces for array items. In that case nothing become applied
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: