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(core): EventHandler subscribed event should be SlickEventData type #1327

Merged
merged 3 commits into from
Jan 12, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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