wrong event args signature in grid.onClick observable handler

[b-giavotto]

Describe the bug

the typescript signature of onClick event handler passes an argument fields which is of type Slick.OnClickEventArgs. However the real object passed in is a DOM event, which has the structure reported in the example below:

NB=[I think the argument passed to the event handler should be SlickEventData<ArgType = any>]
NB=[I suspect even other grid onclick handlers has the same mismatch]

SlickEventData {event: PointerEvent, args: {…}, _isPropagationStopped: false, _isImmediatePropagationStopped: false, _isDefaultPrevented: false, …} _isDefaultPrevented: false _isImmediatePropagationStopped: false _isPropagationStopped: false altKey: false args: {row: 1, cell: 0, grid: ExtSlickGrid} arguments_: {row: 1, cell: 0, grid: ExtSlickGrid} bubbles: true clientX: 208 clientY: 142 ctrlKey: false event: PointerEvent {isTrusted: true, pointerId: 1, width: 1, height: 1, pressure: 0, …} key: undefined keyCode: undefined metaKey: false nativeEvent: PointerEvent {isTrusted: true, pointerId: 1, width: 1, height: 1, pressure: 0, …} offsetX: 96 offsetY: 6 pageX: 208 pageY: 142 returnValue: undefined returnValues: (0) [] shiftKey: false target: div.slick-cell.l0.r0.selected type: 'click' which: 1 x: 208 y: 142

Reproduction

does not apply

Which Framework are you using?

Vanilla / Plain JS

Environment Info

| Executable          | Version |
| ------------------- | ------- |
| (framework used)    | latest |
| SlickGrid | VERSION |
| TypeScript          | -- |
| Browser(s)          | --|
| System OS           | -- |

Validations

• Follow our Code of Conduct
• Read the Wikis.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion.
• The provided reproduction is a minimal reproducible example of the bug.

[ghiscoding]

I don't really understand what is wrong, the code you provided is really hard to read and understand what the difference is. Perhaps you could use diff``` and show what is removed by adding a - in front of the line and a + for row to add and show what you expect.

This looks totally fine to me

[ghiscoding]

The 2nd argument of notify is most often a SlickEventData but not always and could be a regular DOM Event which is why there's an instanceof SlickEventData as the first condition check in the method

SlickGrid/src/slick.core.ts

Lines 173 to 185 in 28198a8

	* Fires an event notifying all subscribers.
	* @method notify
	* @param {Object} args Additional data object to be passed to all handlers.
	* @param {EventData} [event] - An <code>EventData</code> object to be passed to all handlers.
	* For DOM events, an existing W3C event object can be passed in.
	* @param {Object} [scope] - The scope ("this") within which the handler will be executed.
	* If not specified, the scope will be set to the <code>Event</code> instance.
	*/
	notify(args: ArgType, evt?: SlickEventData | Event | MergeTypes<SlickEventData, Event> | null, scope?: any) {
	const sed: SlickEventData = evt instanceof SlickEventData
	? evt
	: new SlickEventData(evt, args);
	scope = scope || this;

So I still don't see what the problem is, please provide a repro or print screens to display the problem because I don't see what the problem is. It's possible that I misunderstood original author's intention but please prove me wrong by providing more tangible info.

[ghiscoding]

actually I think I understand now and I've opened PR #969 to address this

[b-giavotto]

actually I think I understand now and I've opened PR #969 to address this

Glad to read this.

[ghiscoding]

actually I can do even better and also infer type on SlickEventData with a second PR #970 and now the first even argument is now fully typed

[b-giavotto]

that's great.