Skip to content
This repository was archived by the owner on Jun 1, 2025. It is now read-only.

Commit e24c6de

Browse files
ghiscodingghiscoding
authored
fix(pinning): reordering cols position freezing cols shouldn't affect (#708)
- the steps to reproduce the issue was to create a grid with `frozenColumn` in the grid options, then change column position (cols reordering) and finally open header menu and freeze any of the column and the bug was that it was going back to original positions while it should keep new positions and just add new freeze column
1 parent 2d686ff commit e24c6de

File tree

9 files changed

+102
-52
lines changed

9 files changed

+102
-52
lines changed

‎src/app/examples/grid-colspan.component.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ export class GridColspanComponent implements OnInit {
5454
this.gridOptions1 = {
5555
enableAutoResize: false,
5656
enableCellNavigation: true,
57-
enableColumnReorder: false,
5857
enableSorting: true,
5958
createPreHeaderPanel: true,
6059
showPreHeaderPanel: true,
@@ -79,7 +78,6 @@ export class GridColspanComponent implements OnInit {
7978

8079
this.gridOptions2 = {
8180
enableCellNavigation: true,
82-
enableColumnReorder: false,
8381
createPreHeaderPanel: true,
8482
showPreHeaderPanel: true,
8583
preHeaderPanelHeight: 25,

‎src/app/modules/angular-slickgrid/components/__tests__/angular-slickgrid-constructor.spec.ts‎

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
305305
component.gridId = 'grid1';
306306
component.columnDefinitions = [{ id: 'name', field: 'name' }];
307307
component.dataset = [];
308-
component.gridOptions = { enableExcelExport: false, dataView: null } as GridOption;
308+
component.gridOptions = { enableExcelExport: false, dataView: null } as unknown as GridOption;
309309
component.gridHeight = 600;
310310
component.gridWidth = 800;
311311
});
@@ -469,9 +469,9 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
469469

470470
setTimeout(() => {
471471
expect(getColSpy).toHaveBeenCalled();
472-
expect(component.columnDefinitions[0].editor.collection).toEqual(mockCollection);
473-
expect(component.columnDefinitions[0].internalColumnEditor.collection).toEqual(mockCollection);
474-
expect(component.columnDefinitions[0].internalColumnEditor.model).toEqual(Editors.text);
472+
expect(component.columnDefinitions[0].editor!.collection).toEqual(mockCollection);
473+
expect(component.columnDefinitions[0].internalColumnEditor!.collection).toEqual(mockCollection);
474+
expect(component.columnDefinitions[0].internalColumnEditor!.model).toEqual(Editors.text);
475475
done();
476476
});
477477
});
@@ -722,9 +722,9 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
722722
component.ngAfterViewInit();
723723

724724
setTimeout(() => {
725-
expect(component.paginationOptions.pageSize).toBe(2);
726-
expect(component.paginationOptions.pageNumber).toBe(expectedPageNumber);
727-
expect(component.paginationOptions.totalItems).toBe(expectedTotalItems);
725+
expect(component.paginationOptions!.pageSize).toBe(2);
726+
expect(component.paginationOptions!.pageNumber).toBe(expectedPageNumber);
727+
expect(component.paginationOptions!.totalItems).toBe(expectedTotalItems);
728728
expect(refreshSpy).toHaveBeenCalledWith(mockData);
729729
done();
730730
});
@@ -749,9 +749,9 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
749749

750750
setTimeout(() => {
751751
expect(getPagingSpy).toHaveBeenCalled();
752-
expect(component.paginationOptions.pageSize).toBe(10);
753-
expect(component.paginationOptions.pageNumber).toBe(expectedPageNumber);
754-
expect(component.paginationOptions.totalItems).toBe(expectedTotalItems);
752+
expect(component.paginationOptions!.pageSize).toBe(10);
753+
expect(component.paginationOptions!.pageNumber).toBe(expectedPageNumber);
754+
expect(component.paginationOptions!.totalItems).toBe(expectedTotalItems);
755755
expect(refreshSpy).toHaveBeenCalledWith(mockData);
756756
done();
757757
});
@@ -781,55 +781,55 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
781781
component.ngAfterViewInit();
782782

783783
expect(spy).toHaveBeenCalled();
784-
expect(component.gridOptions.backendServiceApi.internalPostProcess).toEqual(expect.any(Function));
784+
expect(component.gridOptions.backendServiceApi!.internalPostProcess).toEqual(expect.any(Function));
785785
});
786786

787787
it('should execute the "internalPostProcess" callback method that was created by "createBackendApiInternalPostProcessCallback" with Pagination', () => {
788-
jest.spyOn(component.gridOptions.backendServiceApi.service, 'getDatasetName').mockReturnValue('users');
788+
jest.spyOn(component.gridOptions.backendServiceApi!.service, 'getDatasetName').mockReturnValue('users');
789789
const spy = jest.spyOn(component, 'refreshGridData');
790790

791791
component.ngAfterViewInit();
792-
component.gridOptions.backendServiceApi.internalPostProcess({ data: { users: { nodes: [{ firstName: 'John' }], totalCount: 2 } } } as GraphqlPaginatedResult);
792+
component.gridOptions.backendServiceApi!.internalPostProcess!({ data: { users: { nodes: [{ firstName: 'John' }], totalCount: 2 } } } as GraphqlPaginatedResult);
793793

794794
expect(spy).toHaveBeenCalled();
795-
expect(component.gridOptions.backendServiceApi.internalPostProcess).toEqual(expect.any(Function));
795+
expect(component.gridOptions.backendServiceApi!.internalPostProcess).toEqual(expect.any(Function));
796796
});
797797

798798
it('should execute the "internalPostProcess" callback and expect totalItems to be updated in the PaginationService when "refreshGridData" is called on the 2nd time', () => {
799-
jest.spyOn(component.gridOptions.backendServiceApi.service, 'getDatasetName').mockReturnValue('users');
799+
jest.spyOn(component.gridOptions.backendServiceApi!.service, 'getDatasetName').mockReturnValue('users');
800800
const refreshSpy = jest.spyOn(component, 'refreshGridData');
801801
const paginationSpy = jest.spyOn(paginationServiceStub, 'totalItems', 'set');
802802
const mockDataset = [{ firstName: 'John' }, { firstName: 'Jane' }];
803803

804804
component.ngAfterViewInit();
805-
component.gridOptions.backendServiceApi.internalPostProcess({ data: { users: { nodes: mockDataset, totalCount: mockDataset.length } } } as GraphqlPaginatedResult);
805+
component.gridOptions.backendServiceApi!.internalPostProcess!({ data: { users: { nodes: mockDataset, totalCount: mockDataset.length } } } as GraphqlPaginatedResult);
806806
component.refreshGridData(mockDataset, 1);
807807
component.refreshGridData(mockDataset, 1);
808808

809809
expect(refreshSpy).toHaveBeenCalledTimes(3);
810810
expect(paginationSpy).toHaveBeenCalledWith(2);
811-
expect(component.gridOptions.backendServiceApi.internalPostProcess).toEqual(expect.any(Function));
811+
expect(component.gridOptions.backendServiceApi!.internalPostProcess).toEqual(expect.any(Function));
812812
});
813813

814814
it('should execute the "internalPostProcess" callback method that was created by "createBackendApiInternalPostProcessCallback" without Pagination (when disabled)', () => {
815815
component.gridOptions.enablePagination = false;
816-
jest.spyOn(component.gridOptions.backendServiceApi.service, 'getDatasetName').mockReturnValue('users');
816+
jest.spyOn(component.gridOptions.backendServiceApi!.service, 'getDatasetName').mockReturnValue('users');
817817
const spy = jest.spyOn(component, 'refreshGridData');
818818

819819
component.ngAfterViewInit();
820-
component.gridOptions.backendServiceApi.internalPostProcess({ data: { users: [{ firstName: 'John' }] } });
820+
component.gridOptions.backendServiceApi!.internalPostProcess!({ data: { users: [{ firstName: 'John' }] } });
821821

822822
expect(spy).toHaveBeenCalled();
823-
expect(component.gridOptions.backendServiceApi.internalPostProcess).toEqual(expect.any(Function));
823+
expect(component.gridOptions.backendServiceApi!.internalPostProcess).toEqual(expect.any(Function));
824824
});
825825

826826
xit('should execute the "internalPostProcess" callback method but return an empty dataset when dataset name does not match "getDatasetName"', () => {
827827
component.gridOptions.enablePagination = true;
828-
jest.spyOn(component.gridOptions.backendServiceApi.service, 'getDatasetName').mockReturnValue('users');
828+
jest.spyOn(component.gridOptions.backendServiceApi!.service, 'getDatasetName').mockReturnValue('users');
829829
const spy = jest.spyOn(component, 'refreshGridData');
830830

831831
component.ngAfterViewInit();
832-
component.gridOptions.backendServiceApi.internalPostProcess({ data: { notUsers: { nodes: [{ firstName: 'John' }], totalCount: 2 } } } as GraphqlPaginatedResult);
832+
component.gridOptions.backendServiceApi!.internalPostProcess!({ data: { notUsers: { nodes: [{ firstName: 'John' }], totalCount: 2 } } } as GraphqlPaginatedResult);
833833

834834
expect(spy).not.toHaveBeenCalled();
835835
expect(component.dataset).toEqual([]);
@@ -902,10 +902,10 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
902902
metrics: { startTime: now, endTime: now, executionTime: 0, totalItemCount: 0 }
903903
};
904904
const promise = new Promise((resolve) => setTimeout(() => resolve(processResult), 1));
905-
const processSpy = jest.spyOn(component.gridOptions.backendServiceApi, 'process').mockReturnValue(promise);
906-
jest.spyOn(component.gridOptions.backendServiceApi.service, 'buildQuery').mockReturnValue(query);
905+
const processSpy = jest.spyOn((component.gridOptions as any).backendServiceApi, 'process').mockReturnValue(promise);
906+
jest.spyOn((component.gridOptions as any).backendServiceApi.service, 'buildQuery').mockReturnValue(query);
907907

908-
component.gridOptions.backendServiceApi.service.options = { executeProcessCommandOnInit: true };
908+
(component.gridOptions as any).backendServiceApi.service.options = { executeProcessCommandOnInit: true };
909909
component.ngAfterViewInit();
910910

911911
expect(processSpy).toHaveBeenCalled();
@@ -923,10 +923,10 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
923923
data: { users: { nodes: [] }, pageInfo: { hasNextPage: true }, totalCount: 0 },
924924
metrics: { startTime: now, endTime: now, executionTime: 0, totalItemCount: 0 }
925925
};
926-
const processSpy = jest.spyOn(component.gridOptions.backendServiceApi, 'process').mockReturnValue(of(processResult));
927-
jest.spyOn(component.gridOptions.backendServiceApi.service, 'buildQuery').mockReturnValue(query);
926+
const processSpy = jest.spyOn((component.gridOptions as any).backendServiceApi, 'process').mockReturnValue(of(processResult));
927+
jest.spyOn((component.gridOptions as any).backendServiceApi.service, 'buildQuery').mockReturnValue(query);
928928

929-
component.gridOptions.backendServiceApi.service.options = { executeProcessCommandOnInit: true };
929+
(component.gridOptions as any).backendServiceApi.service.options = { executeProcessCommandOnInit: true };
930930
component.ngAfterViewInit();
931931

932932
expect(processSpy).toHaveBeenCalled();
@@ -944,10 +944,10 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
944944
data: { users: [] },
945945
metrics: { startTime: now, endTime: now, executionTime: 0, totalItemCount: 0 }
946946
};
947-
const processSpy = jest.spyOn(component.gridOptions.backendServiceApi, 'process').mockReturnValue(of(processResult));
948-
jest.spyOn(component.gridOptions.backendServiceApi.service, 'buildQuery').mockReturnValue(query);
947+
const processSpy = jest.spyOn((component.gridOptions as any).backendServiceApi, 'process').mockReturnValue(of(processResult));
948+
jest.spyOn((component.gridOptions as any).backendServiceApi.service, 'buildQuery').mockReturnValue(query);
949949

950-
component.gridOptions.backendServiceApi.service.options = { executeProcessCommandOnInit: true };
950+
(component.gridOptions as any).backendServiceApi.service.options = { executeProcessCommandOnInit: true };
951951
component.ngAfterViewInit();
952952

953953
expect(processSpy).toHaveBeenCalled();
@@ -962,10 +962,10 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
962962
const mockError = { error: '404' };
963963
const query = `query { users (first:20,offset:0) { totalCount, nodes { id,name,gender,company } } }`;
964964
const promise = new Promise((resolve, reject) => setTimeout(() => reject(mockError), 1));
965-
const processSpy = jest.spyOn(component.gridOptions.backendServiceApi, 'process').mockReturnValue(promise);
966-
jest.spyOn(component.gridOptions.backendServiceApi.service, 'buildQuery').mockReturnValue(query);
965+
const processSpy = jest.spyOn((component.gridOptions as any).backendServiceApi, 'process').mockReturnValue(promise);
966+
jest.spyOn((component.gridOptions as any).backendServiceApi.service, 'buildQuery').mockReturnValue(query);
967967

968-
component.gridOptions.backendServiceApi.service.options = { executeProcessCommandOnInit: true };
968+
(component.gridOptions as any).backendServiceApi.service.options = { executeProcessCommandOnInit: true };
969969
component.ngAfterViewInit();
970970

971971
expect(processSpy).toHaveBeenCalled();
@@ -979,10 +979,10 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
979979
it('should throw an error when the process method on initialization when "executeProcessCommandOnInit" is set as a backend service options from an Observable', (done) => {
980980
const mockError = { error: '404' };
981981
const query = `query { users (first:20,offset:0) { totalCount, nodes { id,name,gender,company } } }`;
982-
const processSpy = jest.spyOn(component.gridOptions.backendServiceApi, 'process').mockReturnValue(throwError(mockError));
983-
jest.spyOn(component.gridOptions.backendServiceApi.service, 'buildQuery').mockReturnValue(query);
982+
const processSpy = jest.spyOn((component.gridOptions as any).backendServiceApi, 'process').mockReturnValue(throwError(mockError));
983+
jest.spyOn((component.gridOptions as any).backendServiceApi.service, 'buildQuery').mockReturnValue(query);
984984

985-
component.gridOptions.backendServiceApi.service.options = { executeProcessCommandOnInit: true };
985+
(component.gridOptions as any).backendServiceApi.service.options = { executeProcessCommandOnInit: true };
986986
component.ngAfterViewInit();
987987

988988
expect(processSpy).toHaveBeenCalled();
@@ -1492,7 +1492,7 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
14921492

14931493
component.gridOptions.enableRowSelection = true;
14941494
component.gridOptions.enablePagination = true;
1495-
component.gridOptions.backendServiceApi = null;
1495+
(component.gridOptions as any).backendServiceApi = null;
14961496
component.gridOptions.presets = { rowSelection: { dataContextIds: selectedGridRows } };
14971497
component.dataset = mockData;
14981498
component.ngAfterViewInit();
@@ -1511,7 +1511,7 @@ describe('Angular-Slickgrid Custom Component instantiated via Constructor', () =
15111511

15121512
it('should change "showPagination" flag when "onPaginationVisibilityChanged" from the Pagination Service is triggered', (done) => {
15131513
component.gridOptions.enablePagination = true;
1514-
component.gridOptions.backendServiceApi = null;
1514+
(component.gridOptions as any).backendServiceApi = null;
15151515
component.ngAfterViewInit();
15161516
paginationServiceStub.onPaginationVisibilityChanged.next({ visible: false });
15171517
setTimeout(() => {

‎src/app/modules/angular-slickgrid/components/__tests__/angular-slickgrid.component.spec.ts‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,8 @@ describe('App Component', () => {
194194
component.gridId = 'grid1';
195195

196196
fixture.detectChanges();
197-
const gridPaneElm = document.querySelector<HTMLDivElement>('.gridPane');
198-
const gridContainerElm = document.querySelector<HTMLDivElement>('.slickgrid-container');
197+
const gridPaneElm = document.querySelector('.gridPane') as HTMLDivElement;
198+
const gridContainerElm = document.querySelector('.slickgrid-container') as HTMLDivElement;
199199

200200
expect(gridPaneElm.id).toBe('slickGridContainer-grid1');
201201
expect(gridContainerElm.id).toBe('grid1');
@@ -209,8 +209,8 @@ describe('App Component', () => {
209209
component.gridOptions = { enableTreeData: true, treeDataOptions: { columnId: 'file' } } as GridOption;
210210
component.dataset = mockFlatDataset;
211211
fixture.detectChanges();
212-
const gridPaneElm = document.querySelector<HTMLDivElement>('.gridPane');
213-
const gridContainerElm = document.querySelector<HTMLDivElement>('.slickgrid-container');
212+
const gridPaneElm = document.querySelector('.gridPane') as HTMLDivElement;
213+
const gridContainerElm = document.querySelector('.slickgrid-container') as HTMLDivElement;
214214

215215
expect(gridPaneElm.id).toBe('slickGridContainer-grid1');
216216
expect(gridContainerElm.id).toBe('grid1');
@@ -227,8 +227,8 @@ describe('App Component', () => {
227227
component.gridWidth = 800;
228228

229229
fixture.detectChanges();
230-
const gridPaneElm = document.querySelector<HTMLDivElement>('.gridPane');
231-
const gridContainerElm = document.querySelector<HTMLDivElement>('.slickgrid-container');
230+
const gridPaneElm = document.querySelector('.gridPane') as HTMLDivElement;
231+
const gridContainerElm = document.querySelector('.slickgrid-container') as HTMLDivElement;
232232

233233
expect(component.gridHeightString).toBe('600px');
234234
expect(component.gridWidthString).toBe('800px');
@@ -237,7 +237,7 @@ describe('App Component', () => {
237237
});
238238

239239
it('should throw an error when the "enableAutoResize" is disabled and no "grid-height" is provided', () => {
240-
component.gridHeight = null;
240+
(component as any).gridHeight = null;
241241
component.gridOptions = { enableAutoResize: false };
242242

243243
expect(() => fixture.detectChanges()).toThrowError('[Angular-Slickgrid] requires a "grid-height" or the "enableAutoResize"');
@@ -268,4 +268,16 @@ describe('App Component', () => {
268268
done();
269269
}, 10);
270270
});
271+
272+
it('should update "visibleColumns" in the Shared Service when "onColumnsReordered" event is triggered', () => {
273+
const sharedHasColumnsReorderedSpy = jest.spyOn(SharedService.prototype, 'hasColumnsReordered', 'set');
274+
const sharedVisibleColumnsSpy = jest.spyOn(SharedService.prototype, 'visibleColumns', 'set');
275+
const newVisibleColumns = [{ id: 'lastName', field: 'lastName' }, { id: 'fristName', field: 'fristName' }];
276+
277+
fixture.detectChanges();
278+
component.grid.onColumnsReordered.notify({ impactedColumns: newVisibleColumns });
279+
280+
expect(sharedHasColumnsReorderedSpy).toHaveBeenCalledWith(true);
281+
expect(sharedVisibleColumnsSpy).toHaveBeenCalledWith(newVisibleColumns);
282+
});
271283
});

‎src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,12 @@ export class AngularSlickgridComponent implements AfterViewInit, OnDestroy, OnIn
659659
}
660660
}
661661
});
662+
663+
// when column are reordered, we need to update the visibleColumn array
664+
this._eventHandler.subscribe(grid.onColumnsReordered, (e: any, args: { impactedColumns: Column[]; grid: any; }) => {
665+
this.sharedService.hasColumnsReordered = true;
666+
this.sharedService.visibleColumns = args.impactedColumns;
667+
});
662668
}
663669

664670
// does the user have a colspan callback?

‎src/app/modules/angular-slickgrid/extensions/headerMenuExtension.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,8 +348,8 @@ export class HeaderMenuExtension implements Extension {
348348
this.sharedService.frozenVisibleColumnId = args.column.id;
349349

350350
// to freeze columns, we need to take only the visible columns and we also need to use setColumns() when some of them are hidden
351-
// to make sure that we only use the visible columns, not doing this would show back some of the hidden columns
352-
if (Array.isArray(visibleColumns) && Array.isArray(this.sharedService.allColumns) && visibleColumns.length !== this.sharedService.allColumns.length) {
351+
// to make sure that we only use the visible columns, not doing this will have the undesired effect of showing back some of the hidden columns
352+
if (this.sharedService.hasColumnsReordered || (Array.isArray(this.sharedService.allColumns) && visibleColumns.length !== this.sharedService.allColumns.length)) {
353353
this.sharedService.grid.setColumns(visibleColumns);
354354
}
355355
break;

0 commit comments

Comments
 (0)