Skip to content

Commit

Permalink
fix(core): SlickEvent handler event should be type of ArgType (#1328)
Browse files Browse the repository at this point in the history
* fix(core): SlickEvent handler event should be type of ArgType
- continuation of previous PR #1327 to add proper types to SlickEventHandler arguments
  • Loading branch information
ghiscoding authored Jan 12, 2024
1 parent 7f53b52 commit a9cb8ee
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 25 deletions.
8 changes: 4 additions & 4 deletions packages/common/src/core/slickCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import { MergeTypes } from '../enums/index';
import type { CSSStyleDeclarationWritable, EditController } from '../interfaces';

export type Handler<ArgType = any> = (e: SlickEventData, args: ArgType) => void;
export type Handler<ArgType = any> = (e: SlickEventData<ArgType>, args: ArgType) => void;

export interface BasePubSub {
publish<ArgType = any>(_eventName: string | any, _data?: ArgType): any;
Expand Down Expand Up @@ -203,8 +203,8 @@ export class SlickEvent<ArgType = any> {
* @param {Object} [scope] - The scope ("this") within which the handler will be executed.
* If not specified, the scope will be set to the <code>Event</code> instance.
*/
notify(args: ArgType, evt?: SlickEventData | Event | MergeTypes<SlickEventData, Event> | null, scope?: any, ignorePrevEventDataValue = false) {
const sed = evt instanceof SlickEventData ? evt : new SlickEventData(evt, args);
notify(args: ArgType, evt?: SlickEventData<ArgType> | Event | MergeTypes<SlickEventData, Event> | null, scope?: any, ignorePrevEventDataValue = false) {
const sed = evt instanceof SlickEventData ? evt : new SlickEventData<ArgType>(evt, args);
if (ignorePrevEventDataValue) {
sed.resetReturnValue();
}
Expand All @@ -217,7 +217,7 @@ export class SlickEvent<ArgType = any> {

// user can optionally add a global PubSub Service which makes it easy to publish/subscribe to events
if (typeof this._pubSubService?.publish === 'function' && this.eventName) {
const ret = this._pubSubService.publish<{ args: ArgType; eventData?: SlickEventData; nativeEvent?: Event; }>(this.eventName, { args, eventData: sed });
const ret = this._pubSubService.publish<{ args: ArgType; eventData?: SlickEventData<ArgType>; nativeEvent?: Event; }>(this.eventName, { args, eventData: sed });
sed.addReturnValue(ret);
}
return sed;
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/core/slickDataview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1409,7 +1409,7 @@ export class SlickDataView<TData extends SlickDataItem = any> implements CustomD
}
};

grid.onSelectedRowsChanged.subscribe((_e: SlickEventData, args: { rows: number[]; }) => {
grid.onSelectedRowsChanged.subscribe((_e, args) => {
if (!inHandler) {
const newSelectedRowIds = this.mapRowsToIds(args.rows);
const selectedRowsChangedArgs = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ const columnsMock: Column[] = [
];

describe('HeaderMenu Plugin', () => {
const consoleWarnSpy = jest.spyOn(global.console, 'warn').mockReturnValue();
let backendUtilityService: BackendUtilityService;
let extensionUtility: ExtensionUtility;
let translateService: TranslateServiceStub;
Expand Down
3 changes: 3 additions & 0 deletions packages/common/src/interfaces/rowDetailView.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ export interface OnRowDetailAsyncResponseArgs {

/** An explicit view to use instead of template (Optional) */
detailView?: any;

/** SlickGrid instance */
grid?: SlickGrid;
}

/** Fired when the async response finished */
Expand Down
51 changes: 38 additions & 13 deletions packages/row-detail-view-plugin/src/slickRowDetailView.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,11 @@ describe('SlickRowDetailView plugin', () => {
plugin.onAsyncResponse.notify({ item: itemMock, itemDetail: itemMock, }, new SlickEventData());

expect(updateItemSpy).toHaveBeenCalledWith(123, { _detailContent: '<span>Post 123</span>', _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' });
expect(asyncEndUpdateSpy).toHaveBeenCalledWith({ grid: gridStub, item: itemMock, itemDetail: { _detailContent: '<span>Post 123</span>', _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } });
expect(asyncEndUpdateSpy).toHaveBeenCalledWith(
{ grid: gridStub, item: itemMock, itemDetail: { _detailContent: '<span>Post 123</span>', _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } },
expect.anything(),
plugin
);
});

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', () => {
Expand All @@ -275,7 +279,11 @@ describe('SlickRowDetailView plugin', () => {
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' } });
expect(asyncEndUpdateSpy).toHaveBeenCalledWith(
{ grid: gridStub, item: itemMock, itemDetail: { _detailContent: createDomElement('span', { textContent: 'Post 123' }), _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } },
expect.anything(),
plugin
);
});

it('should trigger "onAsyncResponse" with Row Detail template when detailView is provided and expect "updateItem" from DataView to be called with new template & data', () => {
Expand All @@ -288,7 +296,11 @@ describe('SlickRowDetailView plugin', () => {
plugin.onAsyncResponse.notify({ item: itemMock, itemDetail: itemMock, detailView, }, new SlickEventData());

expect(updateItemSpy).toHaveBeenCalledWith(123, { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' });
expect(asyncEndUpdateSpy).toHaveBeenCalledWith({ grid: gridStub, item: itemMock, itemDetail: { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } });
expect(asyncEndUpdateSpy).toHaveBeenCalledWith(
{ grid: gridStub, item: itemMock, itemDetail: { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } },
expect.anything(),
plugin
);
});

it('should trigger onClick and not call anything when "expandableOverride" returns False', () => {
Expand All @@ -310,7 +322,11 @@ describe('SlickRowDetailView plugin', () => {
gridStub.onClick.notify({ row: 0, cell: 1, grid: gridStub }, clickEvent);

expect(updateItemSpy).toHaveBeenCalledWith(123, { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' });
expect(asyncEndUpdateSpy).toHaveBeenCalledWith({ grid: gridStub, item: itemMock, itemDetail: { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } });
expect(asyncEndUpdateSpy).toHaveBeenCalledWith(
{ grid: gridStub, item: itemMock, itemDetail: { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } },
expect.anything(),
plugin
);
expect(preventDefaultSpy).not.toHaveBeenCalled();
expect(stopPropagationSpy).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -391,7 +407,11 @@ describe('SlickRowDetailView plugin', () => {
const filteredItem = plugin.getFilterItem(itemMock);

expect(updateItemSpy).toHaveBeenCalledWith(123, { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' });
expect(asyncEndUpdateSpy).toHaveBeenCalledWith({ grid: gridStub, item: itemMock, itemDetail: { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } });
expect(asyncEndUpdateSpy).toHaveBeenCalledWith(
{ grid: gridStub, item: itemMock, itemDetail: { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } },
expect.anything(),
plugin
);

const clickEvent = new Event('click');
Object.defineProperty(clickEvent, 'target', { writable: true, configurable: true, value: document.createElement('div') });
Expand Down Expand Up @@ -424,7 +444,11 @@ describe('SlickRowDetailView plugin', () => {
plugin.onAsyncResponse.notify({ item: itemMock, itemDetail: itemMock, detailView, }, new SlickEventData());

expect(updateItemSpy).toHaveBeenCalledWith(123, { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' });
expect(asyncEndUpdateSpy).toHaveBeenCalledWith({ grid: gridStub, item: itemMock, itemDetail: { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } });
expect(asyncEndUpdateSpy).toHaveBeenCalledWith(
{ grid: gridStub, item: itemMock, itemDetail: { _detailContent: `<span>loading...</span>`, _detailViewLoaded: true, id: 123, firstName: 'John', lastName: 'Doe' } },
expect.anything(),
plugin
);

plugin.expandDetailView(itemMock);

Expand All @@ -451,14 +475,15 @@ describe('SlickRowDetailView plugin', () => {
id: 123, firstName: 'John', lastName: 'Doe',
}
}, expect.anything(), expect.anything(), true);
expect(afterRowDetailToggleSpy).toHaveBeenCalledWith({
grid: gridStub,
item: {
_collapsed: true, _detailViewLoaded: true, _sizePadding: 0, _height: 150, _detailContent: '<span>loading...</span>',
id: 123, firstName: 'John', lastName: 'Doe',
expect(afterRowDetailToggleSpy).toHaveBeenCalledWith(
{
grid: gridStub,
item: { _collapsed: true, _detailViewLoaded: true, _sizePadding: 0, _height: 150, _detailContent: '<span>loading...</span>', id: 123, firstName: 'John', lastName: 'Doe', },
expandedRows: [],
},
expandedRows: [],
});
expect.anything(),
plugin
);
expect(insertItemSpy).toHaveBeenCalled();
expect(beginUpdateSpy).toHaveBeenCalled();
expect(deleteItemSpy).toHaveBeenCalledTimes(6); // panelRows(5) + 1
Expand Down
13 changes: 7 additions & 6 deletions packages/row-detail-view-plugin/src/slickRowDetailView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,8 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
this.onAsyncResponse.notify({
item,
itemDetail: item,
detailView: item[`${this._keyPrefix}detailContent`]
detailView: item[`${this._keyPrefix}detailContent`],
grid: this._grid
});
this.applyTemplateNewLineHeight(item);
this.dataView.updateItem(item[this._dataViewIdProperty], item);
Expand Down Expand Up @@ -334,7 +335,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
* subscribe to the onAsyncResponse so that the plugin knows when the user server side calls finished
* the response has to be as "args.item" (or "args.itemDetail") with it's data back
*/
handleOnAsyncResponse(_e: SlickEventData, args: { item: any; itemDetail: any; detailView?: any; }) {
handleOnAsyncResponse(e: SlickEventData, args: { item: any; itemDetail: any; detailView?: any; }) {
if (!args || (!args.item && !args.itemDetail)) {
console.error('SlickRowDetailView plugin requires the onAsyncResponse() to supply "args.item" property.');
return;
Expand All @@ -353,7 +354,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
grid: this._grid,
item: itemDetail,
itemDetail,
});
}, e, this);
}

/**
Expand Down Expand Up @@ -711,7 +712,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
grid: this._grid,
item: dataContext,
expandedRows: this._expandedRows,
});
}, e, this);

e.stopPropagation();
e.stopImmediatePropagation();
Expand All @@ -737,7 +738,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
rowIndex,
expandedRows: this._expandedRows,
rowIdsOutOfViewport: this.syncOutOfViewportArray(rowId, true)
});
}, null, this);
}

protected notifyBackToViewportWhenDomExist(item: any, rowId: number | string) {
Expand All @@ -753,7 +754,7 @@ export class SlickRowDetailView implements ExternalResource, UniversalRowDetailV
rowIndex,
expandedRows: this._expandedRows,
rowIdsOutOfViewport: this.syncOutOfViewportArray(rowId, false)
});
}, null, this);
}
}, 100);
}
Expand Down

0 comments on commit a9cb8ee

Please sign in to comment.