fix(TreeData): addItem should keep current sorted column (#1558)

* fix(TreeData): addItem should keep current sorted column
- prior to this PR, calling `addItem()` (or `addItems()`) was always resorting using the Tree Data initial sort because the method being called is the same for the initial Tree Data first load and/or adding an item. So we should really reuse any existing current sort when resorting the updated tree.

examples/vite-demo-vanilla-bundle/src/examples/example05.html

@@ -47,7 +47,7 @@ <h6 class="title is-6 italic">
     <div class="row mb-1">
       <button onclick.delegate="addNewRow()" data-test="add-item-btn" class="button is-small is-info">
         <span class="mdi mdi-plus"></span>
-        <span>Add New Item (in 1st group)</span>
+        <span>Add New Item to "Task 1" group</span>
       </button>
       <button onclick.delegate="updateFirstRow()" data-test="update-item-btn" class="button is-small">
         <span class="mdi mdi-pencil"></span>

examples/vite-demo-vanilla-bundle/src/examples/example05.ts

@@ -272,19 +272,15 @@ export default class Example05 {
   }
 
   /**
-   * A simple method to add a new item inside the first group that we find (it's random and is only for demo purposes).
-   * After adding the item, it will sort by parent/child recursively
+   * A simple method to add a new item inside the first group that has children which is "Task 1"
+   * After adding the item, it will resort by parent/child recursively but keep current sort column
    */
   addNewRow() {
     const newId = this.sgb.dataset.length;
-    const parentPropName = 'parentId';
-    const treeLevelPropName = 'treeLevel'; // if undefined in your options, the default prop name is "__treeLevel"
-    const newTreeLevel = 1;
-    // find first parent object and add the new item as a child
-    const childItemFound = this.sgb.dataset.find((item) => item[treeLevelPropName] === newTreeLevel);
-    const parentItemFound = this.sgb.dataView?.getItemByIdx(childItemFound[parentPropName]);
+    // find "Task 1" which has `id = 1`
+    const parentItemFound = this.sgb.dataView?.getItemById(1);
 
-    if (childItemFound && parentItemFound) {
+    if (parentItemFound?.__hasChildren) {
       const newItem = {
         id: newId,
         parentId: parentItemFound.id,

packages/common/src/services/__tests__/grid.service.spec.ts

@@ -36,6 +36,7 @@ const pubSubServiceStub = {
 
 const sortServiceStub = {
   clearSorting: jest.fn(),
+  getCurrentColumnSorts: jest.fn(),
 } as unknown as SortService;
 
 const dataviewStub = {
@@ -950,6 +951,7 @@ describe('Grid Service', () => {
 
       jest.spyOn(dataviewStub, 'getItems').mockReturnValue(mockFlatDataset);
       jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(0);
+      jest.spyOn(sortServiceStub, 'getCurrentColumnSorts').mockReturnValueOnce([{ columnId: 'title', sortCol: mockColumns[0], sortAsc: false }]);
       jest.spyOn(treeDataServiceStub, 'convertFlatParentChildToTreeDatasetAndSort').mockReturnValue({ flat: mockFlatDataset as any[], hierarchical: mockHierarchical as any[] });
       jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, enableRowSelection: true, enableTreeData: true } as GridOption);
       jest.spyOn(SharedService.prototype, 'allColumns', 'get').mockReturnValue(mockColumns);
@@ -977,6 +979,7 @@ describe('Grid Service', () => {
 
       jest.spyOn(dataviewStub, 'getItems').mockReturnValue(mockFlatDataset);
       jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(0);
+      jest.spyOn(sortServiceStub, 'getCurrentColumnSorts').mockReturnValueOnce([{ columnId: 'title', sortCol: mockColumns[0], sortAsc: false }]);
       jest.spyOn(treeDataServiceStub, 'convertFlatParentChildToTreeDatasetAndSort').mockReturnValue({ flat: mockFlatDataset as any[], hierarchical: mockHierarchical as any[] });
       jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, enableRowSelection: true, enableTreeData: true } as GridOption);
       jest.spyOn(SharedService.prototype, 'allColumns', 'get').mockReturnValue(mockColumns);
@@ -1004,6 +1007,7 @@ describe('Grid Service', () => {
 
       jest.spyOn(dataviewStub, 'getItems').mockReturnValue(mockFlatDataset);
       jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(0);
+      jest.spyOn(sortServiceStub, 'getCurrentColumnSorts').mockReturnValueOnce([{ columnId: 'title', sortCol: mockColumns[0], sortAsc: true }]);
       jest.spyOn(treeDataServiceStub, 'convertFlatParentChildToTreeDatasetAndSort').mockReturnValue({ flat: mockFlatDataset as any[], hierarchical: mockHierarchical as any[] });
       jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, enableRowSelection: true, enableTreeData: true } as GridOption);
       jest.spyOn(SharedService.prototype, 'allColumns', 'get').mockReturnValue(mockColumns);
@@ -1031,6 +1035,7 @@ describe('Grid Service', () => {
 
       jest.spyOn(dataviewStub, 'getItems').mockReturnValue(mockFlatDataset);
       jest.spyOn(dataviewStub, 'getRowById').mockReturnValue(0);
+      jest.spyOn(sortServiceStub, 'getCurrentColumnSorts').mockReturnValueOnce([{ columnId: 'title', sortCol: mockColumns[0], sortAsc: true }]);
       jest.spyOn(treeDataServiceStub, 'convertFlatParentChildToTreeDatasetAndSort').mockReturnValue({ flat: mockFlatDataset as any[], hierarchical: mockHierarchical as any[] });
       jest.spyOn(gridStub, 'getOptions').mockReturnValue({ enableAutoResize: true, enableRowSelection: true, enableTreeData: true } as GridOption);
       jest.spyOn(SharedService.prototype, 'allColumns', 'get').mockReturnValue(mockColumns);

packages/common/src/services/__tests__/treeData.service.spec.ts

@@ -375,7 +375,7 @@ describe('TreeData Service', () => {
       expect(beginUpdateSpy).toHaveBeenCalled();
       expect(updateItemSpy).toHaveBeenNthCalledWith(1, 0, { __collapsed: true, __hasChildren: true, id: 0, file: 'TXT', size: 5.8, __treeLevel: 0 });
       expect(updateItemSpy).toHaveBeenNthCalledWith(2, 4, { __collapsed: true, __hasChildren: true, id: 4, file: 'MP3', size: 3.4, __treeLevel: 0 });
-      expect(SharedService.prototype.hierarchicalDataset![0].file).toBe('TXT')
+      expect(SharedService.prototype.hierarchicalDataset![0].file).toBe('TXT');
       expect(SharedService.prototype.hierarchicalDataset![0].__collapsed).toBeTrue();
       expect(SharedService.prototype.hierarchicalDataset![1].file).toBe('MP3');
       expect(SharedService.prototype.hierarchicalDataset![1].__collapsed).toBeTrue();
@@ -642,6 +642,32 @@ describe('TreeData Service', () => {
       }]);
       expect(result).toEqual({ flat: mockFlatDataset as any[], hierarchical: mockHierarchical as any[] });
     });
+
+    it('should sort Tree by provided Sort Column when provided', () => {
+      gridOptionsMock.treeDataOptions!.initialSort = {
+        columnId: 'size',
+        direction: 'desc'
+      };
+      const mockHierarchical = [{
+        id: 0,
+        file: 'documents',
+        files: [{ id: 1, file: 'vacation.txt', size: 1.2, }, { id: 2, file: 'todo.txt', size: 2.3, }]
+      }];
+      const setSortSpy = jest.spyOn(gridStub, 'setSortColumns');
+      jest.spyOn(gridStub, 'getColumnIndex').mockReturnValue(0);
+      jest.spyOn(sortServiceStub, 'sortHierarchicalDataset').mockReturnValue({ flat: mockFlatDataset as any[], hierarchical: mockHierarchical as any[] });
+
+      service.init(gridStub);
+      const sortCols = [{ columnId: mockColumns[0].id, sortCol: mockColumns[0], sortAsc: false }];
+      const result = service.convertFlatParentChildToTreeDatasetAndSort(mockFlatDataset, mockColumns, gridOptionsMock, sortCols);
+
+      expect(setSortSpy).toHaveBeenCalledWith([{
+        columnId: mockColumns[0].id,
+        sortAsc: false,
+        sortCol: mockColumns[0]
+      }]);
+      expect(result).toEqual({ flat: mockFlatDataset as any[], hierarchical: mockHierarchical as any[] });
+    });
   });
 
   describe('sortHierarchicalDataset method', () => {
@@ -652,7 +678,7 @@ describe('TreeData Service', () => {
         file: 'documents',
         files: [{ id: 2, file: 'todo.txt', size: 2.3, }, { id: 1, file: 'vacation.txt', size: 1.2, }]
       }];
-      const mockColumnSort = { columnId: 'size', sortAsc: true, sortCol: mockColumns[1], }
+      const mockColumnSort = { columnId: 'size', sortAsc: true, sortCol: mockColumns[1], };
       jest.spyOn(SharedService.prototype, 'allColumns', 'get').mockReturnValue(mockColumns);
       const getInitialSpy = jest.spyOn(service, 'getInitialSort').mockReturnValue(mockColumnSort);
       const sortHierarchySpy = jest.spyOn(sortServiceStub, 'sortHierarchicalDataset');

packages/common/src/services/grid.service.ts

@@ -834,15 +834,16 @@ export class GridService {
 
   /**
    * When dealing with hierarchical (tree) dataset, we can invalidate all the rows and force a full resort & re-render of the hierarchical tree dataset.
-   * This method will automatically be called anytime user called `addItem()` or `addItems()`.
+   * This method will automatically be called anytime user called `addItem()` or `addItems()` and it will reuse current column sorting when found (or use initial sort).
    * However please note that it won't be called when `updateItem`, if the data that gets updated does change the tree data column then you should call this method.
    * @param {Array<Object>} [items] - optional flat array of parent/child items to use while redoing the full sort & refresh
    */
   invalidateHierarchicalDataset(items?: any[]) {
     // if we add/remove item(s) from the dataset, we need to also refresh our tree data filters
     if (this._gridOptions?.enableTreeData && this.treeDataService) {
       const inputItems = items ?? this._dataView.getItems();
-      const sortedDatasetResult = this.treeDataService.convertFlatParentChildToTreeDatasetAndSort(inputItems || [], this.sharedService.allColumns, this._gridOptions);
+      const sortCols = this.sortService.getCurrentColumnSorts();
+      const sortedDatasetResult = this.treeDataService.convertFlatParentChildToTreeDatasetAndSort(inputItems || [], this.sharedService.allColumns, this._gridOptions, sortCols);
       this.sharedService.hierarchicalDataset = sortedDatasetResult.hierarchical;
       this.filterService.refreshTreeDataFilters(items);
       this._dataView.setItems(sortedDatasetResult.flat);

packages/common/src/services/treeData.service.ts

@@ -287,15 +287,16 @@ export class TreeDataService {
    * @param {Array<Object>} flatDataset - parent/child flat dataset
    * @param {Column[]} columnDefinitions - column definitions
    * @param {Object} gridOptions - grid options
+   * @param {Array<ColumnSort>} [columnSorts] - optional sort columns
    * @returns {Array<Object>} - tree dataset
    */
-  convertFlatParentChildToTreeDatasetAndSort<P, T extends P & { [childrenPropName: string]: T[]; }>(flatDataset: P[], columnDefinitions: Column[], gridOptions: GridOption) {
+  convertFlatParentChildToTreeDatasetAndSort<P, T extends P & { [childrenPropName: string]: T[]; }>(flatDataset: P[], columnDefinitions: Column[], gridOptions: GridOption, columnSorts?: ColumnSort[]) {
     // 1- convert the flat array into a hierarchical array
     const datasetHierarchical = this.convertFlatParentChildToTreeDataset(flatDataset, gridOptions);
 
     // 2- sort the hierarchical array recursively by an optional "initialSort" OR if nothing is provided we'll sort by the column defined as the Tree column
-    // also note that multi-column is not currently supported with Tree Data
-    const columnSort = this.getInitialSort(columnDefinitions, gridOptions);
+    // also note that multi-column is not currently supported with Tree Data and so we'll take only the first Sort column
+    const columnSort = Array.isArray(columnSorts) && columnSorts.length ? columnSorts[0] : this.getInitialSort(columnDefinitions, gridOptions);
     const datasetSortResult = this.sortService.sortHierarchicalDataset(datasetHierarchical, [columnSort], true);
 
     // and finally add the sorting icon (this has to be done manually in SlickGrid) to the column we used for the sorting