Skip to content

Commit

Permalink
fix(core): EventHandler subscribed event should be SlickEventData type (
Browse files Browse the repository at this point in the history
#1327)

* fix(core): EventHandler subscribed event should be SlickEventData type
  • Loading branch information
ghiscoding authored Jan 12, 2024
1 parent b558f68 commit 2573310
Show file tree
Hide file tree
Showing 35 changed files with 249 additions and 215 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/cypress.yml
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ jobs:
wait-on: 'http://localhost:8888'
config-file: test/cypress.config.ts
browser: chrome
record: false
record: true
env:
# pass the Dashboard record key as an environment variable
CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }}
Expand Down
11 changes: 7 additions & 4 deletions packages/common/src/core/__tests__/slickCore.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@ describe('SlickCore file', () => {
it('should call isDefaultPrevented() and expect truthy when event propagation is stopped by calling preventDefault()', () => {
const ed = new SlickEventData();

expect(ed.defaultPrevented).toBeFalsy();
expect(ed.isDefaultPrevented()).toBeFalsy();

ed.preventDefault();

expect(ed.defaultPrevented).toBeTruthy();
expect(ed.isDefaultPrevented()).toBeTruthy();
});

Expand All @@ -59,6 +61,7 @@ describe('SlickCore file', () => {
const evtSpy = jest.spyOn(evt, 'preventDefault');
const ed = new SlickEventData(evt);

expect(ed.defaultPrevented).toBeFalsy();
expect(ed.isDefaultPrevented()).toBeFalsy();

ed.preventDefault();
Expand Down Expand Up @@ -364,23 +367,23 @@ describe('SlickCore file', () => {
const elock = new SlickEditorLock();
elock.activate(ec);

expect(() => elock.activate(ec2)).toThrow(`SlickEditorLock.activate: an editController is still active, can't activate another editController`)
expect(() => elock.activate(ec2)).toThrow(`SlickEditorLock.activate: an editController is still active, can't activate another editController`);
});

it('should throw when trying to call activate() with an EditController that forgot to implement commitCurrentEdit() method', () => {
const cancelSpy = jest.fn();
const ec = { cancelCurrentEdit: cancelSpy, } as any;

const elock = new SlickEditorLock();
expect(() => elock.activate(ec)).toThrow(`SlickEditorLock.activate: editController must implement .commitCurrentEdit()`)
expect(() => elock.activate(ec)).toThrow(`SlickEditorLock.activate: editController must implement .commitCurrentEdit()`);
});

it('should throw when trying to call activate() with an EditController that forgot to implement cancelCurrentEdit() method', () => {
const commitSpy = jest.fn();
const ec = { commitCurrentEdit: commitSpy, } as any;

const elock = new SlickEditorLock();
expect(() => elock.activate(ec)).toThrow(`SlickEditorLock.activate: editController must implement .cancelCurrentEdit()`)
expect(() => elock.activate(ec)).toThrow(`SlickEditorLock.activate: editController must implement .cancelCurrentEdit()`);
});

it('should deactivate an EditController and expect isActive() to be falsy', () => {
Expand Down Expand Up @@ -408,7 +411,7 @@ describe('SlickCore file', () => {
const elock = new SlickEditorLock();
elock.activate(ec);

expect(() => elock.deactivate(ec2)).toThrow(`SlickEditorLock.deactivate: specified editController is not the currently active one`)
expect(() => elock.deactivate(ec2)).toThrow(`SlickEditorLock.deactivate: specified editController is not the currently active one`);
});

it('should expect active EditController.commitCurrentEdit() being called when calling commitCurrentEdit() after it was activated', () => {
Expand Down
37 changes: 31 additions & 6 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: any, args: ArgType) => void;
export type Handler<ArgType = any> = (e: SlickEventData, args: ArgType) => void;

export interface BasePubSub {
publish<ArgType = any>(_eventName: string | any, _data?: ArgType): any;
Expand All @@ -30,7 +30,32 @@ export class SlickEventData<ArgType = any> {
protected _isDefaultPrevented = false;
protected nativeEvent?: Event | null;
protected returnValue: any = undefined;
protected target?: EventTarget | null;
protected _eventTarget?: EventTarget | 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, ...)
readonly altKey?: boolean;
readonly ctrlKey?: boolean;
readonly metaKey?: boolean;
readonly shiftKey?: boolean;
readonly key?: string;
readonly keyCode?: number;
readonly clientX?: number;
readonly clientY?: number;
readonly offsetX?: number;
readonly offsetY?: number;
readonly pageX?: number;
readonly pageY?: number;
readonly bubbles?: boolean;
readonly target?: HTMLElement;
readonly type?: string;
readonly which?: number;
readonly x?: number;
readonly y?: number;

get defaultPrevented() {
return this._isDefaultPrevented;
}

constructor(protected event?: Event | null, protected args?: ArgType) {
this.nativeEvent = event;
Expand All @@ -42,10 +67,10 @@ export class SlickEventData<ArgType = any> {
[
'altKey', 'ctrlKey', 'metaKey', 'shiftKey', 'key', 'keyCode',
'clientX', 'clientY', 'offsetX', 'offsetY', 'pageX', 'pageY',
'bubbles', 'type', 'which', 'x', 'y'
'bubbles', 'target', 'type', 'which', 'x', 'y'
].forEach(key => (this as any)[key] = event[key as keyof Event]);
}
this.target = this.nativeEvent ? this.nativeEvent.target : undefined;
this._eventTarget = this.nativeEvent ? this.nativeEvent.target : undefined;
}

/**
Expand Down Expand Up @@ -186,13 +211,13 @@ export class SlickEvent<ArgType = any> {
scope = scope || this;

for (let i = 0; i < this._handlers.length && !(sed.isPropagationStopped() || sed.isImmediatePropagationStopped()); i++) {
const returnValue = this._handlers[i].call(scope, sed as SlickEvent | SlickEventData, args);
const returnValue = this._handlers[i].call(scope, sed, args);
sed.addReturnValue(returnValue);
}

// 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?: Event | SlickEventData; nativeEvent?: Event; }>(this.eventName, { args, eventData: sed });
const ret = this._pubSubService.publish<{ args: ArgType; eventData?: SlickEventData; nativeEvent?: Event; }>(this.eventName, { args, eventData: sed });
sed.addReturnValue(ret);
}
return sed;
Expand Down
10 changes: 5 additions & 5 deletions packages/common/src/core/slickDataview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ export class SlickDataView<TData extends SlickDataItem = any> implements CustomD
protected items: TData[] = []; // data by index
protected rows: TData[] = []; // data by row
protected idxById = new Map<DataIdType, number>(); // indexes by id
protected rowsById: { [id: DataIdType]: number } | undefined = undefined; // rows by id; lazy-calculated
protected rowsById: { [id: DataIdType]: number; } | undefined = undefined; // rows by id; lazy-calculated
protected filter: FilterFn<TData> | null = null; // filter function
protected filterCSPSafe: FilterFn<TData> | null = null; // filter function
protected updated: ({ [id: DataIdType]: boolean }) | null = null; // updated item ids
protected updated: ({ [id: DataIdType]: boolean; }) | null = null; // updated item ids
protected suspend = false; // suspends the recalculation
protected isBulkSuspend = false; // delays protectedious operations like the
// index update and delete to efficient
Expand Down Expand Up @@ -107,7 +107,7 @@ export class SlickDataView<TData extends SlickDataItem = any> implements CustomD
displayTotalsRow: true,
lazyTotalsCalculation: false
};
protected groupingInfos: Array<Grouping & { aggregators: Aggregator[]; getterIsAFn?: boolean; compiledAccumulators: any[]; getter: GroupGetterFn | string }> = [];
protected groupingInfos: Array<Grouping & { aggregators: Aggregator[]; getterIsAFn?: boolean; compiledAccumulators: any[]; getter: GroupGetterFn | string; }> = [];
protected groups: SlickGroup[] = [];
protected toggledGroupsByLevel: any[] = [];
protected groupingDelimiter = ':|:';
Expand Down Expand Up @@ -752,7 +752,7 @@ export class SlickDataView<TData extends SlickDataItem = any> implements CustomD

// overrides for totals rows
if ((item as SlickGroupTotals).__groupTotals) {
return this._options.groupItemMetadataProvider!.getTotalsRowMetadata(item as { group: GroupingFormatterItem });
return this._options.groupItemMetadataProvider!.getTotalsRowMetadata(item as { group: GroupingFormatterItem; });
}

return null;
Expand Down Expand Up @@ -1409,7 +1409,7 @@ export class SlickDataView<TData extends SlickDataItem = any> implements CustomD
}
};

grid.onSelectedRowsChanged.subscribe((_e: Event, args: { rows: number[]; }) => {
grid.onSelectedRowsChanged.subscribe((_e: SlickEventData, args: { rows: number[]; }) => {
if (!inHandler) {
const newSelectedRowIds = this.mapRowsToIds(args.rows);
const selectedRowsChangedArgs = {
Expand Down
17 changes: 8 additions & 9 deletions packages/common/src/core/slickGrid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ import type {
PagingInfo,
SingleColumnSort,
SlickPlugin,
SlickGridEventData,
} from '../interfaces';
import type { SlickDataView } from './slickDataview';

Expand Down Expand Up @@ -128,13 +127,13 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e

// Events
onActiveCellChanged: SlickEvent<OnActiveCellChangedEventArgs>;
onActiveCellPositionChanged: SlickEvent<SlickGridEventData>;
onActiveCellPositionChanged: SlickEvent<{ grid: SlickGrid; }>;
onAddNewRow: SlickEvent<OnAddNewRowEventArgs>;
onAutosizeColumns: SlickEvent<OnAutosizeColumnsEventArgs>;
onBeforeAppendCell: SlickEvent<OnBeforeAppendCellEventArgs>;
onBeforeCellEditorDestroy: SlickEvent<OnBeforeCellEditorDestroyEventArgs>;
onBeforeColumnsResize: SlickEvent<OnBeforeColumnsResizeEventArgs>;
onBeforeDestroy: SlickEvent<SlickGridEventData>;
onBeforeDestroy: SlickEvent<{ grid: SlickGrid; }>;
onBeforeEditCell: SlickEvent<OnBeforeEditCellEventArgs>;
onBeforeFooterRowCellDestroy: SlickEvent<OnBeforeFooterRowCellDestroyEventArgs>;
onBeforeHeaderCellDestroy: SlickEvent<OnBeforeHeaderCellDestroyEventArgs>;
Expand All @@ -150,7 +149,7 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
onColumnsResized: SlickEvent<OnColumnsResizedEventArgs>;
onColumnsResizeDblClick: SlickEvent<OnColumnsResizeDblClickEventArgs>;
onCompositeEditorChange: SlickEvent<OnCompositeEditorChangeEventArgs>;
onContextMenu: SlickEvent<SlickGridEventData>;
onContextMenu: SlickEvent<{ grid: SlickGrid; }>;
onDrag: SlickEvent<DragRowMove>;
onDblClick: SlickEvent<OnDblClickEventArgs>;
onDragInit: SlickEvent<DragRowMove>;
Expand All @@ -177,7 +176,7 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
onActivateChangedOptions: SlickEvent<OnActivateChangedOptionsEventArgs>;
onSort: SlickEvent<SingleColumnSort | MultiColumnSort>;
onValidationError: SlickEvent<OnValidationErrorEventArgs>;
onViewportChanged: SlickEvent<SlickGridEventData>;
onViewportChanged: SlickEvent<{ grid: SlickGrid; }>;

// ---
// protected variables
Expand Down Expand Up @@ -485,13 +484,13 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e

this._pubSubService = externalPubSub;
this.onActiveCellChanged = new SlickEvent<OnActiveCellChangedEventArgs>('onActiveCellChanged', externalPubSub);
this.onActiveCellPositionChanged = new SlickEvent<SlickGridEventData>('onActiveCellPositionChanged', externalPubSub);
this.onActiveCellPositionChanged = new SlickEvent<{ grid: SlickGrid; }>('onActiveCellPositionChanged', externalPubSub);
this.onAddNewRow = new SlickEvent<OnAddNewRowEventArgs>('onAddNewRow', externalPubSub);
this.onAutosizeColumns = new SlickEvent<OnAutosizeColumnsEventArgs>('onAutosizeColumns', externalPubSub);
this.onBeforeAppendCell = new SlickEvent<OnBeforeAppendCellEventArgs>('onBeforeAppendCell', externalPubSub);
this.onBeforeCellEditorDestroy = new SlickEvent<OnBeforeCellEditorDestroyEventArgs>('onBeforeCellEditorDestroy', externalPubSub);
this.onBeforeColumnsResize = new SlickEvent<OnBeforeColumnsResizeEventArgs>('onBeforeColumnsResize', externalPubSub);
this.onBeforeDestroy = new SlickEvent<SlickGridEventData>('onBeforeDestroy', externalPubSub);
this.onBeforeDestroy = new SlickEvent<{ grid: SlickGrid; }>('onBeforeDestroy', externalPubSub);
this.onBeforeEditCell = new SlickEvent<OnBeforeEditCellEventArgs>('onBeforeEditCell', externalPubSub);
this.onBeforeFooterRowCellDestroy = new SlickEvent<OnBeforeFooterRowCellDestroyEventArgs>('onBeforeFooterRowCellDestroy', externalPubSub);
this.onBeforeHeaderCellDestroy = new SlickEvent<OnBeforeHeaderCellDestroyEventArgs>('onBeforeHeaderCellDestroy', externalPubSub);
Expand All @@ -507,7 +506,7 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
this.onColumnsResized = new SlickEvent<OnColumnsResizedEventArgs>('onColumnsResized', externalPubSub);
this.onColumnsResizeDblClick = new SlickEvent<OnColumnsResizeDblClickEventArgs>('onColumnsResizeDblClick', externalPubSub);
this.onCompositeEditorChange = new SlickEvent<OnCompositeEditorChangeEventArgs>('onCompositeEditorChange', externalPubSub);
this.onContextMenu = new SlickEvent<SlickGridEventData>('onContextMenu', externalPubSub);
this.onContextMenu = new SlickEvent<{ grid: SlickGrid; }>('onContextMenu', externalPubSub);
this.onDrag = new SlickEvent<DragRowMove>('onDrag', externalPubSub);
this.onDblClick = new SlickEvent<OnDblClickEventArgs>('onDblClick', externalPubSub);
this.onDragInit = new SlickEvent<DragRowMove>('onDragInit', externalPubSub);
Expand All @@ -534,7 +533,7 @@ export class SlickGrid<TData = any, C extends Column<TData> = Column<TData>, O e
this.onActivateChangedOptions = new SlickEvent<OnActivateChangedOptionsEventArgs>('onActivateChangedOptions', externalPubSub);
this.onSort = new SlickEvent<SingleColumnSort | MultiColumnSort>('onSort', externalPubSub);
this.onValidationError = new SlickEvent<OnValidationErrorEventArgs>('onValidationError', externalPubSub);
this.onViewportChanged = new SlickEvent<SlickGridEventData>('onViewportChanged', externalPubSub);
this.onViewportChanged = new SlickEvent<{ grid: SlickGrid; }>('onViewportChanged', externalPubSub);

this.initialize(options);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ describe('CellMenu Plugin', () => {
expect(commandListElm.querySelectorAll('.slick-menu-item').length).toBe(7);
expect(document.body.querySelector('button.close')!.ariaLabel).toBe('Close'); // JSDOM doesn't support ariaLabel, but we can test attribute this way
expect(removeExtraSpaces(document.body.innerHTML)).toBe(removeExtraSpaces(
`<div class="slick-cell-menu slick-menu-level-0 slickgrid12345 dropdown dropleft" style="display: block; top: 0px; left: 0px;" aria-expanded="true">
`<div class="slick-cell-menu slick-menu-level-0 slickgrid12345 dropdown dropleft" style="top: 0px; display: block; left: 0px;" aria-expanded="true">
<div class="slick-menu-command-list" role="menu">
<div class="slick-command-header no-title with-close">
<button aria-label="Close" class="close" type="button" data-dismiss="slick-menu">×</button>
Expand Down Expand Up @@ -784,7 +784,7 @@ describe('CellMenu Plugin', () => {
expect(optionListElm.querySelectorAll('.slick-menu-item').length).toBe(6);
expect(document.body.querySelector('button.close')!.ariaLabel).toBe('Close'); // JSDOM doesn't support ariaLabel, but we can test attribute this way
expect(removeExtraSpaces(document.body.innerHTML)).toBe(removeExtraSpaces(
`<div class="slick-cell-menu slick-menu-level-0 slickgrid12345 dropdown dropright" style="display: block; top: 0px; left: 0px;" aria-expanded="true">
`<div class="slick-cell-menu slick-menu-level-0 slickgrid12345 dropdown dropright" style="top: 0px; display: block; left: 0px;" aria-expanded="true">
<div class="slick-menu-option-list" role="menu">
<div class="slick-option-header no-title with-close">
<button aria-label="Close" class="close" type="button" data-dismiss="slick-menu">×</button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ describe('ContextMenu Plugin', () => {
expect(commandListElm.querySelectorAll('.slick-menu-item').length).toBe(6);
expect(document.body.querySelector('button.close')!.ariaLabel).toBe('Close'); // JSDOM doesn't support ariaLabel, but we can test attribute this way
expect(removeExtraSpaces(document.body.innerHTML)).toBe(removeExtraSpaces(
`<div class="slick-context-menu slick-menu-level-0 slickgrid12345 dropdown dropright" style="display: block; top: 0px; left: 0px;" aria-expanded="true">
`<div class="slick-context-menu slick-menu-level-0 slickgrid12345 dropdown dropright" style="top: 0px; display: block; left: 0px;" aria-expanded="true">
<div class="slick-menu-command-list" role="menu">
<div class="slick-command-header no-title with-close">
<button aria-label="Close" class="close" type="button" data-dismiss="slick-menu">×</button>
Expand Down Expand Up @@ -1335,7 +1335,7 @@ describe('ContextMenu Plugin', () => {
expect(optionListElm.querySelectorAll('.slick-menu-item').length).toBe(6);
expect(document.body.querySelector('button.close')!.ariaLabel).toBe('Close'); // JSDOM doesn't support ariaLabel, but we can test attribute this way
expect(removeExtraSpaces(document.body.innerHTML)).toBe(removeExtraSpaces(
`<div class="slick-context-menu slick-menu-level-0 slickgrid12345 dropdown dropright" style="display: block; top: 0px; left: 0px;" aria-expanded="true">
`<div class="slick-context-menu slick-menu-level-0 slickgrid12345 dropdown dropright" style="top: 0px; display: block; left: 0px;" aria-expanded="true">
<div class="slick-menu-option-list" role="menu">
<div class="slick-option-header no-title with-close">
<button aria-label="Close" class="close" type="button" data-dismiss="slick-menu">×</button>
Expand Down
Loading

0 comments on commit 2573310

Please sign in to comment.