Skip to content

Commit

Permalink
fix: return all onBeforeX events in delayed promise to fix spinner
Browse files Browse the repository at this point in the history
- most framework requires one cycle for the binding to be executed and because of that the spinner was showing too late, we can fix this by having the pubsub publish method to return a promise with setTimeout of 0 (which is 1 cycle) and now the spinner works as intended for all events recently added
  • Loading branch information
ghiscoding committed May 11, 2021
1 parent 360c62c commit bb36d1a
Show file tree
Hide file tree
Showing 9 changed files with 277 additions and 176 deletions.
29 changes: 20 additions & 9 deletions examples/webpack-demo-vanilla-bundle/src/examples/example05.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export class Example5 {
sgb: SlickVanillaGridBundle;
durationOrderByCount = false;
loadingClass = '';
isLargeDataset = false;

constructor() {
this._bindingEventService = new BindingEventService();
Expand All @@ -37,21 +38,30 @@ export class Example5 {
// this.sgb.dataset = this.dataset;

// with large dataset you maybe want to show spinner before/after these events: sorting/filtering/collapsing/expanding
const spinnerClass = 'mdi mdi-load mdi-spin-1s mdi-22px';
this._bindingEventService.bind(gridContainerElm, 'onbeforefilterchanged', () => this.loadingClass = spinnerClass);
this._bindingEventService.bind(gridContainerElm, 'onfilterchanged', () => this.loadingClass = '');
this._bindingEventService.bind(gridContainerElm, 'onbeforefilterclear', () => this.loadingClass = spinnerClass);
this._bindingEventService.bind(gridContainerElm, 'onfiltercleared', () => this.loadingClass = '');
this._bindingEventService.bind(gridContainerElm, 'onbeforesortchange', () => this.loadingClass = spinnerClass);
this._bindingEventService.bind(gridContainerElm, 'onsortchanged', () => this.loadingClass = '');
this._bindingEventService.bind(gridContainerElm, 'onbeforetoggletreecollapse', () => this.loadingClass = spinnerClass);
this._bindingEventService.bind(gridContainerElm, 'ontoggletreecollapsed', () => this.loadingClass = '');
this._bindingEventService.bind(gridContainerElm, 'onbeforefilterchange', this.showSpinner.bind(this));
this._bindingEventService.bind(gridContainerElm, 'onfilterchanged', this.hideSpinner.bind(this));
this._bindingEventService.bind(gridContainerElm, 'onbeforefilterclear', this.showSpinner.bind(this));
this._bindingEventService.bind(gridContainerElm, 'onfiltercleared', this.hideSpinner.bind(this));
this._bindingEventService.bind(gridContainerElm, 'onbeforesortchange', this.showSpinner.bind(this));
this._bindingEventService.bind(gridContainerElm, 'onsortchanged', this.hideSpinner.bind(this));
this._bindingEventService.bind(gridContainerElm, 'onbeforetoggletreecollapse', this.showSpinner.bind(this));
this._bindingEventService.bind(gridContainerElm, 'ontoggletreecollapsed', this.hideSpinner.bind(this));
}

dispose() {
this.sgb?.dispose();
}

hideSpinner() {
setTimeout(() => this.loadingClass = '', 200); // delay the hide spinner a bit so it won't to avoid show/hide too quickly
}

showSpinner() {
if (this.isLargeDataset) {
this.loadingClass = 'mdi mdi-load mdi-spin-1s mdi-24px color-alt-success';
}
}

initializeGrid() {
this.columnDefinitions = [
{
Expand Down Expand Up @@ -187,6 +197,7 @@ export class Example5 {
}

loadData(rowCount: number) {
this.isLargeDataset = rowCount > 5000; // we'll show a spinner when it's large, else don't show show since it should be fast enough
let indent = 0;
const parents = [];
const data = [];
Expand Down
123 changes: 66 additions & 57 deletions packages/common/src/services/__tests__/filter.service.spec.ts

Large diffs are not rendered by default.

207 changes: 126 additions & 81 deletions packages/common/src/services/__tests__/sort.service.spec.ts

Large diffs are not rendered by default.

12 changes: 6 additions & 6 deletions packages/common/src/services/__tests__/treeData.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,13 @@ describe('TreeData Service', () => {
jest.clearAllMocks();
});

it('should collapse all items when calling the method with collapsing True', () => {
it('should collapse all items when calling the method with collapsing True', async () => {
const dataGetItemsSpy = jest.spyOn(dataViewStub, 'getItems').mockReturnValue(itemsMock);
const dataSetItemsSpy = jest.spyOn(dataViewStub, 'setItems');
const pubSubSpy = jest.spyOn(pubSubServiceStub, 'publish');

service.init(gridStub);
service.toggleTreeDataCollapse(true);
await service.toggleTreeDataCollapse(true);

expect(pubSubSpy).toHaveBeenCalledWith(`onBeforeToggleTreeCollapse`, { collapsing: true });
expect(pubSubSpy).toHaveBeenCalledWith(`onToggleTreeCollapsed`, { collapsing: true });
Expand All @@ -257,14 +257,14 @@ describe('TreeData Service', () => {
]);
});

it('should collapse all items with a custom collapsed property when calling the method with collapsing True', () => {
it('should collapse all items with a custom collapsed property when calling the method with collapsing True', async () => {
gridOptionsMock.treeDataOptions!.collapsedPropName = 'customCollapsed';
const dataGetItemsSpy = jest.spyOn(dataViewStub, 'getItems').mockReturnValue(itemsMock);
const dataSetItemsSpy = jest.spyOn(dataViewStub, 'setItems');
const pubSubSpy = jest.spyOn(pubSubServiceStub, 'publish');

service.init(gridStub);
service.toggleTreeDataCollapse(true);
await service.toggleTreeDataCollapse(true);

expect(pubSubSpy).toHaveBeenCalledWith(`onBeforeToggleTreeCollapse`, { collapsing: true });
expect(pubSubSpy).toHaveBeenCalledWith(`onToggleTreeCollapsed`, { collapsing: true });
Expand All @@ -275,13 +275,13 @@ describe('TreeData Service', () => {
]);
});

it('should expand all items when calling the method with collapsing False', () => {
it('should expand all items when calling the method with collapsing False', async () => {
const dataGetItemsSpy = jest.spyOn(dataViewStub, 'getItems').mockReturnValue(itemsMock);
const dataSetItemsSpy = jest.spyOn(dataViewStub, 'setItems');
const pubSubSpy = jest.spyOn(pubSubServiceStub, 'publish');

service.init(gridStub);
service.toggleTreeDataCollapse(false);
await service.toggleTreeDataCollapse(false);

expect(pubSubSpy).toHaveBeenCalledWith(`onBeforeToggleTreeCollapse`, { collapsing: false });
expect(pubSubSpy).toHaveBeenCalledWith(`onToggleTreeCollapsed`, { collapsing: false });
Expand Down
36 changes: 20 additions & 16 deletions packages/common/src/services/filter.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,12 @@ export class FilterService {
// bind any search filter change (e.g. input filter input change event)
if (this._onSearchChange) {
const onSearchChangeHandler = this._onSearchChange;
(this._eventHandler as SlickEventHandler<GetSlickEventType<typeof onSearchChangeHandler>>).subscribe(this._onSearchChange, (_e, args) => {
(this._eventHandler as SlickEventHandler<GetSlickEventType<typeof onSearchChangeHandler>>).subscribe(this._onSearchChange, async (_e, args) => {
const isClearFilterEvent = args?.clearFilterTriggered ?? false;

// emit an onBeforeFilterChange event when it's not called by a clear filter
// emit an onBeforeFilterChange event except when it's called by a clear filter
if (!isClearFilterEvent) {
this.emitFilterChanged(EmitterType.local, true);
await this.emitFilterChanged(EmitterType.local, true);
}

// When using Tree Data, we need to do it in 2 steps
Expand All @@ -219,7 +219,7 @@ export class FilterService {
this._dataView.refresh();
}

// emit an onFilterChanged event when it's not called by a clear filter
// emit an onFilterChanged event except when it's called by a clear filter
if (!isClearFilterEvent) {
this.emitFilterChanged(EmitterType.local);
}
Expand All @@ -233,8 +233,8 @@ export class FilterService {
});
}

clearFilterByColumnId(event: Event, columnId: number | string) {
this.pubSubService.publish('onBeforeFilterClear', { columnId });
async clearFilterByColumnId(event: Event, columnId: number | string): Promise<boolean> {
await this.pubSubService.publish('onBeforeFilterClear', { columnId });

const isBackendApi = this._gridOptions?.backendServiceApi ?? false;
const emitter = isBackendApi ? EmitterType.remote : EmitterType.local;
Expand All @@ -261,13 +261,14 @@ export class FilterService {

// emit an event when filter is cleared
this.emitFilterChanged(emitter);
return true;
}

/** Clear the search filters (below the column titles) */
clearFilters(triggerChange = true) {
async clearFilters(triggerChange = true) {
// emit an event before the process start
if (triggerChange) {
this.pubSubService.publish('onBeforeFilterClear', true);
await this.pubSubService.publish('onBeforeFilterClear', true);
}

this._filtersMetadata.forEach((filter: Filter) => {
Expand Down Expand Up @@ -631,7 +632,7 @@ export class FilterService {
* Other services, like Pagination, can then subscribe to it.
* @param caller
*/
emitFilterChanged(caller: EmitterType, isBeforeExecution = false) {
emitFilterChanged(caller: EmitterType, isBeforeExecution = false): void | Promise<boolean> {
const eventName = isBeforeExecution ? 'onBeforeFilterChange' : 'onFilterChanged';

if (caller === EmitterType.remote && this._gridOptions?.backendServiceApi) {
Expand All @@ -640,17 +641,18 @@ export class FilterService {
if (backendService?.getCurrentFilters) {
currentFilters = backendService.getCurrentFilters() as CurrentFilter[];
}
this.pubSubService.publish(eventName, currentFilters);
return this.pubSubService.publish(eventName, currentFilters);
} else if (caller === EmitterType.local) {
this.pubSubService.publish(eventName, this.getCurrentLocalFilters());
return this.pubSubService.publish(eventName, this.getCurrentLocalFilters());
}
return Promise.resolve(true);
}

async onBackendFilterChange(event: KeyboardEvent, args: any) {
const isTriggeringQueryEvent = args?.shouldTriggerQuery;

if (isTriggeringQueryEvent) {
this.emitFilterChanged(EmitterType.remote, true);
await this.emitFilterChanged(EmitterType.remote, true);
}

if (!args || !args.grid) {
Expand Down Expand Up @@ -792,7 +794,7 @@ export class FilterService {
* @param triggerBackendQuery defaults to True, which will query the backend.
* @param triggerOnSearchChangeEvent defaults to False, can be useful with Tree Data structure where the onSearchEvent has to run to execute a prefiltering step
*/
updateFilters(filters: CurrentFilter[], emitChangedEvent = true, triggerBackendQuery = true, triggerOnSearchChangeEvent = false) {
async updateFilters(filters: CurrentFilter[], emitChangedEvent = true, triggerBackendQuery = true, triggerOnSearchChangeEvent = false): Promise<boolean> {
if (!this._filtersMetadata || this._filtersMetadata.length === 0 || !this._gridOptions || !this._gridOptions.enableFiltering) {
throw new Error('[Slickgrid-Universal] in order to use "updateFilters" method, you need to have Filterable Columns defined in your grid and "enableFiltering" set in your Grid Options');
}
Expand Down Expand Up @@ -822,7 +824,7 @@ export class FilterService {

// trigger the onBeforeFilterChange event before the process
if (emitChangedEvent) {
this.emitFilterChanged(emitterType, true);
await this.emitFilterChanged(emitterType, true);
}

// refresh the DataView and trigger an event after all filters were updated and rendered
Expand All @@ -842,6 +844,7 @@ export class FilterService {
this.emitFilterChanged(emitterType);
}
}
return true;
}

/**
Expand All @@ -856,7 +859,7 @@ export class FilterService {
* @param triggerEvent defaults to True, do we want to emit a filter changed event?
* @param triggerBackendQuery defaults to True, which will query the backend.
*/
updateSingleFilter(filter: CurrentFilter, emitChangedEvent = true, triggerBackendQuery = true) {
async updateSingleFilter(filter: CurrentFilter, emitChangedEvent = true, triggerBackendQuery = true): Promise<boolean> {
const columnDef = this.sharedService.allColumns.find(col => col.id === filter.columnId);
if (columnDef && filter.columnId) {
this._columnFilters = {};
Expand All @@ -878,7 +881,7 @@ export class FilterService {

// trigger the onBeforeFilterChange event before the process
if (emitChangedEvent) {
this.emitFilterChanged(emitterType, true);
await this.emitFilterChanged(emitterType, true);
}

if (backendApi) {
Expand All @@ -905,6 +908,7 @@ export class FilterService {
this.emitFilterChanged(emitterType);
}
}
return true;
}

// --
Expand Down
2 changes: 1 addition & 1 deletion packages/common/src/services/pubSub.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export abstract class PubSubService {
* @param event The event or channel to publish to.
* @param data The data to publish on the channel.
*/
publish<T = any>(_eventName: string | any, _data?: T): void {
publish<T = any>(_eventName: string | any, _data?: T): void | Promise<boolean> {
throw new Error('PubSubService "publish" method must be implemented');
}

Expand Down
4 changes: 2 additions & 2 deletions packages/common/src/services/sort.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -372,11 +372,11 @@ export class SortService {
}

/** When a Sort Changes on a Local grid (JSON dataset) */
onLocalSortChanged(grid: SlickGrid, sortColumns: Array<ColumnSort & { clearSortTriggered?: boolean; }>, forceReSort = false, emitSortChanged = false) {
async onLocalSortChanged(grid: SlickGrid, sortColumns: Array<ColumnSort & { clearSortTriggered?: boolean; }>, forceReSort = false, emitSortChanged = false) {
const datasetIdPropertyName = this._gridOptions?.datasetIdPropertyName ?? 'id';
const isTreeDataEnabled = this._gridOptions?.enableTreeData ?? false;
const dataView = grid.getData?.() as SlickDataView;
this.pubSubService.publish('onBeforeSortChange', { sortColumns });
await this.pubSubService.publish('onBeforeSortChange', { sortColumns });

if (grid && dataView) {
if (forceReSort && !isTreeDataEnabled) {
Expand Down
5 changes: 3 additions & 2 deletions packages/common/src/services/treeData.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,9 @@ export class TreeDataService {
return this.sortService.sortHierarchicalDataset(hierarchicalDataset, finalColumnSorts);
}

toggleTreeDataCollapse(collapsing: boolean) {
async toggleTreeDataCollapse(collapsing: boolean): Promise<boolean> {
// emit an event when filters are all cleared
this.pubSubService.publish('onBeforeToggleTreeCollapse', { collapsing });
await this.pubSubService.publish('onBeforeToggleTreeCollapse', { collapsing });

if (this.gridOptions) {
const treeDataOptions = this.gridOptions.treeDataOptions;
Expand All @@ -165,5 +165,6 @@ export class TreeDataService {
}

this.pubSubService.publish('onToggleTreeCollapsed', { collapsing });
return true;
}
}
35 changes: 33 additions & 2 deletions packages/vanilla-bundle/src/services/eventPubSub.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,29 @@ export class EventPubSubService implements PubSubService {
this._elementSource = elementSource || document.createElement('div');
}

publish<T = any>(eventName: string, data?: T) {
/**
* Method to publish a message via a dispatchEvent.
* We return the dispatched event in a Promise with a delayed cycle and we do this because
* most framework require a cycle before the binding is processed and binding a spinner end up showing too late
* for example this is used for these events: onBeforeFilterClear, onBeforeFilterChange, onBeforeToggleTreeCollapse, onBeforeSortChange
* @param event The event or channel to publish to.
* @param data The data to publish on the channel.
*/
publish<T = any>(eventName: string, data?: T): Promise<boolean> {
const eventNameByConvention = this.getEventNameByNamingConvention(eventName, '');
this.dispatchCustomEvent<T>(eventNameByConvention, data, true, false);

return new Promise(resolve => {
const isDispatched = this.dispatchCustomEvent<T>(eventNameByConvention, data, true, false);
setTimeout(() => resolve(isDispatched), 0);
});
}

/**
* Subscribes to a message channel or message type.
* @param event The event channel or event data type.
* @param callback The callback to be invoked when the specified message is published.
* @return possibly a Subscription
*/
subscribe<T = any>(eventName: string, callback: (data: any) => void): any {
const eventNameByConvention = this.getEventNameByNamingConvention(eventName, '');

Expand All @@ -39,17 +57,30 @@ export class EventPubSubService implements PubSubService {
this._subscribedEvents.push({ name: eventNameByConvention, listener: callback });
}

/**
* Subscribes to a custom event message channel or message type.
* This is similar to the "subscribe" except that the callback receives an event typed as CustomEventInit and the data will be inside its "event.detail"
* @param event The event channel or event data type.
* @param callback The callback to be invoked when the specified message is published.
* @return possibly a Subscription
*/
subscribeEvent<T = any>(eventName: string, listener: (event: CustomEventInit<T>) => void): any | void {
const eventNameByConvention = this.getEventNameByNamingConvention(eventName, '');
this._elementSource.addEventListener(eventNameByConvention, listener);
this._subscribedEvents.push({ name: eventNameByConvention, listener });
}

/**
* Unsubscribes a message name
* @param event The event name
* @return possibly a Subscription
*/
unsubscribe(eventName: string, listener: (event: CustomEventInit) => void) {
const eventNameByConvention = this.getEventNameByNamingConvention(eventName, '');
this._elementSource.removeEventListener(eventNameByConvention, listener);
}

/** Unsubscribes all subscriptions that currently exists */
unsubscribeAll(subscriptions?: EventSubscription[]) {
if (Array.isArray(subscriptions)) {
for (const subscription of subscriptions) {
Expand Down

0 comments on commit bb36d1a

Please sign in to comment.