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 click with pointer compatibility #1121

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
76 changes: 37 additions & 39 deletions src/system/pointer/index.ts
Expand Up @@ -40,10 +40,10 @@ export class PointerHost {

new(pointerName: string, keyDef: pointerKey) {
const isPrimary =
keyDef.pointerType !== 'touch' ||
!Object.values(this.registry).some(
p => p.pointerType === 'touch' && !p.isCancelled,
)
keyDef.pointerType !== 'touch' ||
!Object.values(this.registry).some(
p => p.pointerType === 'touch' && !p.isCancelled,
)

if (!isPrimary) {
Object.values(this.registry).forEach(p => {
Expand All @@ -65,7 +65,7 @@ export class PointerHost {
get(pointerName: string) {
if (!this.has(pointerName)) {
throw new Error(
`Trying to access pointer "${pointerName}" which does not exist.`,
`Trying to access pointer "${pointerName}" which does not exist.`,
)
}
return this.registry[pointerName]
Expand All @@ -81,15 +81,15 @@ export class PointerHost {
}

async press(
instance: Instance,
keyDef: pointerKey,
position: PointerPosition,
instance: Instance,
keyDef: pointerKey,
position: PointerPosition,
) {
const pointerName = this.getPointerName(keyDef)
const pointer =
keyDef.pointerType === 'touch'
? this.pointers.new(pointerName, keyDef).init(instance, position)
: this.pointers.get(pointerName)
keyDef.pointerType === 'touch'
? this.pointers.new(pointerName, keyDef).init(instance, position)
: this.pointers.get(pointerName)

// TODO: deprecate the following implicit setting of position
pointer.position = position
Expand All @@ -102,27 +102,27 @@ export class PointerHost {
this.buttons.down(keyDef)
pointer.down(instance, keyDef)

if (pointer.pointerType !== 'touch' && !pointer.isPrevented) {
if (pointer.pointerType !== 'touch') {
this.mouse.down(instance, keyDef, pointer)
}
}

async move(
instance: Instance,
pointerName: string,
position: PointerPosition,
instance: Instance,
pointerName: string,
position: PointerPosition,
) {
const pointer = this.pointers.get(pointerName)

// In (some?) browsers this order of events can be observed.
// This interweaving of events is probably unnecessary.
// While the order of mouse (or pointer) events is defined per spec,
// the order in which they interweave/follow on a user interaction depends on the implementation.
const pointermove = pointer.move(instance, position)
const pointermove = pointer.move(instance, position, pointer)
const mousemove =
pointer.pointerType === 'touch' || (pointer.isPrevented && pointer.isDown)
? undefined
: this.mouse.move(instance, position)
pointer.pointerType === 'touch' || (pointer.isPrevented && pointer.isDown)
? undefined
: this.mouse.move(instance, position, pointer)

pointermove?.leave()
mousemove?.leave()
Expand All @@ -133,9 +133,9 @@ export class PointerHost {
}

async release(
instance: Instance,
keyDef: pointerKey,
position: PointerPosition,
instance: Instance,
keyDef: pointerKey,
position: PointerPosition,
) {
const device = this.devices.get(keyDef.pointerType)
device.removePressed(keyDef)
Expand All @@ -158,23 +158,21 @@ export class PointerHost {
pointer.release(instance)
}

if (!pointer.isPrevented) {
if (pointer.pointerType === 'touch' && !pointer.isMultitouch) {
const mousemove = this.mouse.move(instance, pointer.position)
mousemove?.leave()
mousemove?.enter()
mousemove?.move()
if (pointer.pointerType === 'touch' && !pointer.isMultitouch) {
const mousemove = this.mouse.move(instance, position, pointer)
mousemove?.leave()
mousemove?.enter()
mousemove?.move()

this.mouse.down(instance, keyDef, pointer)
}
if (!pointer.isMultitouch) {
const mousemove = this.mouse.move(instance, pointer.position)
mousemove?.leave()
mousemove?.enter()
mousemove?.move()
this.mouse.down(instance, keyDef, pointer)
}
if (!pointer.isMultitouch) {
const mousemove = this.mouse.move(instance, position, pointer)
mousemove?.leave()
mousemove?.enter()
mousemove?.move()

this.mouse.up(instance, keyDef, pointer)
}
this.mouse.up(instance, keyDef, pointer)
}
}

Expand All @@ -184,8 +182,8 @@ export class PointerHost {

getPreviousPosition(pointerName: string) {
return this.pointers.has(pointerName)
? this.pointers.get(pointerName).position
: undefined
? this.pointers.get(pointerName).position
: undefined
}

resetClickCount() {
Expand Down
63 changes: 36 additions & 27 deletions src/system/pointer/mouse.ts
Expand Up @@ -36,14 +36,14 @@ export class Mouse {

incOnClick(button: number) {
const current =
this.down[button] === undefined
? undefined
: Number(this.down[button]) + 1
this.down[button] === undefined
? undefined
: Number(this.down[button]) + 1

this.count =
this.count[button] === undefined
? {}
: {[button]: Number(this.count[button]) + 1}
this.count[button] === undefined
? {}
: {[button]: Number(this.count[button]) + 1}

return current
}
Expand All @@ -57,16 +57,19 @@ export class Mouse {

getOnUp(button: number) {
return this.down[button] === undefined
? undefined
: Number(this.down[button]) + 1
? undefined
: Number(this.down[button]) + 1
}

reset() {
this.count = {}
}
})()

move(instance: Instance, position: PointerPosition) {
move(instance: Instance, position: PointerPosition, pointer: Pointer) {
if (pointer.isPrevented) {
return
}
const prevPosition = this.position
const prevTarget = this.getTarget(instance)

Expand Down Expand Up @@ -112,17 +115,20 @@ export class Mouse {

const target = this.getTarget(instance)
this.buttonDownTarget[button] = target
const disabled = isDisabled(target)
const init = this.getEventInit('mousedown', keyDef.button)
if (pointer.isPrevented) {
return
}
const disabled = isDisabled(target)
if (disabled || instance.dispatchUIEvent(target, 'mousedown', init)) {
this.startSelecting(instance, init.detail as number)
focusElement(target)
}
if (!disabled && getMouseEventButton(keyDef.button) === 2) {
instance.dispatchUIEvent(
target,
'contextmenu',
this.getEventInit('contextmenu', keyDef.button, pointer),
target,
'contextmenu',
this.getEventInit('contextmenu', keyDef.button, pointer),
)
}
}
Expand All @@ -135,24 +141,27 @@ export class Mouse {
}
const target = this.getTarget(instance)
if (!isDisabled(target)) {
instance.dispatchUIEvent(
target,
'mouseup',
this.getEventInit('mouseup', keyDef.button),
)
this.endSelecting()
const mouseUpInit = this.getEventInit('mouseup', keyDef.button)
if (!pointer.isPrevented) {
instance.dispatchUIEvent(
target,
'mouseup',
mouseUpInit,
)
this.endSelecting()
}

const clickTarget = getTreeDiff(
this.buttonDownTarget[button],
target,
this.buttonDownTarget[button],
target,
)[2][0] as Element | undefined
if (clickTarget) {
const init = this.getEventInit('click', keyDef.button, pointer)
if (init.detail) {
instance.dispatchUIEvent(
clickTarget,
init.button === 0 ? 'click' : 'auxclick',
init,
clickTarget,
init.button === 0 ? 'click' : 'auxclick',
init,
)
if (init.button === 0 && init.detail === 2) {
instance.dispatchUIEvent(clickTarget, 'dblclick', {
Expand All @@ -170,9 +179,9 @@ export class Mouse {
}

private getEventInit(
type: EventType,
button?: MouseButton,
pointer?: Pointer,
type: EventType,
button?: MouseButton,
pointer?: Pointer,
) {
const init: PointerEventInit = {
...this.position.coords,
Expand Down
13 changes: 7 additions & 6 deletions src/system/pointer/pointer.ts
Expand Up @@ -41,7 +41,8 @@ export class Pointer {
return this
}

move(instance: Instance, position: PointerPosition) {
// eslint-disable-next-line @typescript-eslint/no-unused-vars
move(instance: Instance, position: PointerPosition, pointer: Pointer) {
const prevPosition = this.position
const prevTarget = this.getTarget(instance)

Expand All @@ -63,7 +64,7 @@ export class Pointer {
if (prevTarget !== nextTarget) {
instance.dispatchUIEvent(prevTarget, 'pointerout', init)
leave.forEach(el =>
instance.dispatchUIEvent(el, 'pointerleave', init),
instance.dispatchUIEvent(el, 'pointerleave', init),
)
}
}
Expand All @@ -74,7 +75,7 @@ export class Pointer {
if (prevTarget !== nextTarget) {
instance.dispatchUIEvent(nextTarget, 'pointerover', init)
enter.forEach(el =>
instance.dispatchUIEvent(el, 'pointerenter', init),
instance.dispatchUIEvent(el, 'pointerenter', init),
)
}
},
Expand All @@ -94,9 +95,9 @@ export class Pointer {

this.isDown = true
this.isPrevented = !instance.dispatchUIEvent(
target,
'pointerdown',
this.getEventInit(),
target,
'pointerdown',
this.getEventInit(),
)
}

Expand Down
27 changes: 22 additions & 5 deletions tests/convenience/click.ts
Expand Up @@ -13,24 +13,41 @@ describe.each([

expect(getEvents('mouseover')).toHaveLength(1)
expect(getEvents('mousedown')).toHaveLength(clickCount)
expect(getEvents('pointerdown')).toHaveLength(clickCount)
expect(getEvents('click')).toHaveLength(clickCount)
expect(getEvents('dblclick')).toHaveLength(clickCount >= 2 ? 1 : 0)
})


test('preventDefault on pointer down prevents compatibility events', async () => {
const {element, getEvents, user} = setup(`<div></div>`, {eventHandlers: {pointerdown: e => e.preventDefault()}})

await user[method](element)

expect(getEvents('mouseover')).toHaveLength(1)
expect(getEvents('mousedown')).toHaveLength(0)
expect(getEvents('mouseup')).toHaveLength(0)
expect(getEvents('pointerdown')).toHaveLength(clickCount)
expect(getEvents('pointerup')).toHaveLength(clickCount)
expect(getEvents('click')).toHaveLength(clickCount)
expect(getEvents('dblclick')).toHaveLength(clickCount >= 2 ? 1 : 0)
})


test('throw when clicking element with pointer-events set to none', async () => {
const {element, user} = setup(`<div style="pointer-events: none"></div>`)

await expect(user[method](element)).rejects.toThrowError(
/has `pointer-events: none`/i,
/has `pointer-events: none`/i,
)
})

test('skip check for pointer-events', async () => {
const {element, getEvents, user} = setup(
`<div style="pointer-events: none"></div>`,
{
pointerEventsCheck: PointerEventsCheckLevel.Never,
},
`<div style="pointer-events: none"></div>`,
{
pointerEventsCheck: PointerEventsCheckLevel.Never,
},
)

await user[method](element)
Expand Down