Skip to content

Commit

Permalink
fix(addons): onGroupChanged callback should be executed with Draggable (
Browse files Browse the repository at this point in the history
#826)

- after rewriting all SlickGrid plugins into standalone  Slickgrid-Universal extensions, it seems that the `onGroupChanged` function was forgotten and the new code was never executing it when it was defined as a function callback by the user, this PR fixes that
  • Loading branch information
ghiscoding committed Nov 30, 2022
1 parent 76817e3 commit 35c2631
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 10 deletions.
Expand Up @@ -102,7 +102,7 @@ const mockColumns = [
}
},
{
id: 'medals', name: 'Medals', field: 'medals', width: 50,sortable: true,
id: 'medals', name: 'Medals', field: 'medals', width: 50, sortable: true,
grouping: {
getter: 'medals', aggregators: [new Aggregators.Sum('medals')],
formatter: (g) => `Medals: ${g.value} <span style="color:green">(${g.count} items)</span>`,
Expand Down Expand Up @@ -416,8 +416,10 @@ describe('Draggable Grouping Plugin', () => {
let groupChangedSpy: any;
let mockHeaderColumnDiv1: HTMLDivElement;
let mockHeaderColumnDiv2: HTMLDivElement;
let onGroupChangedCallbackSpy: any;

beforeEach(() => {
onGroupChangedCallbackSpy = jest.fn();
groupChangedSpy = jest.spyOn(plugin.onGroupChanged, 'notify');
mockHeaderColumnDiv1 = document.createElement('div');
mockHeaderColumnDiv1.className = 'slick-dropped-grouping';
Expand All @@ -435,7 +437,7 @@ describe('Draggable Grouping Plugin', () => {
mockHeaderColumnDiv1.appendChild(mockDivPaneContainer1);
mockHeaderColumnDiv2.appendChild(mockDivPaneContainer1);

plugin.init(gridStub, { ...addonOptions, deleteIconCssClass: 'mdi mdi-close' });
plugin.init(gridStub, { ...addonOptions, deleteIconCssClass: 'mdi mdi-close', onGroupChanged: onGroupChangedCallbackSpy });
plugin.setAddonOptions({ deleteIconCssClass: 'mdi mdi-close' });

jest.spyOn(gridStub.getEditorLock(), 'commitCurrentEdit').mockReturnValue(false);
Expand All @@ -454,7 +456,7 @@ describe('Draggable Grouping Plugin', () => {
it('should call sortable "update" from setupColumnDropbox and expect "updateGroupBy" to be called with a sort-group', () => {
expect(plugin.dropboxElement).toEqual(dropzoneElm);
expect(plugin.columnsGroupBy.length).toBeGreaterThan(0);
expect(groupChangedSpy).toHaveBeenCalledWith({
const onGroupChangedArgs = {
caller: 'sort-group',
groupColumns: [{
aggregators: expect.toBeArray(),
Expand All @@ -463,7 +465,9 @@ describe('Draggable Grouping Plugin', () => {
collapsed: false,
sortAsc: true,
}],
});
};
expect(onGroupChangedCallbackSpy).toHaveBeenCalledWith(expect.anything(), onGroupChangedArgs);
expect(groupChangedSpy).toHaveBeenCalledWith(onGroupChangedArgs);

jest.spyOn(gridStub, 'getHeaderColumn').mockReturnValue(mockHeaderColumnDiv1);
plugin.setDroppedGroups('age');
Expand All @@ -477,6 +481,7 @@ describe('Draggable Grouping Plugin', () => {
plugin.clearDroppedGroups();
dropboxPlaceholderElm = preHeaderElm.querySelector('.slick-draggable-dropzone-placeholder') as HTMLDivElement;
expect(dropboxPlaceholderElm.style.display).toBe('inline-block');
expect(onGroupChangedCallbackSpy).toHaveBeenCalledWith(expect.anything(), { caller: 'clear-all', groupColumns: [], });
expect(groupChangedSpy).toHaveBeenCalledWith({ caller: 'clear-all', groupColumns: [], });
});

Expand Down Expand Up @@ -605,7 +610,8 @@ describe('Draggable Grouping Plugin', () => {
});

it('should toggle ascending/descending order when original sort is ascending then user clicked the sorting icon twice', () => {
plugin.init(gridStub, { ...addonOptions });
const onGroupChangedCallbackSpy = jest.fn();
plugin.init(gridStub, { ...addonOptions, onGroupChanged: onGroupChangedCallbackSpy });
const fn = plugin.setupColumnReorder(gridStub, mockHeaderLeftDiv1, {}, setColumnsSpy, setColumnResizeSpy, mockColumns, getColumnIndexSpy, GRID_UID, triggerSpy);
jest.spyOn(fn.sortableLeftInstance, 'toArray').mockReturnValue(['age', 'medals']);
const invalidateSpy = jest.spyOn(gridStub, 'invalidate');
Expand Down Expand Up @@ -642,11 +648,13 @@ describe('Draggable Grouping Plugin', () => {
expect(groupBySortAscIconElm).toBeTruthy();
expect(groupBySortDescIconElm).toBeFalsy();
expect(invalidateSpy).toHaveBeenCalledTimes(2);
expect(onGroupChangedCallbackSpy).toHaveBeenCalledWith(expect.anything(), { caller: 'sort-group', groupColumns: expect.toBeArray(), });
expect(groupChangedSpy).toHaveBeenCalledWith({ caller: 'sort-group', groupColumns: expect.toBeArray(), });
});

it('should toggle ascending/descending order with different icons when original sort is ascending then user clicked the sorting icon twice', () => {
plugin.init(gridStub, { ...addonOptions, sortAscIconCssClass: 'mdi mdi-arrow-up', sortDescIconCssClass: 'mdi mdi-arrow-down' });
const onGroupChangedCallbackSpy = jest.fn();
plugin.init(gridStub, { ...addonOptions, sortAscIconCssClass: 'mdi mdi-arrow-up', sortDescIconCssClass: 'mdi mdi-arrow-down', onGroupChanged: onGroupChangedCallbackSpy });
const fn = plugin.setupColumnReorder(gridStub, mockHeaderLeftDiv1, {}, setColumnsSpy, setColumnResizeSpy, mockColumns, getColumnIndexSpy, GRID_UID, triggerSpy);
jest.spyOn(fn.sortableLeftInstance, 'toArray').mockReturnValue(['age', 'medals']);
const invalidateSpy = jest.spyOn(gridStub, 'invalidate');
Expand Down Expand Up @@ -679,6 +687,7 @@ describe('Draggable Grouping Plugin', () => {
expect(groupBySortAscIconElm).toBeTruthy();
expect(groupBySortDescIconElm).toBeFalsy();
expect(invalidateSpy).toHaveBeenCalledTimes(2);
expect(onGroupChangedCallbackSpy).toHaveBeenCalledWith(expect.anything(), { caller: 'sort-group', groupColumns: expect.toBeArray(), });
expect(groupChangedSpy).toHaveBeenCalledWith({ caller: 'sort-group', groupColumns: expect.toBeArray(), });

const sortResult1 = mockColumns[2].grouping!.comparer!({ value: 'John', count: 0 }, { value: 'Jane', count: 1 });
Expand Down
14 changes: 11 additions & 3 deletions packages/common/src/extensions/slickDraggableGrouping.ts
Expand Up @@ -56,7 +56,7 @@ declare const Slick: SlickNamespace;
* }];
*/
export class SlickDraggableGrouping {
protected _addonOptions!: DraggableGroupingOption;
protected _addonOptions!: DraggableGrouping;
protected _bindEventService: BindingEventService;
protected _droppableInstance?: SortableInstance;
protected _dropzoneElm!: HTMLDivElement;
Expand Down Expand Up @@ -615,13 +615,21 @@ export class SlickDraggableGrouping {
if (this.columnsGroupBy.length === 0) {
this.dataView.setGrouping([]);
this._dropzonePlaceholderElm.style.display = 'inline-block';
this.onGroupChanged.notify({ caller: originator, groupColumns: [] });
this.triggerOnGroupChangedEvent({ caller: originator, groupColumns: [] });
return;
}
const groupingArray: Grouping<any>[] = [];
this.columnsGroupBy.forEach(element => groupingArray.push(element.grouping!));
this.dataView.setGrouping(groupingArray);
this._dropzonePlaceholderElm.style.display = 'none';
this.onGroupChanged.notify({ caller: originator, groupColumns: groupingArray });
this.triggerOnGroupChangedEvent({ caller: originator, groupColumns: groupingArray });
}

/** call notify on slickgrid event and execute onGroupChanged callback when defined as a function by the user */
protected triggerOnGroupChangedEvent(args: { caller?: string; groupColumns: Grouping[] }) {
if (this._addonOptions && typeof this._addonOptions.onGroupChanged === 'function') {
this._addonOptions.onGroupChanged(new Slick.EventData(), args);
}
this.onGroupChanged.notify(args);
}
}
Expand Up @@ -7,7 +7,7 @@ export interface DraggableGrouping extends DraggableGroupingOption {
// Events
// ---------
/** Fired when grouped columns changed */
onGroupChanged?: (e: SlickEventData, args: { caller?: string; groupColumns: Grouping[] }) => void;
onGroupChanged?: (e: SlickEventData | null, args: { caller?: string; groupColumns: Grouping[] }) => void;

/** Fired after extension (plugin) is registered by SlickGrid */
onExtensionRegistered?: (plugin: SlickDraggableGrouping) => void;
Expand Down

0 comments on commit 35c2631

Please sign in to comment.