fixes #436 - Add PointerEvent support to getEventCoordinates method
#437
Conversation
| @@ -0,0 +1,6 @@ | |||
| export function isPointerEvent(event: Event): event is PointerEvent { | |||
| return ( | |||
| (window?.PointerEvent && event instanceof PointerEvent) || | |||
There was a problem hiding this comment.
If this for server side check, I believe we can't use optional chaining.
typeof window !== 'undefined' && window.PointerEvent != null && event instanceof window.PointerEvent
There was a problem hiding this comment.
I just cloned this one =) https://github.com/clauderic/dnd-kit/blob/master/packages/utilities/src/event/isMouseEvent.ts
There was a problem hiding this comment.
I see, so there is mistake in isMouseEvent too, might be out of scope of the PR :)
However, optional chaining can't help with checking of global variable - it will fail with reference error.
| @@ -23,7 +23,7 @@ export function getEventCoordinates(event: Event): Coordinates { | |||
| } | |||
| } | |||
|
|
|||
| if (isMouseEvent(event)) { | |||
| if (isMouseEvent(event) || isPointerEvent(event)) { | |||
There was a problem hiding this comment.
I think it would be much simpler to just refactor this to:
| if (isMouseEvent(event) || isPointerEvent(event)) { | |
| if ('clientX' in event && 'clientY' in event) { |
There was a problem hiding this comment.
@clauderic updated
🦋 Changeset detectedLatest commit: 06cbdc1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…tEventCoordinates
chestozo
changed the title
getEventCoordinates method
| (isTouchEvent(activatorEvent) || isMouseEvent(activatorEvent)) | ||
| (isTouchEvent(activatorEvent) || | ||
| isMouseEvent(activatorEvent) || | ||
| isPointerEvent(activatorEvent)) |
There was a problem hiding this comment.
looks like isPointerEvent is still beneficial to have because of this check
| @@ -23,10 +23,11 @@ export function getEventCoordinates(event: Event): Coordinates { | |||
| } | |||
| } | |||
|
|
|||
| if (isMouseEvent(event)) { | |||
| // In case of MouseEvent or PointerEvent. | |||
| if ('clientX' in event && 'clientY' in event) { | |||
There was a problem hiding this comment.
function hasViewportRelativeCoordinates(event: Event): event is Event & Pick<PointerEvent, 'clientX' | 'clientY'> {
return 'clientX' in event && 'clientY' in event;
}| if ('clientX' in event && 'clientY' in event) { | |
| if (hasViewportRelativeCoordinates(event)) { |
| x: (event as MouseEvent).clientX, | ||
| y: (event as MouseEvent).clientY, |
There was a problem hiding this comment.
| x: (event as MouseEvent).clientX, | |
| y: (event as MouseEvent).clientY, | |
| x: event.clientX, | |
| y: event.clientY, |
| export function isPointerEvent(event: Event): event is PointerEvent { | ||
| return ( | ||
| (window?.PointerEvent && event instanceof PointerEvent) || | ||
| event.type.includes('pointer') |
There was a problem hiding this comment.
Is event.type.includes('pointer') required? Is this for Cypress?
There was a problem hiding this comment.
yes we are using dnd-lib with pointer events and we were trying to reuse your cypress extensions but for pointer events
…ouse events and pointer events
|
addressed comments |
In order to test DnD with Pointer events - like mentioned here - #208 we need to change how
getEventCoordinateshandlespointer*events (right now we are getting always{x:0, y:0}.