From 74c47a1fb7542ae8ec400853e3eb53b4f42d7e2b Mon Sep 17 00:00:00 2001 From: Ghislain B Date: Mon, 15 Jan 2024 09:04:35 -0500 Subject: [PATCH] chore: remove unused code related to Column Reorder & SortableJS (#1330) * chore: remove unused code related to Column Reorder & SortableJS - when we added SortableJS, we previously had code that is longer required since SortableJS already takes care of the order validation and such, there is also code related to Tree Column Depth that should have been removed. * chore: fix function name typo --- .../src/core/__tests__/slickGrid.spec.ts | 131 ++++++++++++++++++ packages/common/src/core/slickGrid.ts | 48 +------ 2 files changed, 137 insertions(+), 42 deletions(-) diff --git a/packages/common/src/core/__tests__/slickGrid.spec.ts b/packages/common/src/core/__tests__/slickGrid.spec.ts index 7db09000f..3267f1e79 100644 --- a/packages/common/src/core/__tests__/slickGrid.spec.ts +++ b/packages/common/src/core/__tests__/slickGrid.spec.ts @@ -1949,6 +1949,137 @@ describe('SlickGrid core file', () => { }); }); + describe('Column Reorder', () => { + const columns = [ + { id: 'firstName', field: 'firstName', name: 'First Name', sortable: true }, + { id: 'lastName', field: 'lastName', name: 'Last Name', sortable: true }, + { id: 'age', field: 'age', name: 'Age', sortable: true }, + ] as Column[]; + const data = [{ id: 0, firstName: 'John', lastName: 'Doe', age: 30 }, { id: 1, firstName: 'Jane', lastName: 'Doe', age: 28 }]; + let sortInstance; + + it('should reorder column to the left when current column pageX is lower than viewport left position', () => { + grid = new SlickGrid(container, data, columns, defaultOptions); + const headerColumnsElm = document.querySelector('.slick-header-columns.slick-header-columns-left') as HTMLDivElement; + Object.keys(headerColumnsElm).forEach(prop => { + if (prop.startsWith('Sortable')) { + sortInstance = headerColumnsElm[prop]; + } + }); + const onColumnsReorderedSpy = jest.spyOn(grid.onColumnsReordered, 'notify'); + const headerColumnElms = document.querySelectorAll('.slick-header-column'); + const viewportTopLeft = document.querySelector('.slick-viewport-top.slick-viewport-left') as HTMLDivElement; + + const dragEvent = new CustomEvent('DragEvent'); + jest.spyOn(viewportTopLeft, 'getBoundingClientRect').mockReturnValue({ left: 25, top: 10, right: 0, bottom: 0 } as DOMRect); + Object.defineProperty(dragEvent, 'originalEvent', { writable: true, value: { pageX: 20 } }); + Object.defineProperty(dragEvent, 'item', { writable: true, value: headerColumnElms[0] }); + Object.defineProperty(headerColumnElms[0], 'clientLeft', { writable: true, value: 25 }); + Object.defineProperty(viewportTopLeft, 'clientLeft', { writable: true, value: 25 }); + + expect(sortInstance).toBeTruthy(); + sortInstance.options.onStart(dragEvent); + expect(viewportTopLeft.scrollLeft).toBe(0); + + jest.advanceTimersByTime(100); + + expect(viewportTopLeft.scrollLeft).toBe(-10); + sortInstance.options.onEnd(dragEvent); + expect(onColumnsReorderedSpy).toHaveBeenCalled(); + }); + + it('should reorder column to the right when current column pageX is greater than container width', () => { + grid = new SlickGrid(container, data, columns, defaultOptions); + const headerColumnsElm = document.querySelector('.slick-header-columns.slick-header-columns-left') as HTMLDivElement; + Object.keys(headerColumnsElm).forEach(prop => { + if (prop.startsWith('Sortable')) { + sortInstance = headerColumnsElm[prop]; + } + }); + const onColumnsReorderedSpy = jest.spyOn(grid.onColumnsReordered, 'notify'); + const headerColumnElms = document.querySelectorAll('.slick-header-column'); + const viewportTopLeft = document.querySelector('.slick-viewport-top.slick-viewport-left') as HTMLDivElement; + + const dragEvent = new CustomEvent('DragEvent'); + jest.spyOn(viewportTopLeft, 'getBoundingClientRect').mockReturnValue({ left: 25, top: 10, right: 0, bottom: 0 } as DOMRect); + Object.defineProperty(dragEvent, 'originalEvent', { writable: true, value: { pageX: DEFAULT_GRID_WIDTH + 11 } }); + Object.defineProperty(dragEvent, 'item', { writable: true, value: headerColumnElms[0] }); + Object.defineProperty(headerColumnElms[0], 'clientLeft', { writable: true, value: 25 }); + Object.defineProperty(viewportTopLeft, 'clientLeft', { writable: true, value: 25 }); + + expect(sortInstance).toBeTruthy(); + sortInstance.options.onStart(dragEvent); + expect(viewportTopLeft.scrollLeft).toBe(0); + + jest.advanceTimersByTime(100); + + expect(viewportTopLeft.scrollLeft).toBe(10); + sortInstance.options.onEnd(dragEvent); + expect(onColumnsReorderedSpy).toHaveBeenCalled(); + }); + + it('should try reordering column but stay at same scroll position when grid has frozen columns', () => { + grid = new SlickGrid(container, data, columns, { ...defaultOptions, frozenColumn: 0 }); + grid.setActiveCell(0, 1); + const headerColumnsElm = document.querySelector('.slick-header-columns.slick-header-columns-left') as HTMLDivElement; + Object.keys(headerColumnsElm).forEach(prop => { + if (prop.startsWith('Sortable')) { + sortInstance = headerColumnsElm[prop]; + } + }); + const onColumnsReorderedSpy = jest.spyOn(grid.onColumnsReordered, 'notify'); + const headerColumnElms = document.querySelectorAll('.slick-header-column'); + const viewportTopLeft = document.querySelector('.slick-viewport-top.slick-viewport-left') as HTMLDivElement; + + const dragEvent = new CustomEvent('DragEvent'); + jest.spyOn(viewportTopLeft, 'getBoundingClientRect').mockReturnValue({ left: 25, top: 10, right: 0, bottom: 0 } as DOMRect); + Object.defineProperty(dragEvent, 'originalEvent', { writable: true, value: { pageX: 20 } }); + Object.defineProperty(dragEvent, 'item', { writable: true, value: headerColumnElms[0] }); + Object.defineProperty(headerColumnElms[0], 'clientLeft', { writable: true, value: 25 }); + Object.defineProperty(viewportTopLeft, 'clientLeft', { writable: true, value: 25 }); + + expect(sortInstance).toBeTruthy(); + sortInstance.options.onStart(dragEvent); + expect(viewportTopLeft.scrollLeft).toBe(0); + + jest.advanceTimersByTime(100); + + expect(viewportTopLeft.scrollLeft).toBe(0); // same position + sortInstance.options.onEnd(dragEvent); + expect(onColumnsReorderedSpy).toHaveBeenCalled(); + }); + + it('should not allow column reordering when Editor Lock commitCurrentEdit() is failing', () => { + grid = new SlickGrid(container, data, columns, { ...defaultOptions, frozenColumn: 0 }); + jest.spyOn(grid.getEditorLock(), 'commitCurrentEdit').mockReturnValueOnce(false); + const headerColumnsElm = document.querySelector('.slick-header-columns.slick-header-columns-left') as HTMLDivElement; + Object.keys(headerColumnsElm).forEach(prop => { + if (prop.startsWith('Sortable')) { + sortInstance = headerColumnsElm[prop]; + } + }); + const onColumnsReorderedSpy = jest.spyOn(grid.onColumnsReordered, 'notify'); + const headerColumnElms = document.querySelectorAll('.slick-header-column'); + const viewportTopLeft = document.querySelector('.slick-viewport-top.slick-viewport-left') as HTMLDivElement; + + const dragEvent = new CustomEvent('DragEvent'); + jest.spyOn(viewportTopLeft, 'getBoundingClientRect').mockReturnValue({ left: 25, top: 10, right: 0, bottom: 0 } as DOMRect); + Object.defineProperty(dragEvent, 'originalEvent', { writable: true, value: { pageX: 20 } }); + Object.defineProperty(dragEvent, 'item', { writable: true, value: headerColumnElms[0] }); + Object.defineProperty(viewportTopLeft, 'clientLeft', { writable: true, value: 25 }); + + expect(sortInstance).toBeTruthy(); + sortInstance.options.onStart(dragEvent); + expect(viewportTopLeft.scrollLeft).toBe(0); + + jest.advanceTimersByTime(100); + + expect(viewportTopLeft.scrollLeft).toBe(0); + sortInstance.options.onEnd(dragEvent); + expect(onColumnsReorderedSpy).not.toHaveBeenCalled(); + }); + }); + describe('Scrolling', () => { const columns = [ { id: 'firstName', field: 'firstName', name: 'First Name', sortable: true }, diff --git a/packages/common/src/core/slickGrid.ts b/packages/common/src/core/slickGrid.ts index 91b0d9d66..10732b41c 100644 --- a/packages/common/src/core/slickGrid.ts +++ b/packages/common/src/core/slickGrid.ts @@ -1767,27 +1767,6 @@ export class SlickGrid = Column, O e }); } - protected currentPositionInHeader(id: number | string) { - let currentPosition = 0; - this._headers.forEach((header) => { - header.querySelectorAll('.slick-header-column').forEach((column, i) => { - if (column.id === id) { - currentPosition = i; - } - }); - }); - - return currentPosition; - } - - protected remove(arr: any[], elem: HTMLElement) { - const index = arr.lastIndexOf(elem); - if (index > -1) { - arr.splice(index, 1); - this.remove(arr, elem); - } - } - protected setupColumnReorder() { this.sortableSideLeftInstance?.destroy(); this.sortableSideRightInstance?.destroy(); @@ -1798,7 +1777,7 @@ export class SlickGrid = Column, O e const scrollColumnsRight = () => this._viewportScrollContainerX.scrollLeft += 10; const scrollColumnsLeft = () => this._viewportScrollContainerX.scrollLeft -= 10; - let canDragScroll; + let canDragScroll = false; const sortableOptions = { animation: 50, direction: 'horizontal', @@ -1826,12 +1805,10 @@ export class SlickGrid = Column, O e } }, onEnd: (e: SortableEvent) => { - const cancel = false; clearInterval(columnScrollTimer); columnScrollTimer = null; - let limit; - if (cancel || !this.getEditorLock()?.commitCurrentEdit()) { + if (!this.getEditorLock()?.commitCurrentEdit()) { return; } @@ -1844,7 +1821,7 @@ export class SlickGrid = Column, O e } this.setColumns(reorderedColumns); - this.triggerEvent(this.onColumnsReordered, { impactedColumns: this.getImpactedColumns(limit) }); + this.triggerEvent(this.onColumnsReordered, { impactedColumns: this.columns }); e.stopPropagation(); this.setupColumnResize(); if (this.activeCellNode) { @@ -1863,26 +1840,13 @@ export class SlickGrid = Column, O e return a.concat(b) as HTMLElement[]; } - protected getImpactedColumns(limit?: { start: number; end: number; }) { - let impactedColumns: C[] = []; - - if (limit) { - for (let i = limit.start; i <= limit.end; i++) { - impactedColumns.push(this.columns[i]); - } - } else { - impactedColumns = this.columns; - } - - return impactedColumns; - } - - protected handleResizeableHandleDoubleClick(evt: MouseEvent & { target: HTMLDivElement; }) { + protected handleResizeableDoubleClick(evt: MouseEvent & { target: HTMLDivElement; }) { const triggeredByColumn = evt.target.parentElement!.id.replace(this.uid, ''); this.triggerEvent(this.onColumnsResizeDblClick, { triggeredByColumn }); } protected setupColumnResize() { + /* istanbul ignore if */ if (typeof Resizable === 'undefined') { throw new Error(`SlickResizable is undefined, make sure to import "slick.interactions.js"`); } @@ -1930,7 +1894,7 @@ export class SlickGrid = Column, O e } const resizeableHandle = createDomElement('div', { className: 'slick-resizable-handle', role: 'separator', ariaOrientation: 'horizontal' }, colElm); - this._bindingEventService.bind(resizeableHandle, 'dblclick', this.handleResizeableHandleDoubleClick.bind(this) as EventListener); + this._bindingEventService.bind(resizeableHandle, 'dblclick', this.handleResizeableDoubleClick.bind(this) as EventListener); this.slickResizableInstances.push( Resizable({