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

Commit 2b87dca

Browse files
ghiscodingghiscoding
authored
Merge pull request #1536 from ghiscoding/chore/unneeded-rowdetail-event-wrap
fix: Row Detail code cleanup, remove unnecessary if condition wrappers
2 parents e221a16 + ad8c57d commit 2b87dca

File tree

4 files changed

+83
-93
lines changed

4 files changed

+83
-93
lines changed

‎src/app/examples/home.component.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ <h4>Documentation</h4>
3131
<img
3232
height="36"
3333
style="border: 0px; height: 36px"
34-
src="https://az743702.vo.msecnd.net/cdn/kofi2.png?v=0"
34+
src="https://storage.ko-fi.com/cdn/kofi3.png?v=6"
3535
border="0"
3636
alt="Buy Me a Coffee at ko-fi.com"
3737
/>

‎src/app/modules/angular-slickgrid/extensions/__tests__/slickRowDetailView.spec.ts‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import { AngularUtilService } from '../../services';
1616
import { RowDetailView } from '../../models/rowDetailView.interface';
1717
import { RxJsResourceStub } from '../../../../../../test/rxjsResourceStub';
1818
import { SlickRowDetailView } from '../slickRowDetailView';
19-
jest.mock('@slickgrid-universal/row-detail-view-plugin');
2019

2120
jest.mock('@slickgrid-universal/common', () => ({
2221
...(jest.requireActual('@slickgrid-universal/common') as any),
@@ -33,6 +32,16 @@ jest.mock('@slickgrid-universal/common', () => ({
3332
})),
3433
}));
3534

35+
jest.mock('@slickgrid-universal/row-detail-view-plugin', () => ({
36+
...(jest.requireActual('@slickgrid-universal/row-detail-view-plugin') as any),
37+
onAsyncResponse: new SlickEvent(),
38+
onAsyncEndUpdate: new SlickEvent(),
39+
onAfterRowDetailToggle: new SlickEvent(),
40+
onBeforeRowDetailToggle: new SlickEvent(),
41+
onRowOutOfViewportRange: new SlickEvent(),
42+
onRowBackToViewportRange: new SlickEvent(),
43+
}));
44+
3645
const ROW_DETAIL_CONTAINER_PREFIX = 'container_';
3746
const PRELOAD_CONTAINER_PREFIX = 'container_loading';
3847

@@ -74,6 +83,9 @@ const dataViewStub = {
7483
getItem: jest.fn(),
7584
getItems: jest.fn(),
7685
getItemCount: jest.fn(),
86+
onRowCountChanged: new SlickEvent(),
87+
onRowsChanged: new SlickEvent(),
88+
onSetItemsCalled: new SlickEvent(),
7789
};
7890

7991
const gridStub = {
@@ -85,6 +97,10 @@ const gridStub = {
8597
sanitizeHtmlString: (s: string) => s,
8698
onColumnsReordered: new SlickEvent(),
8799
onSelectedRowsChanged: new SlickEvent(),
100+
onBeforeEditCell: new SlickEvent(),
101+
onBeforeRemoveCachedRow: new SlickEvent(),
102+
onClick: new SlickEvent(),
103+
onScroll: new SlickEvent(),
88104
onSort: new SlickEvent(),
89105
} as unknown as SlickGrid;
90106

@@ -109,7 +125,7 @@ describe('SlickRowDetailView', () => {
109125
rxjsResourceStub = new RxJsResourceStub();
110126

111127
plugin = new SlickRowDetailView(angularUtilServiceStub, applicationRefStub, eventPubSubService, document.body as HTMLDivElement, rxjsResourceStub);
112-
plugin.eventHandler = new SlickEventHandler();
128+
jest.spyOn(plugin, 'eventHandler', 'get').mockReturnValue(eventHandler);
113129
jest.spyOn(plugin, 'getOptions').mockReturnValue(gridOptionsMock.rowDetailView as RowDetailView);
114130
});
115131

@@ -423,7 +439,7 @@ describe('SlickRowDetailView', () => {
423439
model: mockColumn,
424440
addon: expect.anything(),
425441
grid: gridStub,
426-
dataView: undefined,
442+
dataView: dataViewStub,
427443
parent: undefined,
428444
},
429445
{ sanitizer: expect.any(Function) }
@@ -460,7 +476,7 @@ describe('SlickRowDetailView', () => {
460476
model: mockColumn,
461477
addon: expect.anything(),
462478
grid: gridStub,
463-
dataView: undefined,
479+
dataView: dataViewStub,
464480
parent: undefined,
465481
},
466482
{ sanitizer: expect.any(Function) }
@@ -495,7 +511,7 @@ describe('SlickRowDetailView', () => {
495511
model: mockColumn,
496512
addon: expect.anything(),
497513
grid: gridStub,
498-
dataView: undefined,
514+
dataView: dataViewStub,
499515
parent: undefined,
500516
},
501517
{ sanitizer: expect.any(Function) }

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

Lines changed: 60 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import type {
44
OnBeforeRowDetailToggleArgs,
55
OnRowBackToViewportRangeArgs,
66
RxJsFacade,
7-
SlickEventHandler,
87
SlickGrid,
98
} from '@slickgrid-universal/common';
109
import {
@@ -58,13 +57,6 @@ export class SlickRowDetailView extends UniversalSlickRowDetailView {
5857
return this.gridOptions.datasetIdPropertyName || 'id';
5958
}
6059

61-
get eventHandler(): SlickEventHandler {
62-
return this._eventHandler;
63-
}
64-
set eventHandler(eventHandler: SlickEventHandler) {
65-
this._eventHandler = eventHandler;
66-
}
67-
6860
/** Getter for the Grid Options pulled through the Grid Object */
6961
get gridOptions(): GridOption {
7062
return (this._grid?.getOptions() || {}) as GridOption;
@@ -150,82 +142,68 @@ export class SlickRowDetailView extends UniversalSlickRowDetailView {
150142
this.rowDetailViewOptions.onExtensionRegistered(this);
151143
}
152144

153-
if (this.onAsyncResponse) {
154-
this.eventHandler.subscribe(this.onAsyncResponse, (event, args) => {
155-
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onAsyncResponse === 'function') {
156-
this.rowDetailViewOptions.onAsyncResponse(event, args);
157-
}
158-
});
159-
}
160-
161-
if (this.onAsyncEndUpdate) {
162-
this.eventHandler.subscribe(this.onAsyncEndUpdate, (e, args) => {
163-
// destroy preload if exists
164-
this._preloadCompRef?.destroy();
165-
166-
// triggers after backend called "onAsyncResponse.notify()"
167-
this.renderViewModel(args?.item);
168-
169-
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onAsyncEndUpdate === 'function') {
170-
this.rowDetailViewOptions.onAsyncEndUpdate(e, args);
171-
}
172-
});
173-
}
174-
175-
if (this.onAfterRowDetailToggle) {
176-
this.eventHandler.subscribe(
177-
this.onAfterRowDetailToggle,
178-
(e: any, args: { grid: SlickGrid; item: any; expandedRows: Array<number | string> }) => {
179-
// display preload template & re-render all the other Detail Views after toggling
180-
// the preload View will eventually go away once the data gets loaded after the "onAsyncEndUpdate" event
181-
this.renderPreloadView();
182-
183-
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onAfterRowDetailToggle === 'function') {
184-
this.rowDetailViewOptions.onAfterRowDetailToggle(e, args);
185-
}
186-
}
187-
);
188-
}
189-
190-
if (this.onBeforeRowDetailToggle) {
191-
this.eventHandler.subscribe(this.onBeforeRowDetailToggle, (e, args) => {
192-
// before toggling row detail, we need to create View Component if it doesn't exist
193-
this.handleOnBeforeRowDetailToggle(e, args);
194-
195-
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onBeforeRowDetailToggle === 'function') {
196-
return this.rowDetailViewOptions.onBeforeRowDetailToggle(e, args);
145+
this.eventHandler.subscribe(this.onAsyncResponse, (event, args) => {
146+
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onAsyncResponse === 'function') {
147+
this.rowDetailViewOptions.onAsyncResponse(event, args);
148+
}
149+
});
150+
151+
this.eventHandler.subscribe(this.onAsyncEndUpdate, (e, args) => {
152+
// destroy preload if exists
153+
this._preloadCompRef?.destroy();
154+
155+
// triggers after backend called "onAsyncResponse.notify()"
156+
this.renderViewModel(args?.item);
157+
158+
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onAsyncEndUpdate === 'function') {
159+
this.rowDetailViewOptions.onAsyncEndUpdate(e, args);
160+
}
161+
});
162+
163+
this.eventHandler.subscribe(
164+
this.onAfterRowDetailToggle,
165+
(e: any, args: { grid: SlickGrid; item: any; expandedRows: Array<number | string> }) => {
166+
// display preload template & re-render all the other Detail Views after toggling
167+
// the preload View will eventually go away once the data gets loaded after the "onAsyncEndUpdate" event
168+
this.renderPreloadView();
169+
170+
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onAfterRowDetailToggle === 'function') {
171+
this.rowDetailViewOptions.onAfterRowDetailToggle(e, args);
197172
}
198-
return true;
199-
});
200-
}
201-
202-
if (this.onRowBackToViewportRange) {
203-
this.eventHandler.subscribe(this.onRowBackToViewportRange, (e, args) => {
204-
// when row is back to viewport range, we will re-render the View Component(s)
205-
this.handleOnRowBackToViewportRange(e, args);
206-
207-
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onRowBackToViewportRange === 'function') {
208-
this.rowDetailViewOptions.onRowBackToViewportRange(e, args);
209-
}
210-
});
211-
}
212-
213-
if (this.onBeforeRowOutOfViewportRange) {
214-
this._eventHandler.subscribe(this.onBeforeRowOutOfViewportRange, (event, args) => {
215-
if (typeof this.rowDetailViewOptions?.onBeforeRowOutOfViewportRange === 'function') {
216-
this.rowDetailViewOptions.onBeforeRowOutOfViewportRange(event, args);
217-
}
218-
this.disposeViewByItem(args.item);
219-
});
220-
}
173+
}
174+
);
221175

222-
if (this.onRowOutOfViewportRange) {
223-
this.eventHandler.subscribe(this.onRowOutOfViewportRange, (e, args) => {
224-
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onRowOutOfViewportRange === 'function') {
225-
this.rowDetailViewOptions.onRowOutOfViewportRange(e, args);
226-
}
227-
});
228-
}
176+
this.eventHandler.subscribe(this.onBeforeRowDetailToggle, (e, args) => {
177+
// before toggling row detail, we need to create View Component if it doesn't exist
178+
this.handleOnBeforeRowDetailToggle(e, args);
179+
180+
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onBeforeRowDetailToggle === 'function') {
181+
return this.rowDetailViewOptions.onBeforeRowDetailToggle(e, args);
182+
}
183+
return true;
184+
});
185+
186+
this.eventHandler.subscribe(this.onRowBackToViewportRange, (e, args) => {
187+
// when row is back to viewport range, we will re-render the View Component(s)
188+
this.handleOnRowBackToViewportRange(e, args);
189+
190+
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onRowBackToViewportRange === 'function') {
191+
this.rowDetailViewOptions.onRowBackToViewportRange(e, args);
192+
}
193+
});
194+
195+
this._eventHandler.subscribe(this.onBeforeRowOutOfViewportRange, (event, args) => {
196+
if (typeof this.rowDetailViewOptions?.onBeforeRowOutOfViewportRange === 'function') {
197+
this.rowDetailViewOptions.onBeforeRowOutOfViewportRange(event, args);
198+
}
199+
this.disposeViewByItem(args.item);
200+
});
201+
202+
this.eventHandler.subscribe(this.onRowOutOfViewportRange, (e, args) => {
203+
if (this.rowDetailViewOptions && typeof this.rowDetailViewOptions.onRowOutOfViewportRange === 'function') {
204+
this.rowDetailViewOptions.onRowOutOfViewportRange(e, args);
205+
}
206+
});
229207

230208
// --
231209
// hook some events needed by the Plugin itself

‎test/cypress/e2e/home.cy.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,6 @@ describe('Home Page', () => {
22
it('should display Home Page', () => {
33
cy.visit(`${Cypress.config('baseUrl')}/home`);
44

5-
cy.get('h2').should(($h2) => {
6-
expect($h2, 'text content').to.have.text('Angular-Slickgrid - Demo Site');
7-
});
8-
9-
cy.get('.subtitle').contains('This site is to demo multiple usage of Angular-Slickgrid');
5+
cy.get('h2').should('have.text', 'Angular-Slickgrid - Demo Site');
106
});
117
});

0 commit comments

Comments
 (0)