Skip to content

Commit

Permalink
fix: apply even/odd classes correctly when rows are grouped
Browse files Browse the repository at this point in the history
BREAKING CHANGE: The type of `CellContext.rowIndex` was changed from
`number` to `{ index: number; indexInGroup?: number; }`.

The type of `CellContext.rowIndex` was actually never just `number` (as it was typed),
instead it was `number | string`. `string` was used if the row was wrapped in a group
and followed this pattern `<indexOfTheGroup>-<indexInsideTheGroup>`.

The new interface now allows easy access to both properties and does now longer require
any typecasting.
  • Loading branch information
spike-rabbit committed Feb 26, 2025
1 parent cf3e343 commit bd9a65b
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { BehaviorSubject } from 'rxjs';
import {
ActivateEvent,
CellContext,
RowIndex,
RowOrGroup,
SortDirection,
SortPropDir,
Expand Down Expand Up @@ -157,14 +158,14 @@ export class DataTableBodyCellComponent<TRow extends { level?: number } = any>
return this._expanded;
}

@Input() set rowIndex(val: number) {
@Input() set rowIndex(val: RowIndex) {
this._rowIndex = val;
this.cellContext.rowIndex = val;
this.checkValueUpdates();
this.cd.markForCheck();
}

get rowIndex(): number {
get rowIndex(): RowIndex {
return this._rowIndex;
}

Expand Down Expand Up @@ -314,7 +315,7 @@ export class DataTableBodyCellComponent<TRow extends { level?: number } = any>
private _row: TRow;
private _group: TRow[];
private _rowHeight: number;
private _rowIndex: number;
private _rowIndex: RowIndex;
private _expanded: boolean;
private _element = inject<ElementRef<HTMLElement>>(ElementRef).nativeElement;
private _treeStatus: TreeStatus;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,28 +1,60 @@
import { ComponentFixture, TestBed, waitForAsync } from '@angular/core/testing';
import { DataTableBodyRowComponent } from './body-row.component';
import { DataTableBodyCellComponent } from './body-cell.component';
import { Component } from '@angular/core';
import { RowIndex } from '../../types/public.types';
import { ScrollbarHelper } from '../../services/scrollbar-helper.service';
import { TableColumn } from '../../types/table-column.type';
import { By } from '@angular/platform-browser';

describe('DataTableBodyRowComponent', () => {
let fixture: ComponentFixture<DataTableBodyRowComponent>;
let component: DataTableBodyRowComponent;
@Component({
template: ` <datatable-body-row [rowIndex]="rowIndex" [row]="row" [columns]="columns" /> `,
imports: [DataTableBodyRowComponent],
standalone: true
})
class TestHostComponent {
rowIndex: RowIndex = { index: 0 };
row: any = { prop: 'value' };
columns: TableColumn[] = [{ prop: 'prop', $$valueGetter: () => 'value' }];
}

let fixture: ComponentFixture<TestHostComponent>;
let component: TestHostComponent;

// provide our implementations or mocks to the dependency injector
beforeEach(() => {
TestBed.configureTestingModule({
imports: [DataTableBodyCellComponent, DataTableBodyRowComponent]
imports: [TestHostComponent],
providers: [ScrollbarHelper]
});
});

beforeEach(waitForAsync(() => {
TestBed.compileComponents().then(() => {
fixture = TestBed.createComponent(DataTableBodyRowComponent);
fixture = TestBed.createComponent(TestHostComponent);
component = fixture.componentInstance;
});
}));

describe('fixture', () => {
it('should have a component instance', () => {
expect(component).toBeTruthy();
});
it('should apply odd/event without groups', () => {
component.rowIndex = { index: 0 };
fixture.detectChanges();
const element = fixture.debugElement.query(By.directive(DataTableBodyRowComponent))
.nativeElement as HTMLElement;
expect(element.classList).toContain('datatable-row-even');
component.rowIndex = { index: 3 };
fixture.detectChanges();
expect(element.classList).toContain('datatable-row-odd');
});

it('should apply event odd/even if row is grouped', () => {
component.rowIndex = { index: 1, indexInGroup: 0 };
fixture.detectChanges();
const element = fixture.debugElement.query(By.directive(DataTableBodyRowComponent))
.nativeElement as HTMLElement;
expect(element.classList).toContain('datatable-row-even');
component.rowIndex = { index: 666, indexInGroup: 3 };
fixture.detectChanges();
expect(element.classList).toContain('datatable-row-odd');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
import { columnGroupWidths, columnsByPin, columnsByPinArr } from '../../utils/column';
import { Keys } from '../../utils/keys';
import { BehaviorSubject } from 'rxjs';
import { ActivateEvent, RowOrGroup, TreeStatus } from '../../types/public.types';
import { ActivateEvent, RowIndex, RowOrGroup, TreeStatus } from '../../types/public.types';
import { AsyncPipe } from '@angular/common';
import { TableColumn } from '../../types/table-column.type';
import { ColumnGroupWidth, PinnedColumns } from '../../types/internal.types';
Expand Down Expand Up @@ -92,7 +92,7 @@ export class DataTableBodyRowComponent<TRow = any> implements DoCheck, OnChanges
@Input() row: TRow;
@Input() group: TRow[];
@Input() isSelected: boolean;
@Input() rowIndex: number;
@Input() rowIndex: RowIndex;
@Input() displayCheck: (row: TRow, column: TableColumn, value?: any) => boolean;
@Input() treeStatus?: TreeStatus = 'collapsed';
@Input() ghostLoadingIndicator = false;
Expand All @@ -113,10 +113,10 @@ export class DataTableBodyRowComponent<TRow = any> implements DoCheck, OnChanges
if (this.isSelected) {
cls += ' active';
}
if (this.rowIndex % 2 !== 0) {
if (this.innerRowIndex % 2 !== 0) {
cls += ' datatable-row-odd';
}
if (this.rowIndex % 2 === 0) {
if (this.innerRowIndex % 2 === 0) {
cls += ' datatable-row-even';
}
if (this.disable$ && this.disable$.value) {
Expand Down Expand Up @@ -228,4 +228,9 @@ export class DataTableBodyRowComponent<TRow = any> implements DoCheck, OnChanges
onTreeAction() {
this.treeAction.emit();
}

/** Returns the row index, or if in a group, the index within a group. */
private get innerRowIndex(): number {
return this.rowIndex.indexInGroup ?? this.rowIndex.index;
}
}
29 changes: 16 additions & 13 deletions projects/ngx-datatable/src/lib/components/body/body.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
ActivateEvent,
DragEventData,
Group,
RowIndex,
RowOrGroup,
ScrollEvent,
SelectionType,
Expand Down Expand Up @@ -92,6 +93,7 @@ import { ProgressBarComponent } from './progress-bar.component';
</datatable-summary-row>
}
@for (group of rowsToRender(); track rowTrackingFn(i, group); let i = $index) {
<!-- getRowIndex will return here always a number, so we need the $any here. -->
<datatable-row-wrapper
#rowWrapper
[attr.hidden]="
Expand All @@ -108,7 +110,7 @@ import { ProgressBarComponent } from './progress-bar.component';
[row]="group"
[disableCheck]="disableRowCheck"
[expanded]="getRowExpanded(group)"
[rowIndex]="getRowIndex(group && group[i])"
[rowIndex]="$any(getRowIndex(group && group[i]))"
[selected]="selected"
(rowContextmenu)="rowContextmenu.emit($event)"
>
Expand Down Expand Up @@ -433,7 +435,7 @@ export class DataTableBodyComponent<TRow extends { treeStatus?: TreeStatus } = a
columnGroupWidths: ColumnGroupWidth;
rowTrackingFn: TrackByFunction<RowOrGroup<TRow>>;
listener: any;
rowIndexes = new WeakMap<any, any>();
rowIndexes = new WeakMap<RowOrGroup<TRow>, RowIndex>();
rowExpansions: any[] = [];

_rows: TRow[];
Expand Down Expand Up @@ -600,13 +602,12 @@ export class DataTableBodyComponent<TRow extends { treeStatus?: TreeStatus } = a
while (rowIndex < last && rowIndex < this.groupedRows.length) {
// Add the groups into this page
const group = this.groupedRows[rowIndex];
this.rowIndexes.set(group, rowIndex);
this.rowIndexes.set(group, { index: rowIndex });

if (group.value) {
// add indexes for each group item
group.value.forEach((g: any, i: number) => {
const _idx = `${rowIndex}-${i}`;
this.rowIndexes.set(g, _idx);
group.value.forEach((g: TRow, i: number) => {
this.rowIndexes.set(g, { index: rowIndex, indexInGroup: i });
});
}
temp[idx] = group;
Expand All @@ -621,7 +622,7 @@ export class DataTableBodyComponent<TRow extends { treeStatus?: TreeStatus } = a

if (row) {
// add indexes for each row
this.rowIndexes.set(row, rowIndex);
this.rowIndexes.set(row, { index: rowIndex });
temp[idx] = row;
} else if (this.ghostLoadingIndicator && this.virtualization) {
temp[idx] = undefined;
Expand Down Expand Up @@ -730,10 +731,12 @@ export class DataTableBodyComponent<TRow extends { treeStatus?: TreeStatus } = a
if (Array.isArray(rows)) {
// Get the latest row rowindex in a group
const row = rows[rows.length - 1];
idx = row ? this.getRowIndex(row) : 0;
// The group row, which has always a numeric index
idx = row ? this.getRowIndex(row).index : 0;
} else {
if (rows) {
idx = this.getRowIndex(rows);
// normal rows always have a numeric index
idx = this.getRowIndex(rows).index;
} else {
// When ghost cells are enabled use index to get the position of them
idx = this.indexes().first + index;
Expand Down Expand Up @@ -872,8 +875,8 @@ export class DataTableBodyComponent<TRow extends { treeStatus?: TreeStatus } = a
// If the detailRowHeight is auto --> only in case of non-virtualized scroll
if (this.scrollbarV && this.virtualization) {
const detailRowHeight = this.getDetailRowHeight(row) * (expanded ? -1 : 1);
// const idx = this.rowIndexes.get(row) || 0;
const idx = this.getRowIndex(row);
// This is hopefully only called with non-grouped rows. Otherwise, the heightCache fails.
const idx = this.getRowIndex(row).index;
this.rowHeightsCache().update(idx, detailRowHeight);
}

Expand Down Expand Up @@ -954,8 +957,8 @@ export class DataTableBodyComponent<TRow extends { treeStatus?: TreeStatus } = a
/**
* Gets the row index given a row
*/
getRowIndex(row: RowOrGroup<TRow>): number {
return this.rowIndexes.get(row) || 0;
getRowIndex(row: RowOrGroup<TRow>): RowIndex {
return this.rowIndexes.get(row);
}

onTreeAction(row: TRow) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function noopSumFunc(cells: any[]): void {
[columns]="_internalColumns"
[rowHeight]="rowHeight"
[row]="summaryRow"
[rowIndex]="-1"
[rowIndex]="{ index: -1 }"
>
</datatable-body-row>
}
Expand Down
10 changes: 9 additions & 1 deletion projects/ngx-datatable/src/lib/types/public.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export interface CellContext<TRow = any> {
column: TableColumn;
rowHeight: number;
isSelected: boolean;
rowIndex: number;
rowIndex: RowIndex;
treeStatus: TreeStatus;
disable$: BehaviorSubject<boolean>;
onTreeAction: () => void;
Expand Down Expand Up @@ -174,3 +174,11 @@ export interface DragEventData {
dragRow: any;
dropRow?: any;
}

/** Represents the index of a row. */
export interface RowIndex {
/** Index of the row. If the row is inside a group, it will hold the index the group. */
index: number;
/** Index of a row inside a group. Only present if the row is inside a group. */
indexInGroup?: number;
}
14 changes: 7 additions & 7 deletions src/app/basic/disabled-rows.component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Component } from '@angular/core';
import { ColumnMode, SelectionType } from 'projects/ngx-datatable/src/public-api';
import { ColumnMode, RowIndex, SelectionType } from 'projects/ngx-datatable/src/public-api';
import { FullEmployee } from '../data.model';
import { BehaviorSubject } from 'rxjs';

Expand Down Expand Up @@ -107,19 +107,19 @@ export class DisabledRowsComponent {
return !(!row.isDisabled && row.age < 40);
}

disableRow(rowIndex: number, disableRow$: BehaviorSubject<boolean>) {
this.rows[rowIndex].isDisabled = true;
disableRow({ index }: RowIndex, disableRow$: BehaviorSubject<boolean>) {
this.rows[index].isDisabled = true;
this.rows = [...this.rows];
disableRow$.next(true);
}

updateValue(event, cell, rowIndex: number, disableRow$: BehaviorSubject<boolean>) {
this.rows[rowIndex][cell] = event.target.value;
updateValue(event, cell, { index }: RowIndex, disableRow$: BehaviorSubject<boolean>) {
this.rows[index][cell] = event.target.value;
this.rows = [...this.rows];
if (disableRow$ && cell === 'age' && this.rows[rowIndex][cell] > 40) {
if (disableRow$ && cell === 'age' && this.rows[index][cell] > 40) {
disableRow$.next(true);
}
if (disableRow$ && cell === 'gender' && this.rows[rowIndex][cell] === 'male') {
if (disableRow$ && cell === 'gender' && this.rows[index][cell] === 'male') {
disableRow$.next(true);
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/app/basic/inline.component.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Component } from '@angular/core';
import { ColumnMode } from 'projects/ngx-datatable/src/public-api';
import { ColumnMode, RowIndex } from 'projects/ngx-datatable/src/public-api';
import { Employee } from '../data.model';

@Component({
Expand Down Expand Up @@ -99,11 +99,11 @@ export class InlineEditComponent {
req.send();
}

updateValue(event, cell, rowIndex: number) {
console.log('inline editing rowIndex', rowIndex);
this.editing[rowIndex + '-' + cell] = false;
this.rows[rowIndex][cell] = event.target.value;
updateValue(event, cell, { index }: RowIndex) {
console.log('inline editing rowIndex', index);
this.editing[index + '-' + cell] = false;
this.rows[index][cell] = event.target.value;
this.rows = [...this.rows];
console.log('UPDATED!', this.rows[rowIndex][cell]);
console.log('UPDATED!', this.rows[index][cell]);
}
}

0 comments on commit bd9a65b

Please sign in to comment.