From 84b3db81cc8b3d87e768d4c86096c43e89b68511 Mon Sep 17 00:00:00 2001 From: ghiscoding Date: Sun, 24 Dec 2023 16:17:07 -0500 Subject: [PATCH 1/4] fix(RowDetail): sort change should collapse all Row Detail - calling sort should collapse all Row Detail, however this was only true when triggered by clicking on column header menu, however it wasn't being triggered when called from Header Menu and Grid Menu (clear all sorting) --- packages/common/src/styles/_variables.scss | 1 + packages/common/src/styles/slick-plugins.scss | 7 ++++++- .../src/slickRowDetailView.spec.ts | 19 +++++++------------ .../src/slickRowDetailView.ts | 13 +++++++++---- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/packages/common/src/styles/_variables.scss b/packages/common/src/styles/_variables.scss index 763e06188..38b4c2b9b 100644 --- a/packages/common/src/styles/_variables.scss +++ b/packages/common/src/styles/_variables.scss @@ -254,6 +254,7 @@ $slick-detail-view-icon-expand: "\f055" !default; $slick-detail-view-icon-expand-color: lighten($slick-primary-color, 25%) !default; $slick-detail-view-icon-expand-color-hover: darken($slick-detail-view-icon-expand-color, 10%) !default; $slick-detail-view-icon-size: calc(#{$slick-icon-font-size} + 2px) !default; +$slick-detail-view-icon-width: 18px !default; $slick-detail-view-container-bgcolor: #f7f7f7 !default; $slick-detail-view-container-border: 1px solid #c0c0c0 !default; $slick-detail-view-container-left: 0 !default; diff --git a/packages/common/src/styles/slick-plugins.scss b/packages/common/src/styles/slick-plugins.scss index 616be10eb..9305d2b90 100644 --- a/packages/common/src/styles/slick-plugins.scss +++ b/packages/common/src/styles/slick-plugins.scss @@ -1279,7 +1279,6 @@ input.flatpickr.form-control { .slick-row { .detail-view-toggle { display: inline-block; - cursor: pointer; &.expand { display: inline-block; @@ -1302,10 +1301,16 @@ input.flatpickr.form-control { content: var(--slick-detail-view-icon-collapse, $slick-detail-view-icon-collapse); } } + &.expand, + &.collapse { + cursor: pointer; + } &.expand:before, &.collapse:before { + display: inline-block; font-family: var(--slick-icon-font-family, $slick-icon-font-family); font-size: var(--slick-detail-view-icon-size, $slick-detail-view-icon-size); + width: var(--slick-detail-view-icon-width, $slick-detail-view-icon-width); } } diff --git a/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts b/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts index 11af9230c..3ab43b13b 100644 --- a/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts +++ b/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts @@ -1,5 +1,6 @@ import 'jest-extended'; -import { Column, GridOption, PubSubService, type SlickDataView, SlickEvent, SlickEventData, SlickGrid, FormatterResultWithHtml } from '@slickgrid-universal/common'; +import { Column, type FormatterResultWithHtml, GridOption, type SlickDataView, SlickEvent, SlickEventData, SlickGrid } from '@slickgrid-universal/common'; +import { EventPubSubService } from '@slickgrid-universal/event-pub-sub'; import { SlickRowDetailView } from './slickRowDetailView'; @@ -48,16 +49,10 @@ const gridStub = { onSort: new SlickEvent(), } as unknown as SlickGrid; -const pubSubServiceStub = { - publish: jest.fn(), - subscribe: jest.fn(), - unsubscribe: jest.fn(), - unsubscribeAll: jest.fn(), -} as PubSubService; - let mockColumns: Column[]; describe('SlickRowDetailView plugin', () => { + let eventPubSubService: EventPubSubService; const divContainer = document.createElement('div'); let plugin: SlickRowDetailView; const gridContainerElm = document.createElement('div'); @@ -68,7 +63,8 @@ describe('SlickRowDetailView plugin', () => { { id: 'firstName', name: 'First Name', field: 'firstName', width: 100 }, { id: 'lasstName', name: 'Last Name', field: 'lasstName', width: 100 }, ]; - plugin = new SlickRowDetailView(pubSubServiceStub); + eventPubSubService = new EventPubSubService(); + plugin = new SlickRowDetailView(eventPubSubService); divContainer.className = `slickgrid-container ${GRID_UID}`; document.body.appendChild(divContainer); }); @@ -134,8 +130,7 @@ describe('SlickRowDetailView plugin', () => { jest.spyOn(gridStub, 'getOptions').mockReturnValue({ ...gridOptionsMock, rowDetailView: { collapseAllOnSort: true } as any }); plugin.init(gridStub); - const eventData = { ...new SlickEventData(), preventDefault: jest.fn() }; - gridStub.onSort.notify({ sortCols: [{ columnId: mockColumns[0].id, sortCol: mockColumns[0], sortAsc: true }], multiColumnSort: true, previousSortColumns: [], grid: gridStub }, eventData as any, gridStub); + eventPubSubService.publish('onSortChanged', {}); expect(plugin.getExpandedRows()).toEqual([]); expect(plugin.getOutOfViewportRows()).toEqual([]); @@ -201,7 +196,7 @@ describe('SlickRowDetailView plugin', () => { }); it('should add the Row Detail to the column definitions at index when calling "create" without specifying position', () => { - const pubSubSpy = jest.spyOn(pubSubServiceStub, 'publish'); + const pubSubSpy = jest.spyOn(eventPubSubService, 'publish'); const processMock = jest.fn(); const overrideMock = jest.fn(); const rowDetailColumnMock = { diff --git a/packages/row-detail-view-plugin/src/slickRowDetailView.ts b/packages/row-detail-view-plugin/src/slickRowDetailView.ts index 96693ca47..7c69bdff0 100644 --- a/packages/row-detail-view-plugin/src/slickRowDetailView.ts +++ b/packages/row-detail-view-plugin/src/slickRowDetailView.ts @@ -155,7 +155,8 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV // Sort will, by default, Collapse all of the open items (unless user implements his own onSort which deals with open row and padding) if (this._addonOptions.collapseAllOnSort) { - this._eventHandler.subscribe(this._grid.onSort, this.collapseAll.bind(this)); + // sort event can be triggered by column header click or from header menu + this.pubSubService.subscribe('onSortChanged', () => this.collapseAll()); this._expandedRows = []; this._rowIdsOutOfViewport = []; } @@ -605,7 +606,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV } /** The Formatter of the toggling icon of the Row Detail */ - protected detailSelectionFormatter(row: number, cell: number, value: any, columnDef: Column, dataContext: any, grid: SlickGrid): FormatterResultWithHtml | HTMLElement | '' { + protected detailSelectionFormatter(row: number, _cell: number, _val: any, _colDef: Column, dataContext: any, grid: SlickGrid): FormatterResultWithHtml | HTMLElement | '' { if (!this.checkExpandableOverride(row, dataContext, grid)) { return ''; } else { @@ -648,13 +649,17 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV }); const innerContainerElm = createDomElement('div', { className: `detail-container detailViewContainer_${dataContext[this._dataViewIdProperty]}` }); const innerDetailViewElm = createDomElement('div', { className: `innerDetailView_${dataContext[this._dataViewIdProperty]}` }); - innerDetailViewElm.innerHTML = this._grid.sanitizeHtmlString(dataContext[`${this._keyPrefix}detailContent`]); + if (dataContext[`${this._keyPrefix}detailContent`] instanceof HTMLElement) { + innerDetailViewElm.appendChild(dataContext[`${this._keyPrefix}detailContent`]); + } else { + innerDetailViewElm.innerHTML = this._grid.sanitizeHtmlString(dataContext[`${this._keyPrefix}detailContent`]); + } innerContainerElm.appendChild(innerDetailViewElm); cellDetailContainerElm.appendChild(innerContainerElm); const result: FormatterResultWithHtml = { - html: createDomElement('div', { className: expandedClasses }), + html: createDomElement('div', { className: expandedClasses.trim() }), insertElementAfterTarget: cellDetailContainerElm, }; From dde2360cfb0cdd7337d1fcf65ecf83298950cc1c Mon Sep 17 00:00:00 2001 From: ghiscoding Date: Sun, 24 Dec 2023 16:22:38 -0500 Subject: [PATCH 2/4] chore: add missing dev dep for SlickRowDetail unit tests --- packages/row-detail-view-plugin/package.json | 1 + pnpm-lock.yaml | 3 +++ 2 files changed, 4 insertions(+) diff --git a/packages/row-detail-view-plugin/package.json b/packages/row-detail-view-plugin/package.json index d7ff1027e..037809d5a 100644 --- a/packages/row-detail-view-plugin/package.json +++ b/packages/row-detail-view-plugin/package.json @@ -60,6 +60,7 @@ "@slickgrid-universal/utils": "workspace:~" }, "devDependencies": { + "@slickgrid-universal/event-pub-sub": "workspace:~", "cross-env": "^7.0.3", "npm-run-all2": "^6.1.1" } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 75cddbeda..3427cbfcc 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -455,6 +455,9 @@ importers: specifier: workspace:~ version: link:../utils devDependencies: + '@slickgrid-universal/event-pub-sub': + specifier: workspace:~ + version: link:../event-pub-sub cross-env: specifier: ^7.0.3 version: 7.0.3 From a5e4572103558ba135239af4fbede924ba6db835 Mon Sep 17 00:00:00 2001 From: ghiscoding Date: Sun, 24 Dec 2023 16:33:27 -0500 Subject: [PATCH 3/4] chore: add SlickRowDetail missing unit test --- .../src/slickRowDetailView.spec.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts b/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts index 3ab43b13b..1ca0cae01 100644 --- a/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts +++ b/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts @@ -1,5 +1,5 @@ import 'jest-extended'; -import { Column, type FormatterResultWithHtml, GridOption, type SlickDataView, SlickEvent, SlickEventData, SlickGrid } from '@slickgrid-universal/common'; +import { Column, type FormatterResultWithHtml, GridOption, type SlickDataView, SlickEvent, SlickEventData, SlickGrid, createDomElement } from '@slickgrid-universal/common'; import { EventPubSubService } from '@slickgrid-universal/event-pub-sub'; import { SlickRowDetailView } from './slickRowDetailView'; @@ -250,7 +250,7 @@ describe('SlickRowDetailView plugin', () => { expect(updateItemSpy).not.toHaveBeenCalled(); }); - it('should trigger "onAsyncResponse" with Row Detail from post template when no detailView is provided and expect "updateItem" from DataView to be called with new template & data', () => { + it('should trigger "onAsyncResponse" with Row Detail from post template from HTML string when no detailView is provided and expect "updateItem" from DataView to be called with new template & data', () => { const updateItemSpy = jest.spyOn(dataviewStub, 'updateItem'); const asyncEndUpdateSpy = jest.spyOn(plugin.onAsyncEndUpdate, 'notify'); const itemMock = { id: 123, firstName: 'John', lastName: 'Doe' }; @@ -264,6 +264,20 @@ describe('SlickRowDetailView plugin', () => { expect(asyncEndUpdateSpy).toHaveBeenCalledWith({ grid: gridStub, item: itemMock, itemDetail: { _detailContent: 'Post 123', _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } }); }); + it('should trigger "onAsyncResponse" with Row Detail from post template with HTML Element when no detailView is provided and expect "updateItem" from DataView to be called with new template & data', () => { + const updateItemSpy = jest.spyOn(dataviewStub, 'updateItem'); + const asyncEndUpdateSpy = jest.spyOn(plugin.onAsyncEndUpdate, 'notify'); + const itemMock = { id: 123, firstName: 'John', lastName: 'Doe' }; + const postViewMock = (item) => createDomElement('span', { textContent: `Post ${item.id}` }); + jest.spyOn(gridStub, 'getOptions').mockReturnValue({ ...gridOptionsMock, rowDetailView: { postTemplate: postViewMock } as any }); + + plugin.init(gridStub); + plugin.onAsyncResponse.notify({ item: itemMock, itemDetail: itemMock, }, new SlickEventData()); + + expect(updateItemSpy).toHaveBeenCalledWith(123, { _detailContent: createDomElement('span', { textContent: 'Post 123' }), _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' }); + expect(asyncEndUpdateSpy).toHaveBeenCalledWith({ grid: gridStub, item: itemMock, itemDetail: { _detailContent: createDomElement('span', { textContent: 'Post 123' }), _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } }); + }); + it('should trigger "onAsyncResponse" with Row Detail template when detailView is provided and expect "updateItem" from DataView to be called with new template & data', () => { const updateItemSpy = jest.spyOn(dataviewStub, 'updateItem'); const asyncEndUpdateSpy = jest.spyOn(plugin.onAsyncEndUpdate, 'notify'); From 07596a2de8ba9727a93ae3d851a1063df50daa46 Mon Sep 17 00:00:00 2001 From: ghiscoding Date: Sun, 24 Dec 2023 17:05:35 -0500 Subject: [PATCH 4/4] chore: add SlickRowDetail missing unit test --- .../src/slickRowDetailView.spec.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts b/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts index 1ca0cae01..46261493f 100644 --- a/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts +++ b/packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts @@ -738,14 +738,24 @@ describe('SlickRowDetailView plugin', () => { expect(formattedVal).toBe(``); }); - it('should execute formatter and expect it to return empty string and render nothing when isPadding is True', () => { - const mockItem = { id: 123, firstName: 'John', lastName: 'Doe', _collapsed: false, _isPadding: false, _sizePadding: 5 }; + it('should execute formatter and expect it to render detail content from HTML string', () => { + const mockItem = { id: 123, firstName: 'John', lastName: 'Doe', _collapsed: false, _isPadding: false, _sizePadding: 5, _detailContent: `
Loading...
` }; + plugin.init(gridStub); + plugin.setOptions({ expandedClass: 'some-expanded', maxRows: 2 }); + plugin.expandableOverride(() => true); + const formattedVal = plugin.getColumnDefinition().formatter!(0, 1, '', mockColumns[0], mockItem, gridStub); + expect(((formattedVal as FormatterResultWithHtml).html as HTMLElement).outerHTML).toBe(`
`); + expect((formattedVal as FormatterResultWithHtml).insertElementAfterTarget!.outerHTML).toBe(`
Loading...
`); + }); + + it('should execute formatter and expect it to render detail content from HTML Element', () => { + const mockItem = { id: 123, firstName: 'John', lastName: 'Doe', _collapsed: false, _isPadding: false, _sizePadding: 5, _detailContent: createDomElement('div', { textContent: 'Loading...' }) }; plugin.init(gridStub); plugin.setOptions({ expandedClass: 'some-expanded', maxRows: 2 }); plugin.expandableOverride(() => true); const formattedVal = plugin.getColumnDefinition().formatter!(0, 1, '', mockColumns[0], mockItem, gridStub); expect(((formattedVal as FormatterResultWithHtml).html as HTMLElement).outerHTML).toBe(`
`); - expect((formattedVal as FormatterResultWithHtml).insertElementAfterTarget!.outerHTML).toBe(`
undefined
`); + expect((formattedVal as FormatterResultWithHtml).insertElementAfterTarget!.outerHTML).toBe(`
Loading...
`); }); }); });