Skip to content

Commit

Permalink
fix(tree): same dataset length but w/different prop should refresh Tree
Browse files Browse the repository at this point in the history
- the issue was found in our LIG project in the use case that the dataset being replaced by the dataset SETTER, it wasn't refreshing the entire Tree Data in the grid and because of that the UX seemed broken, so the fix is to use `dequal` to compare the dataset with all its properties (not just its length) and if anything changed then we call (force) a full Tree Data refresh
  • Loading branch information
ghiscoding committed Jul 16, 2021
1 parent 7edc3bf commit 549008a
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 6 deletions.
Expand Up @@ -2147,6 +2147,22 @@ describe('Slick-Vanilla-Grid-Bundle Component instantiated via Constructor', ()
expect(hierarchicalSpy).toHaveBeenCalledWith(mockHierarchical);
expect(refreshTreeSpy).toHaveBeenCalled();
});

it('should also expect "refreshTreeDataFilters" method to be called even when the dataset length is the same but still has different properties (e.g. different filename)', () => {
const mockFlatDataset = [{ id: 0, file: 'documents' }, { id: 1, file: 'new-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, 'convertFlatParentChildToTreeDatasetAndSort').mockReturnValue({ hierarchical: mockHierarchical as any[], flat: mockFlatDataset as any[] });
const refreshTreeSpy = jest.spyOn(filterServiceStub, 'refreshTreeDataFilters');

component.dataset = [{ id: 0, file: 'documents' }, { id: 1, file: 'old-vacation.txt', parentId: 0 }];
component.gridOptions = { enableTreeData: true, treeDataOptions: { columnId: 'file', parentPropName: 'parentId', childrenPropName: 'files', initialSort: { columndId: 'file', direction: 'ASC' } } } as unknown as GridOption;
component.initialization(divContainer, slickEventHandler);
component.dataset = mockFlatDataset;

expect(hierarchicalSpy).toHaveBeenCalledWith(mockHierarchical);
expect(refreshTreeSpy).toHaveBeenCalled();
});
});
});
});
Expand Down
@@ -1,3 +1,4 @@
import { dequal } from 'dequal/lite';
import 'flatpickr/dist/l10n/fr';
import 'slickgrid/lib/jquery.event.drag-2.3.0';
import 'slickgrid/lib/jquery.mousewheel';
Expand Down Expand Up @@ -169,13 +170,14 @@ export class SlickVanillaGridBundle {
}
set dataset(newDataset: any[]) {
const prevDatasetLn = this._currentDatasetLength;
const isDatasetEqual = dequal(newDataset, this.dataset || []);
const isDeepCopyDataOnPageLoadEnabled = !!(this._gridOptions?.enableDeepCopyDatasetOnPageLoad);
let data = isDeepCopyDataOnPageLoadEnabled ? $.extend(true, [], newDataset) : newDataset;

// 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.slickGrid && this.gridOptions?.enableTreeData && Array.isArray(newDataset) && (newDataset.length > 0 || newDataset.length !== prevDatasetLn)) {
// 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.slickGrid && this.gridOptions?.enableTreeData && Array.isArray(newDataset) && (newDataset.length > 0 || newDataset.length !== prevDatasetLn || !isDatasetEqual)) {
this._isDatasetHierarchicalInitialized = false;
data = this.sortTreeDataset(newDataset);
data = this.sortTreeDataset(newDataset, !isDatasetEqual); // if dataset changed, then force a refresh anyway
}

this.refreshGridData(data || []);
Expand All @@ -193,6 +195,7 @@ export class SlickVanillaGridBundle {
}

set datasetHierarchical(newHierarchicalDataset: any[] | undefined) {
const isDatasetEqual = dequal(newHierarchicalDataset, this.sharedService.hierarchicalDataset || []);
const prevFlatDatasetLn = this._currentDatasetLength;
this.sharedService.hierarchicalDataset = newHierarchicalDataset;

Expand All @@ -209,7 +212,7 @@ export class SlickVanillaGridBundle {
// however we need 1 cpu cycle before having the DataView refreshed, so we need to wrap this check in a setTimeout
setTimeout(() => {
const flatDatasetLn = this.dataView.getItemCount();
if (flatDatasetLn !== prevFlatDatasetLn && flatDatasetLn > 0) {
if (flatDatasetLn > 0 && (flatDatasetLn !== prevFlatDatasetLn || !isDatasetEqual)) {
this.filterService.refreshTreeDataFilters();
}
});
Expand Down Expand Up @@ -1410,7 +1413,7 @@ export class SlickVanillaGridBundle {
* Takes a flat dataset with parent/child relationship, sort it (via its tree structure) and return the sorted flat array
* @returns {Array<Object>} sort flat parent/child dataset
*/
private sortTreeDataset<T>(flatDatasetInput: T[]): T[] {
private sortTreeDataset<T>(flatDatasetInput: T[], forceGridRefresh = false): T[] {
const prevDatasetLn = this._currentDatasetLength;
let sortedDatasetResult;
let flatDatasetOutput: any[] = [];
Expand All @@ -1434,7 +1437,7 @@ export class SlickVanillaGridBundle {
}

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

Expand Down

0 comments on commit 549008a

Please sign in to comment.