Skip to content

fix not sending click event to proper element #5580

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.

Sign up for GitHub

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

Merged
brian-mann merged 5 commits into from
Nov 6, 2019
+188 −91
Merged

fix not sending click event to proper element #5580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
069e398
fix not sending click event
kuceb Nov 1, 2019
4275e06
add issue comment
kuceb Nov 1, 2019
6c8ae5e
slightly less messy code
kuceb Nov 1, 2019
bbefd46
Merge remote-tracking branch 'origin/develop' into fix-mouseup-moused…
kuceb Nov 5, 2019
ad52adb
refactor, use term "phase"
kuceb Nov 5, 2019
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
14 changes: 7 additions & 7 deletions packages/driver/src/cy/commands/actions/click.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const formatMouseEvents = (events) => {
const reason = val.skipped

return {
'Event Name': key.slice(0, -5),
'Event Name': key,
'Target Element': reason,
'Prevented Default?': null,
'Stopped Propagation?': null,
Expand All @@ -52,7 +52,7 @@ const formatMouseEvents = (events) => {
}

return {
'Event Name': key.slice(0, -5),
'Event Name': key,
'Target Element': val.el,
'Prevented Default?': val.preventedDefault,
'Stopped Propagation?': val.stoppedPropagation,
Expand Down Expand Up @@ -265,10 +265,10 @@ module.exports = (Commands, Cypress, cy, state, config) => {
defaultOptions: { multiple: true },
positionOrX,
onReady (fromElViewport, forceEl) {
const { clickEvents1, clickEvents2, dblclickProps } = mouse.dblclick(fromElViewport, forceEl)
const { clickEvents1, clickEvents2, dblclick } = mouse.dblclick(fromElViewport, forceEl)

return {
dblclickProps,
dblclick,
clickEvents: [clickEvents1, clickEvents2],
}
},
Expand All @@ -281,16 +281,16 @@ module.exports = (Commands, Cypress, cy, state, config) => {
return {
name: 'Mouse Click Events',
data: _.concat(
formatMouseEvents(domEvents.clickEvents[0], formatMouseEvents),
formatMouseEvents(domEvents.clickEvents[1], formatMouseEvents)
formatMouseEvents(domEvents.clickEvents[0]),
formatMouseEvents(domEvents.clickEvents[1])
),
}
},
3: () => {
return {
name: 'Mouse Double Click Event',
data: formatMouseEvents({
dblclickProps: domEvents.dblclickProps,
dblclick: domEvents.dblclick,
}),
}
},
Expand Down
110 changes: 60 additions & 50 deletions packages/driver/src/cy/mouse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,12 +310,12 @@ const create = (state, keyboard, focused) => {
}, mouseEvtOptionsExtend)

// TODO: pointer events should have fractional coordinates, not rounded
let pointerdownProps = sendPointerdown(
let pointerdown = sendPointerdown(
el,
pointerEvtOptions
)

const pointerdownPrevented = pointerdownProps.preventedDefault
const pointerdownPrevented = pointerdown.preventedDefault
const elIsDetached = $elements.isDetachedEl(el)

if (pointerdownPrevented || elIsDetached) {
Expand All @@ -326,31 +326,37 @@ const create = (state, keyboard, focused) => {
}

return {
pointerdownProps,
mousedownProps: {
skipped: formatReasonNotFired(reason),
targetEl: el,
events: {
pointerdown,
mousedown: {
skipped: formatReasonNotFired(reason),
},
},
}
}

let mousedownProps = sendMousedown(el, mouseEvtOptions)
let mousedown = sendMousedown(el, mouseEvtOptions)

return {
pointerdownProps,
mousedownProps,
targetEl: el,
events: {
pointerdown,
mousedown,
},
}
},

down (fromElViewport, forceEl, pointerEvtOptionsExtend = {}, mouseEvtOptionsExtend = {}) {
const $previouslyFocused = focused.getFocused()

const mouseDownEvents = mouse._downEvents(fromElViewport, forceEl, pointerEvtOptionsExtend, mouseEvtOptionsExtend)
const mouseDownPhase = mouse._downEvents(fromElViewport, forceEl, pointerEvtOptionsExtend, mouseEvtOptionsExtend)

// el we just send pointerdown
const el = mouseDownEvents.pointerdownProps.el
const el = mouseDownPhase.targetEl

if (mouseDownEvents.pointerdownProps.preventedDefault || mouseDownEvents.mousedownProps.preventedDefault || !$elements.isAttachedEl(el)) {
return mouseDownEvents
if (mouseDownPhase.events.pointerdown.preventedDefault || mouseDownPhase.events.mousedown.preventedDefault || !$elements.isAttachedEl(el)) {
return mouseDownPhase
}

//# retrieve the first focusable $el in our parent chain
Expand All @@ -377,7 +383,7 @@ const create = (state, keyboard, focused) => {
$selection.moveSelectionToEnd($dom.getDocumentFromElement($elToFocus[0]), { onlyIfEmptySelection: true })
}

return mouseDownEvents
return mouseDownPhase
},

/**
Expand Down Expand Up @@ -410,42 +416,41 @@ const create = (state, keyboard, focused) => {
* el2 = moveToCoordsOrNoop(coords)
* sendMouseup(el2)
* el3 = moveToCoordsOrNoop(coords)
* if (notDetached(el1) && el1 === el2)
* sendClick(el3)
* if (notDetached(el1))
* sendClick(ancestorOf(el1, el2))
*/
click (fromElViewport, forceEl, pointerEvtOptionsExtend = {}, mouseEvtOptionsExtend = {}) {
debug('mouse.click', { fromElViewport, forceEl })

const mouseDownEvents = mouse.down(fromElViewport, forceEl, pointerEvtOptionsExtend, mouseEvtOptionsExtend)
const mouseDownPhase = mouse.down(fromElViewport, forceEl, pointerEvtOptionsExtend, mouseEvtOptionsExtend)

const skipMouseupEvent = mouseDownEvents.pointerdownProps.skipped || mouseDownEvents.pointerdownProps.preventedDefault
const skipMouseupEvent = mouseDownPhase.events.pointerdown.skipped || mouseDownPhase.events.pointerdown.preventedDefault

const mouseUpEvents = mouse.up(fromElViewport, forceEl, skipMouseupEvent, pointerEvtOptionsExtend, mouseEvtOptionsExtend)
const mouseUpPhase = mouse.up(fromElViewport, forceEl, skipMouseupEvent, pointerEvtOptionsExtend, mouseEvtOptionsExtend)

// Only send click event if the same element received both pointerdown and pointerup, and it's not detached.
const getSkipClickEventAndReason = () => {
const getElementToClick = () => {
// Never skip the click event when force:true
if (forceEl) {
return false
return { elToClick: forceEl }
}

if ($elements.isDetachedEl(mouseDownEvents.pointerdownProps.el)) {
return 'element was detached'
// Only send click event if mousedown element is not detached.
if ($elements.isDetachedEl(mouseDownPhase.targetEl)) {
return { skipClickEventReason: 'element was detached' }
}

if (!mouseUpEvents.pointerupProps.el || mouseDownEvents.pointerdownProps.el !== mouseUpEvents.pointerupProps.el) {
return 'mouseup and mousedown not received by same element'
}
const commonAncestor = mouseUpPhase.targetEl &&
mouseDownPhase.targetEl &&
$elements.getFirstCommonAncestor(mouseUpPhase.targetEl, mouseDownPhase.targetEl)

// No reason to skip the click event
return false
return { elToClick: commonAncestor }
}

const skipClickEvent = getSkipClickEventAndReason()
const { skipClickEventReason, elToClick } = getElementToClick()

const mouseClickEvents = mouse._mouseClickEvents(fromElViewport, mouseDownEvents.pointerdownProps.el, forceEl, skipClickEvent, mouseEvtOptionsExtend)
const mouseClickEvents = mouse._mouseClickEvents(fromElViewport, elToClick, forceEl, skipClickEventReason, mouseEvtOptionsExtend)

return _.extend({}, mouseDownEvents, mouseUpEvents, mouseClickEvents)
return _.extend({}, mouseDownPhase.events, mouseUpPhase.events, mouseClickEvents)
},

/**
Expand All @@ -471,29 +476,35 @@ const create = (state, keyboard, focused) => {

const el = forceEl || mouse.moveToCoords(fromElViewport)

let pointerupProps = sendPointerup(el, pointerEvtOptions)
let pointerup = sendPointerup(el, pointerEvtOptions)

if (skipMouseEvent || $elements.isDetachedEl($(el))) {
return {
pointerupProps,
mouseupProps: {
skipped: formatReasonNotFired('Previous event cancelled'),
targetEl: el,
events: {
pointerup,
mouseup: {
skipped: formatReasonNotFired('Previous event cancelled'),
},
},
}
}

let mouseupProps = sendMouseup(el, mouseEvtOptions)
let mouseup = sendMouseup(el, mouseEvtOptions)

return {
pointerupProps,
mouseupProps,
targetEl: el,
events: {
pointerup,
mouseup,
},
}
},

_mouseClickEvents (fromElViewport, el, forceEl, skipClickEvent, mouseEvtOptionsExtend = {}) {
if (skipClickEvent) {
return {
clickProps: {
click: {
skipped: formatReasonNotFired(skipClickEvent),
},
}
Expand All @@ -512,9 +523,9 @@ const create = (state, keyboard, focused) => {
detail: 1,
}, mouseEvtOptionsExtend)

let clickProps = sendClick(el, clickEventOptions)
let click = sendClick(el, clickEventOptions)

return { clickProps }
return { click }
},

_contextmenuEvent (fromElViewport, forceEl, mouseEvtOptionsExtend) {
Expand All @@ -530,9 +541,9 @@ const create = (state, keyboard, focused) => {
which: 3,
}, mouseEvtOptionsExtend)

let contextmenuProps = sendContextmenu(el, mouseEvtOptions)
let contextmenu = sendContextmenu(el, mouseEvtOptions)

return { contextmenuProps }
return { contextmenu }
},

dblclick (fromElViewport, forceEl, mouseEvtOptionsExtend = {}) {
Expand All @@ -553,9 +564,9 @@ const create = (state, keyboard, focused) => {
detail: 2,
}, mouseEvtOptionsExtend)

let dblclickProps = sendDblclick(el, dblclickEvtProps)
let dblclick = sendDblclick(el, dblclickEvtProps)

return { clickEvents1, clickEvents2, dblclickProps }
return { clickEvents1, clickEvents2, dblclick }
},

rightclick (fromElViewport, forceEl) {
Expand All @@ -570,15 +581,14 @@ const create = (state, keyboard, focused) => {
which: 3,
}

const mouseDownEvents = mouse.down(fromElViewport, forceEl, pointerEvtOptionsExtend, mouseEvtOptionsExtend)
const mouseDownPhase = mouse.down(fromElViewport, forceEl, pointerEvtOptionsExtend, mouseEvtOptionsExtend)

const contextmenuEvent = mouse._contextmenuEvent(fromElViewport, forceEl)

const skipMouseupEvent = mouseDownEvents.pointerdownProps.skipped || mouseDownEvents.pointerdownProps.preventedDefault

const mouseUpEvents = mouse.up(fromElViewport, forceEl, skipMouseupEvent, pointerEvtOptionsExtend, mouseEvtOptionsExtend)
const skipMouseupEvent = mouseDownPhase.events.pointerdown.skipped || mouseDownPhase.events.pointerdown.preventedDefault
const mouseUpPhase = mouse.up(fromElViewport, forceEl, skipMouseupEvent, pointerEvtOptionsExtend, mouseEvtOptionsExtend)

const clickEvents = _.extend({}, mouseDownEvents, mouseUpEvents)
const clickEvents = _.extend({}, mouseDownPhase.events, mouseUpPhase.events)

return _.extend({}, { clickEvents, contextmenuEvent })
},
Expand Down
Loading