Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pubsub): externalize PubSub event to SlickEventData to stop bubbling #1444

Merged
merged 2 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions examples/vite-demo-vanilla-bundle/src/examples/example12.ts
Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,7 @@ export default class Example12 {

if (column && item) {
if (!checkItemIsEditable(item, column, grid)) {
event.preventDefault();
eventData.stopImmediatePropagation();
eventData.preventDefault();
return false;
}
}
Expand Down
20 changes: 17 additions & 3 deletions packages/common/src/core/__tests__/slickCore.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ describe('SlickCore file', () => {

onClick.notify({ hello: 'world' }, ed);

expect(pubSubServiceStub.publish).toHaveBeenCalledWith('onClick', { eventData: ed, args: { hello: 'world' } });
expect(pubSubServiceStub.publish).toHaveBeenCalledWith('onClick', { eventData: ed, args: { hello: 'world' } }, undefined, expect.any(Function));
});

it('should be able to mix a PubSub with regular SlickEvent subscribe and expect both to be triggered by the SlickEvent call notify()', () => {
Expand All @@ -160,7 +160,7 @@ describe('SlickCore file', () => {
onClick.notify({ hello: 'world' }, ed);

expect(spy1).toHaveBeenCalledWith(ed, { hello: 'world' });
expect(pubSubServiceStub.publish).toHaveBeenCalledWith('onClick', { eventData: ed, args: { hello: 'world' } });
expect(pubSubServiceStub.publish).toHaveBeenCalledWith('onClick', { eventData: ed, args: { hello: 'world' } }, undefined, expect.any(Function));
});

it('should be able to call addSlickEventPubSubWhenDefined() and expect PubSub to be available in SlickEvent', () => {
Expand All @@ -173,7 +173,21 @@ describe('SlickCore file', () => {
onClick.notify({ hello: 'world' }, ed);

expect(setPubSubSpy).toHaveBeenCalledWith(pubSubServiceStub);
expect(pubSubServiceStub.publish).toHaveBeenCalledWith('onClick', { eventData: ed, args: { hello: 'world' } });
expect(pubSubServiceStub.publish).toHaveBeenCalledWith('onClick', { eventData: ed, args: { hello: 'world' } }, undefined, expect.any(Function));
});

it('should be able to add a PubSub instance to the SlickEvent call notify() and expect PubSub .publish() to be called and the externalize event callback be called also', () => {
const ed = new SlickEventData();
const pubSubCopy = { ...pubSubServiceStub };
const onClick = new SlickEvent('onClick', pubSubCopy);
pubSubCopy.publish = (_evtName, _data, _delay, evtCallback) => {
evtCallback!(new CustomEvent('click'));
}

onClick.notify({ hello: 'world' }, ed);

expect(ed.nativeEvent).toBeDefined();
expect(pubSubServiceStub.publish).toHaveBeenCalledWith('onClick', { eventData: expect.any(Object), args: { hello: 'world' } }, undefined, expect.any(Function));
});
});

Expand Down
18 changes: 13 additions & 5 deletions packages/common/src/core/slickCore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import type { CSSStyleDeclarationWritable, EditController } from '../interfaces'
export type Handler<ArgType = any> = (e: SlickEventData<ArgType>, args: ArgType) => void;

export interface BasePubSub {
publish<ArgType = any>(_eventName: string | any, _data?: ArgType): any;
publish<ArgType = any>(_eventName: string | any, _data?: ArgType, delay?: number, assignEventCallback?: any): any;
subscribe<ArgType = any>(_eventName: string | Function, _callback: (data: ArgType) => void): any;
}
type PubSubPublishType<ArgType = any> = { args: ArgType; eventData?: SlickEventData<ArgType>; nativeEvent?: Event; };

/**
* An event object for passing data to event handlers and letting them control propagation.
Expand All @@ -28,9 +29,9 @@ export class SlickEventData<ArgType = any> {
protected _isPropagationStopped = false;
protected _isImmediatePropagationStopped = false;
protected _isDefaultPrevented = false;
protected nativeEvent?: Event | null;
protected returnValue: any = undefined;
protected _eventTarget?: EventTarget | null;
nativeEvent?: Event | null;

// public props that can be optionally pulled from the provided Event in constructor
// they are all optional props because it really depends on the type of Event provided (KeyboardEvent, MouseEvent, ...)
Expand Down Expand Up @@ -100,7 +101,7 @@ export class SlickEventData<ArgType = any> {
if (this.nativeEvent) {
this.nativeEvent.stopImmediatePropagation();
}
};
}

/**
* Returns whether stopImmediatePropagation was called on this event object.\
Expand All @@ -109,7 +110,7 @@ export class SlickEventData<ArgType = any> {
*/
isImmediatePropagationStopped() {
return this._isImmediatePropagationStopped;
};
}

getNativeEvent<E extends Event>() {
return this.nativeEvent as E;
Expand Down Expand Up @@ -217,7 +218,14 @@ 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<ArgType>; nativeEvent?: Event; }>(this.eventName, { args, eventData: sed });
const ret = this._pubSubService.publish<PubSubPublishType>(
this.eventName,
{ args, eventData: sed },
undefined,
// assign the PubSub internal event to our SlickEventData.nativeEvent
// so that we can call preventDefault() which would return a `returnValue = false`
(evt: Event) => sed.nativeEvent ??= evt
);
sed.addReturnValue(ret);
}
return sed;
Expand Down
21 changes: 17 additions & 4 deletions packages/event-pub-sub/src/eventPubSub.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('EventPubSub Service', () => {

expect(publishResult).toBeTruthy();
expect(getEventNameSpy).toHaveBeenCalledWith('onClick', '');
expect(dispatchSpy).toHaveBeenCalledWith('onClick', { name: 'John' }, true, true);
expect(dispatchSpy).toHaveBeenCalledWith('onClick', { name: 'John' }, true, true, undefined);
});

it('should call publish method and expect it to return it a simple boolean (without delay argument provided)', () => {
Expand All @@ -53,7 +53,7 @@ describe('EventPubSub Service', () => {

expect(publishResult).toBeTruthy();
expect(getEventNameSpy).toHaveBeenCalledWith('onClick', '');
expect(dispatchSpy).toHaveBeenCalledWith('onClick', { name: 'John' }, true, true);
expect(dispatchSpy).toHaveBeenCalledWith('onClick', { name: 'John' }, true, true, undefined);
});

it('should call publish method and expect it to return it a boolean in a Promise when a delay is provided', async () => {
Expand All @@ -64,7 +64,7 @@ describe('EventPubSub Service', () => {

expect(publishResult).toBeTruthy();
expect(getEventNameSpy).toHaveBeenCalledWith('onClick', '');
expect(dispatchSpy).toHaveBeenCalledWith('onClick', { name: 'John' }, true, true);
expect(dispatchSpy).toHaveBeenCalledWith('onClick', { name: 'John' }, true, true, undefined);
});

it('should define a different event name styling and expect "dispatchCustomEvent" and "getEventNameByNamingConvention" to be called', () => {
Expand All @@ -76,7 +76,20 @@ describe('EventPubSub Service', () => {

expect(publishResult).toBeTruthy();
expect(getEventNameSpy).toHaveBeenCalledWith('onClick', '');
expect(dispatchSpy).toHaveBeenCalledWith('onclick', { name: 'John' }, true, true);
expect(dispatchSpy).toHaveBeenCalledWith('onclick', { name: 'John' }, true, true, undefined);
});

it('should call publish method with an externalize event callback and expect it to receive the custom event used by the pubsub', () => {
const dispatchSpy = jest.spyOn(service, 'dispatchCustomEvent');
const getEventNameSpy = jest.spyOn(service, 'getEventNameByNamingConvention');

const obj: any = { nativeEvent: null };
const publishResult = service.publish('onClick', { name: 'John' }, undefined, (evt) => obj.nativeEvent = evt);

expect(obj.nativeEvent).toBeInstanceOf(CustomEvent);
expect(publishResult).toBeTruthy();
expect(getEventNameSpy).toHaveBeenCalledWith('onClick', '');
expect(dispatchSpy).toHaveBeenCalledWith('onClick', { name: 'John' }, true, true, expect.any(Function));
});
});

Expand Down
16 changes: 11 additions & 5 deletions packages/event-pub-sub/src/eventPubSub.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,19 @@ export class EventPubSubService implements BasePubSubService {
* @param {*} data - optional data to include in the dispatching
* @param {Boolean} isBubbling - is the event bubbling up?
* @param {Boolean} isCancelable - is the event cancellable?
* @param {Function} externalizeEventCallback - user can optionally retrieve the CustomEvent used in the PubSub for its own usage via a callback (called just before the event dispatch)
* @returns {Boolean} returns true if either event's cancelable attribute value is false or its preventDefault() method was not invoked, and false otherwise.
*/
dispatchCustomEvent<T = any>(eventName: string, data?: T, isBubbling = true, isCancelable = true) {
dispatchCustomEvent<T = any>(eventName: string, data?: T, isBubbling = true, isCancelable = true, externalizeEventCallback?: (e: Event) => void) {
const eventInit: CustomEventInit<T> = { bubbles: isBubbling, cancelable: isCancelable };
if (data) {
eventInit.detail = data;
}
return this._elementSource?.dispatchEvent(new CustomEvent<T>(eventName, eventInit));
const custEvent = new CustomEvent<T>(eventName, eventInit);
if (typeof externalizeEventCallback === 'function') {
externalizeEventCallback(custEvent);
}
return this._elementSource?.dispatchEvent(custEvent);
}

/**
Expand Down Expand Up @@ -93,18 +98,19 @@ export class EventPubSubService implements BasePubSubService {
* @param {String} event - The event or channel to publish to.
* @param {*} data - The data to publish on the channel.
* @param {Number} delay - optional argument to delay the publish event
* @param {Function} externalizeEventCallback - user can optionally retrieve the CustomEvent used in the PubSub for its own usage via a callback (called just before the event dispatch)
* @returns {Boolean | Promise} - return type will be a Boolean unless a `delay` is provided then a `Promise<Boolean>` will be returned
*/
publish<T = any>(eventName: string, data?: T, delay?: number): boolean | Promise<boolean> {
publish<T = any>(eventName: string, data?: T, delay?: number, externalizeEventCallback?: (e: Event) => void): boolean | Promise<boolean> {
const eventNameByConvention = this.getEventNameByNamingConvention(eventName, '');

if (delay) {
return new Promise(resolve => {
clearTimeout(this._timer);
this._timer = setTimeout(() => resolve(this.dispatchCustomEvent<T>(eventNameByConvention, data, true, true)), delay);
this._timer = setTimeout(() => resolve(this.dispatchCustomEvent<T>(eventNameByConvention, data, true, true, externalizeEventCallback)), delay);
});
} else {
return this.dispatchCustomEvent<T>(eventNameByConvention, data, true, true);
return this.dispatchCustomEvent<T>(eventNameByConvention, data, true, true, externalizeEventCallback);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ export interface BasePubSubService {
* Method to publish a message
* @param event The event or channel to publish to.
* @param data The data to publish on the channel.
* @param {Number} delay - optional argument to delay the publish event
* @param {Function} externalizeEventCallback - user can optionally retrieve the CustomEvent used in the PubSub for its own usage via a callback (called just before the event dispatch)
*/
publish<T = any>(_eventName: string | any, _data?: T, _delay?: number): void | boolean | Promise<boolean>;
publish<T = any>(_eventName: string | any, _data?: T, _delay?: number, externalizeEventCallback?: (e: Event) => void): void | boolean | Promise<boolean>;

/**
* Subscribes to a message channel or message type.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ describe('SlickRowDetailView plugin', () => {
expect(beforeRowDetailToggleSpy).not.toHaveBeenCalled();
});

it('should trigger "onAsyncResponse" with Row Detail template with "useRowClick" enabled and then ', () => {
it('should trigger "onAsyncResponse" with Row Detail template with "useRowClick" enabled and then expect DataView to clear/delete rows in the UI when opening Row Detail', () => {
const mockProcess = jest.fn();
const updateItemSpy = jest.spyOn(dataviewStub, 'updateItem');
const asyncEndUpdateSpy = jest.spyOn(plugin.onAsyncEndUpdate, 'notify');
Expand Down