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: make EventHandler better handle mouseenter/mouseleave events #33310

Merged
merged 3 commits into from Apr 13, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 28 additions & 10 deletions js/src/dom/event-handler.js
Expand Up @@ -22,6 +22,7 @@ const customEvents = {
mouseenter: 'mouseover',
mouseleave: 'mouseout'
}
const customEventsRegex = /^(mouseenter|mouseleave)/i
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

childish comment: better leave it as string inline, instead of use a var. Is very specific (either way is ok)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't matter, though, since the minifier should already inline it :)

const nativeEvents = new Set([
'click',
'dblclick',
Expand Down Expand Up @@ -113,7 +114,7 @@ function bootstrapDelegationHandler(element, selector, fn) {

if (handler.oneOff) {
// eslint-disable-next-line unicorn/consistent-destructuring
EventHandler.off(element, event.type, fn)
EventHandler.off(element, event.type, selector, fn)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers, this fixes a bug with the one time delegated event handlers. Added a test to the specs too.

}

return fn.apply(target, [event])
Expand Down Expand Up @@ -144,14 +145,7 @@ function normalizeParams(originalTypeEvent, handler, delegationFn) {
const delegation = typeof handler === 'string'
const originalHandler = delegation ? delegationFn : handler

// allow to get the native events from namespaced events ('click.bs.button' --> 'click')
let typeEvent = originalTypeEvent.replace(stripNameRegex, '')
const custom = customEvents[typeEvent]

if (custom) {
typeEvent = custom
}

let typeEvent = getTypeEvent(originalTypeEvent)
const isNative = nativeEvents.has(typeEvent)

if (!isNative) {
Expand All @@ -171,6 +165,24 @@ function addHandler(element, originalTypeEvent, handler, delegationFn, oneOff) {
delegationFn = null
}

// in case of mouseenter or mouseleave wrap the handler within a function that checks for its DOM position
// this prevents the handler from being dispatched the same way as mouseover or mouseout does
if (customEventsRegex.test(originalTypeEvent)) {
const wrapFn = fn => {
return function (event) {
if (!event.relatedTarget || (event.relatedTarget !== event.delegateTarget && event.relatedTarget.contains(event.delegateTarget))) {
return fn.call(this, event)
}
}
}

if (delegationFn) {
delegationFn = wrapFn(delegationFn)
} else {
handler = wrapFn(handler)
}
}

const [delegation, originalHandler, typeEvent] = normalizeParams(originalTypeEvent, handler, delegationFn)
const events = getEvent(element)
const handlers = events[typeEvent] || (events[typeEvent] = {})
Expand Down Expand Up @@ -219,6 +231,12 @@ function removeNamespacedHandlers(element, events, typeEvent, namespace) {
})
}

function getTypeEvent(event) {
// allow to get the native events from namespaced events ('click.bs.button' --> 'click')
event = event.replace(stripNameRegex, '')
return customEvents[event] || event
}

const EventHandler = {
on(element, event, handler, delegationFn) {
addHandler(element, event, handler, delegationFn, false)
Expand Down Expand Up @@ -272,7 +290,7 @@ const EventHandler = {
}

const $ = getjQuery()
const typeEvent = event.replace(stripNameRegex, '')
const typeEvent = getTypeEvent(event)
const inNamespace = event !== typeEvent
const isNative = nativeEvents.has(typeEvent)

Expand Down
78 changes: 77 additions & 1 deletion js/tests/unit/dom/event-handler.spec.js
Expand Up @@ -77,10 +77,64 @@ describe('EventHandler', () => {

div.click()
})

it('should handle mouseenter/mouseleave like the native counterpart', done => {
fixtureEl.innerHTML = [
'<div class="outer">',
'<div class="inner">',
'<div class="nested">',
'<div class="deep"></div>',
'</div>',
'</div>',
'</div>'
]

const outer = fixtureEl.querySelector('.outer')
const inner = fixtureEl.querySelector('.inner')
const nested = fixtureEl.querySelector('.nested')
const deep = fixtureEl.querySelector('.deep')

const enterSpy = jasmine.createSpy('mouseenter')
const leaveSpy = jasmine.createSpy('mouseleave')
const delegateEnterSpy = jasmine.createSpy('mouseenter')
const delegateLeaveSpy = jasmine.createSpy('mouseleave')

EventHandler.on(inner, 'mouseenter', enterSpy)
EventHandler.on(inner, 'mouseleave', leaveSpy)
EventHandler.on(outer, 'mouseenter', '.inner', delegateEnterSpy)
EventHandler.on(outer, 'mouseleave', '.inner', delegateLeaveSpy)

const moveMouse = (from, to) => {
from.dispatchEvent(new MouseEvent('mouseout', {
bubbles: true,
relatedTarget: to
}))

to.dispatchEvent(new MouseEvent('mouseover', {
bubbles: true,
relatedTarget: from
}))
}

moveMouse(outer, inner)
moveMouse(inner, nested)
moveMouse(nested, deep)
moveMouse(deep, nested)
moveMouse(nested, inner)
moveMouse(inner, outer)

setTimeout(() => {
expect(enterSpy.calls.count()).toBe(1)
expect(leaveSpy.calls.count()).toBe(1)
expect(delegateEnterSpy.calls.count()).toBe(1)
expect(delegateLeaveSpy.calls.count()).toBe(1)
done()
}, 20)
})
})

describe('one', () => {
it('should call listener just one', done => {
it('should call listener just once', done => {
fixtureEl.innerHTML = '<div></div>'

let called = 0
Expand All @@ -101,6 +155,28 @@ describe('EventHandler', () => {
done()
}, 20)
})

it('should call delegated listener just once', done => {
fixtureEl.innerHTML = '<div></div>'

let called = 0
const div = fixtureEl.querySelector('div')
const obj = {
oneListener() {
called++
}
}

EventHandler.one(fixtureEl, 'bootstrap', 'div', obj.oneListener)

EventHandler.trigger(div, 'bootstrap')
EventHandler.trigger(div, 'bootstrap')

setTimeout(() => {
expect(called).toEqual(1)
done()
}, 20)
})
})

describe('off', () => {
Expand Down