Skip to content

Commit

Permalink
fix(tree): couple of issues found in Tree Data
Browse files Browse the repository at this point in the history
1. initial sort not always working
2. tree level property should not be required while providing a `parentId` relation
3. tree data was loading and rendering the grid more than once (at least 1x time before the sort and another time after the tree was built) while it should only be rendered once
  • Loading branch information
ghiscoding committed May 4, 2021
1 parent 49437d8 commit 0b120f4
Show file tree
Hide file tree
Showing 15 changed files with 255 additions and 118 deletions.
2 changes: 1 addition & 1 deletion src/app/examples/grid-resize-by-content.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const myCustomTitleValidator = (value: any, args: any) => {
@Injectable()
export class GridResizeByContentComponent implements OnInit {
title = 'Example 30: Columns Resize by Content';
subTitle = `The grid below uses the optional resize by cell content (with a fixed 950px for demo purposes), you can click on the 2 buttons to see the difference. The "autosizeColumns" is really the default option used by SlickGrid-Universal, the resize by cell content is optional because it requires to read the first thousand rows and do extra width calculation.`;
subTitle = `The grid below uses the optional resize by cell content (with a fixed 950px for demo purposes), you can click on the 2 buttons to see the difference. The "autosizeColumns" is really the default option used by Angular-SlickGrid, the resize by cell content is optional because it requires to read the first thousand rows and do extra width calculation.`;

angularGrid!: AngularGridInstance;
gridOptions!: GridOption;
Expand Down
8 changes: 4 additions & 4 deletions src/app/examples/grid-tree-data-hierarchical.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ <h2>
<span>Expand All</span>
</button>
<button (click)="logFlatStructure()" class="btn btn-outline-secondary btn-sm">
<span>Log Flag Structure</span>
<span>Log Flat Structure</span>
</button>
<button (click)="logExpandedStructure()" class="btn btn-outline-secondary btn-sm">
<span>Log Expanded Structure</span>
<button (click)="logHierarchicalStructure()" class="btn btn-outline-secondary btn-sm">
<span>Log Hierarchical Structure</span>
</button>
</div>

Expand All @@ -54,4 +54,4 @@ <h2>
[datasetHierarchical]="datasetHierarchical"
(onAngularGridCreated)="angularGridReady($event)">
</angular-slickgrid>
</div>
</div>
2 changes: 1 addition & 1 deletion src/app/examples/grid-tree-data-hierarchical.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ export class GridTreeDataHierarchicalComponent implements OnInit {
this.angularGrid.treeDataService.toggleTreeDataCollapse(false);
}

logExpandedStructure() {
logHierarchicalStructure() {
console.log('exploded array', this.angularGrid.treeDataService.datasetHierarchical /* , JSON.stringify(explodedArray, null, 2) */);
}

Expand Down
8 changes: 4 additions & 4 deletions src/app/examples/grid-tree-data-parent-child.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ <h2>
<span>Expand All</span>
</button>
<button (click)="logFlatStructure()" class="btn btn-outline-secondary btn-sm">
<span>Log Flag Structure</span>
<span>Log Flat Structure</span>
</button>
<button (click)="logExpandedStructure()" class="btn btn-outline-secondary btn-sm">
<span>Log Expanded Structure</span>
<button (click)="logHierarchicalStructure()" class="btn btn-outline-secondary btn-sm">
<span>Log Hierarchical Structure</span>
</button>
</div>
</div>
Expand All @@ -42,4 +42,4 @@ <h2>
[dataset]="dataset"
(onAngularGridCreated)="angularGridReady($event)">
</angular-slickgrid>
</div>
</div>
21 changes: 12 additions & 9 deletions src/app/examples/grid-tree-data-parent-child.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,16 @@ export class GridTreeDataParentChildComponent implements OnInit {
enableTreeData: true, // you must enable this flag for the filtering & sorting to work as expected
treeDataOptions: {
columnId: 'title',
levelPropName: 'indent',
parentPropName: 'parentId'
levelPropName: 'indent', // this is optional, except that in our case we just need to define it because we are adding new item in the demo
parentPropName: 'parentId',

// you can optionally sort by a different column and/or sort direction
initialSort: {
columnId: 'title',
direction: 'ASC'
}
},
showCustomFooter: true,
multiColumnSort: false, // multi-column sorting is not supported with Tree Data, so you need to disable it
// change header/cell row height for material design theme
headerRowHeight: 45,
Expand Down Expand Up @@ -166,7 +173,7 @@ export class GridTreeDataParentChildComponent implements OnInit {
parentId: parentItemFound.id,
title: `Task ${newId}`,
duration: '1 day',
percentComplete: Math.round(Math.random() * 100),
percentComplete: 99,
start: new Date(),
finish: new Date(),
effortDriven: false
Expand All @@ -176,11 +183,7 @@ export class GridTreeDataParentChildComponent implements OnInit {
this.dataset = [...dataset]; // make a copy to trigger a dataset refresh

// add setTimeout to wait a full cycle because datasetChanged needs a full cycle
// force a resort because of the tree data structure
setTimeout(() => {
const titleColumn = this.columnDefinitions.find(col => col.id === 'title') as Column;
this.angularGrid.sortService.onLocalSortChanged(this.gridObj, [{ columnId: 'title', sortCol: titleColumn, sortAsc: true }]);

// scroll into the position, after insertion cycle, where the item was added
const rowIndex = this.dataViewObj.getRowById(newItem.id);
this.gridObj.scrollRowIntoView(rowIndex + 3);
Expand All @@ -195,7 +198,7 @@ export class GridTreeDataParentChildComponent implements OnInit {
this.angularGrid.treeDataService.toggleTreeDataCollapse(false);
}

logExpandedStructure() {
logHierarchicalStructure() {
console.log('exploded array', this.angularGrid.treeDataService.datasetHierarchical /* , JSON.stringify(explodedArray, null, 2) */);
}

Expand Down Expand Up @@ -232,8 +235,8 @@ export class GridTreeDataParentChildComponent implements OnInit {
}

d['id'] = i;
d['indent'] = indent;
d['parentId'] = parentId;
d['indent'] = indent;
d['title'] = 'Task ' + i;
d['duration'] = '5 days';
d['percentComplete'] = Math.round(Math.random() * 100);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ const filterServiceStub = {
bindLocalOnSort: jest.fn(),
bindBackendOnSort: jest.fn(),
populateColumnFilterSearchTermPresets: jest.fn(),
refreshTreeDataFilters: jest.fn(),
getColumnFilters: jest.fn(),
} as unknown as FilterService;

Expand Down Expand Up @@ -150,10 +151,13 @@ const sortServiceStub = {
dispose: jest.fn(),
loadGridSorters: jest.fn(),
processTreeDataInitialSort: jest.fn(),
sortHierarchicalDataset: jest.fn(),
} as unknown as SortService;

const treeDataServiceStub = {
convertFlatDatasetConvertToHierarhicalView: jest.fn(),
init: jest.fn(),
initializeHierarchicalDataset: jest.fn(),
dispose: jest.fn(),
handleOnCellClick: jest.fn(),
toggleTreeDataCollapse: jest.fn(),
Expand Down Expand Up @@ -1633,16 +1637,22 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
}
});

it('should change flat dataset and expect being called with other methods', () => {
it('should change flat dataset and expect being called with other methods', (done) => {
const mockFlatDataset = [{ id: 0, file: 'documents' }, { id: 1, file: 'vacation.txt', parentId: 0 }];
const mockHierarchical = [{ id: 0, file: 'documents', files: [{ id: 1, file: 'vacation.txt' }] }];
const hierarchicalSpy = jest.spyOn(SharedService.prototype, 'hierarchicalDataset', 'set');
jest.spyOn(treeDataServiceStub, 'initializeHierarchicalDataset').mockReturnValue({ hierarchical: mockHierarchical, flat: mockFlatDataset });
const refreshTreeSpy = jest.spyOn(filterServiceStub, 'refreshTreeDataFilters');

component.gridOptions = { enableTreeData: true, treeDataOptions: { columnId: 'file', parentPropName: 'parentId', childrenPropName: 'files' } } as GridOption;
component.ngAfterViewInit();
component.dataset = mockFlatDataset;

expect(hierarchicalSpy).toHaveBeenCalledWith(mockHierarchical);
setTimeout(() => {
expect(hierarchicalSpy).toHaveBeenCalledWith(mockHierarchical);
expect(refreshTreeSpy).not.toHaveBeenCalled();
done();
})
});

it('should change hierarchical dataset and expect processTreeDataInitialSort being called with other methods', (done) => {
Expand All @@ -1665,6 +1675,22 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
done();
}, 2);
});

it('should expect "refreshTreeDataFilters" method to be called when our flat dataset was already set and it just got changed a 2nd time', () => {
const mockFlatDataset = [{ id: 0, file: 'documents' }, { id: 1, file: 'vacation.txt', parentId: 0 }];
const mockHierarchical = [{ id: 0, file: 'documents', files: [{ id: 1, file: 'vacation.txt' }] }];
const hierarchicalSpy = jest.spyOn(SharedService.prototype, 'hierarchicalDataset', 'set');
jest.spyOn(treeDataServiceStub, 'initializeHierarchicalDataset').mockReturnValue({ hierarchical: mockHierarchical, flat: mockFlatDataset });
const refreshTreeSpy = jest.spyOn(filterServiceStub, 'refreshTreeDataFilters');

component.dataset = [{ id: 0, file: 'documents' }];
component.gridOptions = { enableTreeData: true, treeDataOptions: { columnId: 'file', parentPropName: 'parentId', childrenPropName: 'files' } } as GridOption;
component.ngAfterViewInit();
component.dataset = mockFlatDataset;

expect(hierarchicalSpy).toHaveBeenCalledWith(mockHierarchical);
expect(refreshTreeSpy).toHaveBeenCalled();
});
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,9 @@ import {
RowMoveManagerExtension,
RowSelectionExtension,
} from '../../extensions';
import * as utilities from '../../services/utilities';

declare const Slick: any;

function removeExtraSpaces(text: string) {
return `${text}`.replace(/\s{2,}/g, '');
}

const mockConvertParentChildArray = jest.fn();
// @ts-ignore
utilities.convertParentChildArrayToHierarchicalView = mockConvertParentChildArray;

const excelExportServiceStub = {
init: jest.fn(),
dispose: jest.fn(),
Expand Down Expand Up @@ -123,7 +114,9 @@ const sortServiceStub = {
} as unknown as SortService;

const treeDataServiceStub = {
convertFlatDatasetConvertToHierarhicalView: jest.fn(),
init: jest.fn(),
initializeHierarchicalDataset: jest.fn(),
dispose: jest.fn(),
handleOnCellClick: jest.fn(),
toggleTreeDataCollapse: jest.fn(),
Expand Down Expand Up @@ -237,6 +230,7 @@ describe('App Component', () => {
it('should convert parent/child dataset to hierarchical dataset when Tree Data is enabled and "onRowsChanged" was triggered', async (done) => {
const mockFlatDataset = [{ id: 0, file: 'documents' }, { id: 1, file: 'vacation.txt', parentId: 0 }];
const hierarchicalSpy = jest.spyOn(SharedService.prototype, 'hierarchicalDataset', 'set');
jest.spyOn(treeDataServiceStub, 'initializeHierarchicalDataset').mockReturnValue({ hierarchical: [], flat: mockFlatDataset });

component.gridId = 'grid1';
component.gridOptions = { enableTreeData: true, treeDataOptions: { columnId: 'file' } } as GridOption;
Expand All @@ -250,7 +244,6 @@ describe('App Component', () => {

setTimeout(() => {
expect(hierarchicalSpy).toHaveBeenCalled();
expect(mockConvertParentChildArray).toHaveBeenCalled();
done();
}, 51)
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import {
Pagination,
ServicePagination,
SlickEventHandler,
TreeDataOption,
} from './../models/index';
import { FilterFactory } from '../filters/filterFactory';
import { autoAddEditorFormatterToColumnsWithEditor } from './slick-vanilla-utilities';
Expand Down Expand Up @@ -124,6 +123,7 @@ const slickgridEventPrefix = 'sg';
export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnInit {
private _dataset?: any[] | null;
private _columnDefinitions!: Column[];
private _currentDatasetLength = 0;
private _eventHandler: SlickEventHandler = new Slick.EventHandler();
private _angularGridInstances: AngularGridInstance | undefined;
private _fixedHeight?: number | null;
Expand Down Expand Up @@ -213,9 +213,24 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
get dataset(): any[] {
return this.dataView.getItems();
}
set dataset(dataset: any[]) {
this._dataset = dataset;
this.refreshGridData(dataset);
set dataset(newDataset: any[]) {
const prevDatasetLn = this._currentDatasetLength;
let data = newDataset;
this._dataset = data;

// when Tree Data is enabled and we don't yet have the hierarchical dataset filled, we can force a convert & sort of the array
if (this.gridOptions?.enableTreeData && Array.isArray(newDataset) && (newDataset.length > 0 || newDataset.length !== prevDatasetLn)) {
const sortedDatasetResult = this.treeDataService.initializeHierarchicalDataset(data, this._columnDefinitions);
this.sharedService.hierarchicalDataset = sortedDatasetResult.hierarchical;
data = sortedDatasetResult.flat;

// if we add/remove item(s) from the dataset, we need to also refresh our tree data filters
if (newDataset.length > 0 && prevDatasetLn > 0 && newDataset.length !== prevDatasetLn) {
this.filterService.refreshTreeDataFilters();
}
}
this.refreshGridData(data || []);
this._currentDatasetLength = newDataset.length;
}

@Input()
Expand All @@ -225,13 +240,13 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
set datasetHierarchical(newHierarchicalDataset: any[] | undefined) {
this.sharedService.hierarchicalDataset = newHierarchicalDataset;

if (newHierarchicalDataset && this.columnDefinitions && this.filterService && this.filterService.clearFilters) {
if (newHierarchicalDataset && this.columnDefinitions && this.filterService?.clearFilters) {
this.filterService.clearFilters();
}

// when a hierarchical dataset is set afterward, we can reset the flat dataset and call a tree data sort that will overwrite the flat dataset
setTimeout(() => {
if (newHierarchicalDataset && this.dataView && this.sortService && this.sortService.processTreeDataInitialSort && this.gridOptions && this.gridOptions.enableTreeData) {
if (newHierarchicalDataset && this.dataView && this.sortService?.processTreeDataInitialSort && this.gridOptions?.enableTreeData) {
this.dataView.setItems([], this.gridOptions.datasetIdPropertyName);
this.sortService.processTreeDataInitialSort();
}
Expand Down Expand Up @@ -426,7 +441,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn

if (Array.isArray(dataset) && this.grid && this.dataView && typeof this.dataView.setItems === 'function') {
this.dataView.setItems(dataset, this.gridOptions.datasetIdPropertyName);
if (!this.gridOptions.backendServiceApi) {
if (!this.gridOptions.backendServiceApi && !this.gridOptions.enableTreeData) {
this.dataView.reSort();
}

Expand All @@ -439,11 +454,6 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
}
}
this._isDatasetInitialized = true;

// also update the hierarchical dataset
if (dataset.length > 0 && this.gridOptions.treeDataOptions) {
this.sharedService.hierarchicalDataset = this.treeDataSortComparer(dataset);
}
}

if (dataset) {
Expand Down Expand Up @@ -667,16 +677,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
throw new Error('[Angular-Slickgrid] When enabling tree data, you must also provide the "treeDataOption" property in your Grid Options with "childrenPropName" or "parentPropName" (depending if your array is hierarchical or flat) for the Tree Data to work properly');
}

this._eventHandler.subscribe(dataView.onRowsChanged, (e: any, args: any) => {
// when dealing with Tree Data, anytime the flat dataset changes, we need to update our hierarchical dataset
// this could be triggered by a DataView setItems or updateItem
if (this.gridOptions && this.gridOptions.enableTreeData) {
const items = this.dataView.getItems();
if (Array.isArray(items) && items.length > 0 && !this._isDatasetInitialized) {
this.sharedService.hierarchicalDataset = this.treeDataSortComparer(items);
}
}

this._eventHandler.subscribe(dataView.onRowsChanged, (_e: Event, args: any) => {
// filtering data with local dataset will not always show correctly unless we call this updateRow/render
// also don't use "invalidateRows" since it destroys the entire row and as bad user experience when updating a row
// see commit: https://github.com/ghiscoding/Angular-Slickgrid/commit/bb62c0aa2314a5d61188ff005ccb564577f08805
Expand Down Expand Up @@ -805,6 +806,7 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn

/** When data changes in the DataView, we'll refresh the metrics and/or display a warning if the dataset is empty */
private handleOnItemCountChanged(currentPageRowItemCount: number, totalItemCount: number) {
this._currentDatasetLength = totalItemCount;
this.metrics = {
startTime: new Date(),
endTime: new Date(),
Expand Down Expand Up @@ -1218,13 +1220,6 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
}
}

private treeDataSortComparer(flatDataset: any[]): any[] {
const dataViewIdIdentifier = this.gridOptions && this.gridOptions.datasetIdPropertyName || 'id';
const treeDataOpt: TreeDataOption = this.gridOptions && this.gridOptions.treeDataOptions || { columnId: '' };
const treeDataOptions = { ...treeDataOpt, identifierPropName: treeDataOpt.identifierPropName || dataViewIdIdentifier };
return convertParentChildArrayToHierarchicalView(flatDataset, treeDataOptions);
}

/**
* For convenience to the user, we provide the property "editor" as an Angular-Slickgrid editor complex object
* however "editor" is used internally by SlickGrid for it's own Editor Factory
Expand Down

0 comments on commit 0b120f4

Please sign in to comment.