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

feat(core): expose all SlickEvent via internal PubSub Service #1311

Merged
merged 1 commit into from
Jan 6, 2024

Conversation

ghiscoding
Copy link
Owner

@ghiscoding ghiscoding commented Jan 5, 2024

  • it was already available for SlickGrid and SlickDataView SlickEvents but it wasn't yet available for other plugin events
  • add a getPubSubService() method in SlickGrid to be able to easily get PubSub instance from within any plugins or for the user themselves
  • you can also cancel the event from bubbling but this is only available when it's a SlickEvent (because regular PubSub events do not include any native event), see example below
  • Note: the internal PubSub can now deal with both regular PubSub events and SlickEvents but their structure are different
  1. regular PubSub event have a structure of:: ((args: any) => ...
    • e.g. onBeforeGridCreate
  2. SlickEvent have a different structure of:: ((e: { eventData: SlickEventData; args: any; }) => ...
    • e.g. onSelectedRangesChanged

Example

So for example, the user can subscribe the event in multiple ways

  1. old approach (still works), in this example we subscribe to the CellSelectionModel
// bind any of the grid events, e.g. onSelectedRangesChanged to show selection range on screen
const cellSelectionModel = this.sgb.slickGrid!.getSelectionModel();
this._eventHandler.subscribe(cellSelectionModel!.onSelectedRangesChanged, (_e, args) => {
  const targetRange = document.querySelector('#selectionRange') as HTMLSpanElement;
  targetRange.textContent = '';
  for (const slickRange of args) {
    targetRange.textContent += JSON.stringify(slickRange);
  }
});
  1. new approach (get PubSub instance via this.sgb.instances?.eventPubSubService
this.sgb.instances?.eventPubSubService?.subscribe('onSelectedRangesChanged', ((e: { eventData: SlickEventData; args: any; })) => console.log(e));
  1. or second new approach (get PubSub instance from SlickGrid via this.sgb.slickGrid?.getPubSubService())
this.sgb.slickGrid?.getPubSubService()?.subscribe('onSelectedRangesChanged', ((e: { eventData: SlickEventData; args: any; })) => console.log(e));

Cancel Event Bubbling

You can also cancel the event bubbling for some of the SlickEvent that supports it, for example the onBeforeEditCell

this.sgb.instances?.eventPubSubService?.subscribe('onSelectedRangesChanged', ((e: { eventData: SlickEventData; args: any; })) => {
  const { column, item, grid } = args;
  if (item.percentComplete < 10) {
    return false;
  }
  return true;
});

- it was already available for SlickGrid and SlickDataView SlickEvents but it wasn't yet available for other plugin events
- adds a `getPubSubService()` method in SlickGrid to be able to easily get PubSub instance within any plugins or for the user themselves
- Note: the internal PubSub can now deal with both regular PubSub events and SlickEvents but their structure are different
1. regular PubSub event have a structure of:: `((e: args: any) => ...`
2. SlickEvent have a different structure of:: `((e: { eventData: SlickEventData; args: any; }) => ...`
Copy link

codecov bot commented Jan 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ac2a526) 95.7% compared to head (25f269d) 95.7%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #1311     +/-   ##
========================================
+ Coverage    95.7%   95.7%   +0.1%     
========================================
  Files         198     198             
  Lines       21339   21358     +19     
  Branches     7095    7100      +5     
========================================
+ Hits        20416   20435     +19     
  Misses        923     923             

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ghiscoding
Copy link
Owner Author

ghiscoding commented Jan 6, 2024

@zewa666 this might interest you... I added this for SlickGrid & SlickDataView when I created the new major but I forgot to add this approach to all plugins.

The reason I added this was mainly for SlickGrid/SlickDataView because prior to this, I was looping through all of their events and was subscribing to all of them, if an SlickEvent was triggering, I was then rethrowing the event to my internal PubSub so that it could be used in frameworks like Angular-Slickgrid... but as you can imagine that had a lot of overhead, so this removes the overhead and also indirectly exposes them to the user if they wish to use them :) Most users probably won't use it since they typically subscribe from the View but sometime it might be useful to subscribe directly in the VM

below is the overhead code I had prior to this new approach (lots of overhead)

if (dataView && grid) {
// expose all Slick Grid Events through dispatch
for (const prop in grid) {
if (grid.hasOwnProperty(prop) && prop.startsWith('on')) {
this._eventHandler.subscribe((grid as any)[prop], (event, args) => {
const gridEventName = this._eventPubSubService.getEventNameByNamingConvention(prop, this._gridOptions?.defaultSlickgridEventPrefix ?? '');
return this._eventPubSubService.dispatchCustomEvent(gridEventName, { eventData: event, args });
});
}
}
// expose all Slick DataView Events through dispatch
for (const prop in dataView) {
if (dataView.hasOwnProperty(prop) && prop.startsWith('on')) {
this._eventHandler.subscribe((dataView as any)[prop], (event, args) => {
const dataViewEventName = this._eventPubSubService.getEventNameByNamingConvention(prop, this._gridOptions?.defaultSlickgridEventPrefix ?? '');
return this._eventPubSubService.dispatchCustomEvent(dataViewEventName, { eventData: event, args });
});
}
}

@ghiscoding ghiscoding merged commit f56edef into master Jan 6, 2024
7 checks passed
@ghiscoding ghiscoding deleted the feat/expose-all-slick-event branch January 6, 2024 00:09
@zewa666
Copy link
Contributor

zewa666 commented Jan 6, 2024

this looks like a great api simplification. not directly affecting me for now but great to know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants