Skip to content

Commit

Permalink
fix(Portal): replace mouseover event listener with mouseenter
Browse files Browse the repository at this point in the history
  • Loading branch information
David Zukowski committed Dec 20, 2016
1 parent 45c2e55 commit e017826
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 40 deletions.
2 changes: 1 addition & 1 deletion docs/app/Components/IconSearch/IconSearch.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export default class IconSearch extends Component {
renderIconColumn = (name) => (
<Popup
key={name}
mouseOverDelay={500}
mouseEnterDelay={500}
inverted
hoverable={false}
size='mini'
Expand Down
32 changes: 16 additions & 16 deletions src/addons/Portal/Portal.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class Portal extends Component {
mouseLeaveDelay: PropTypes.number,

/** Milliseconds to wait before opening on mouse over */
mouseOverDelay: PropTypes.number,
mouseEnterDelay: PropTypes.number,

/**
* Called when a close event happens
Expand Down Expand Up @@ -120,7 +120,7 @@ class Portal extends Component {
openOnTriggerFocus: PropTypes.bool,

/** Controls whether or not the portal should open when mousing over the trigger. */
openOnTriggerMouseOver: PropTypes.bool,
openOnTriggerMouseEnter: PropTypes.bool,

/** Controls whether the portal should be prepended to the mountNode instead of appended. */
prepend: PropTypes.bool,
Expand Down Expand Up @@ -166,7 +166,7 @@ class Portal extends Component {
this.unmountPortal()

// Clean up timers
clearTimeout(this.mouseOverTimer)
clearTimeout(this.mouseEnterTimer)
clearTimeout(this.mouseLeaveTimer)
}

Expand Down Expand Up @@ -211,14 +211,14 @@ class Portal extends Component {
this.mouseLeaveTimer = this.closeWithTimeout(e, mouseLeaveDelay)
}

handlePortalMouseOver = (e) => {
handlePortalMouseEnter = (e) => {
// In order to enable mousing from the trigger to the portal, we need to
// clear the mouseleave timer that was set when leaving the trigger.
const { closeOnPortalMouseLeave } = this.props

if (!closeOnPortalMouseLeave) return

debug('handlePortalMouseOver()')
debug('handlePortalMouseEnter()')
clearTimeout(this.mouseLeaveTimer)
}

Expand Down Expand Up @@ -272,7 +272,7 @@ class Portal extends Component {
}

handleTriggerMouseLeave = (e) => {
clearTimeout(this.mouseOverTimer)
clearTimeout(this.mouseEnterTimer)

const { trigger, closeOnTriggerMouseLeave, mouseLeaveDelay } = this.props

Expand All @@ -285,18 +285,18 @@ class Portal extends Component {
this.mouseLeaveTimer = this.closeWithTimeout(e, mouseLeaveDelay)
}

handleTriggerMouseOver = (e) => {
handleTriggerMouseEnter = (e) => {
clearTimeout(this.mouseLeaveTimer)

const { trigger, mouseOverDelay, openOnTriggerMouseOver } = this.props
const { trigger, mouseEnterDelay, openOnTriggerMouseEnter } = this.props

// Call original event handler
_.invoke(trigger, 'props.onMouseOver', e)
_.invoke(trigger, 'props.onMouseEnter', this.handleTriggerMouseEnter)

if (!openOnTriggerMouseOver) return
if (!openOnTriggerMouseEnter) return

debug('handleTriggerMouseOver()')
this.mouseOverTimer = this.openWithTimeout(e, mouseOverDelay)
debug('handleTriggerMouseEnter()')
this.mouseEnterTimer = this.openWithTimeout(e, mouseEnterDelay)
}

// ----------------------------------------
Expand Down Expand Up @@ -353,7 +353,7 @@ class Portal extends Component {
// when re-rendering, first remove listeners before re-adding them to the new node
if (this.portal) {
this.portal.removeEventListener('mouseleave', this.handlePortalMouseLeave)
this.portal.removeEventListener('mouseover', this.handlePortalMouseOver)
this.portal.removeEventListener('mouseenter', this.handlePortalMouseEnter)
}

ReactDOM.unstable_renderSubtreeIntoContainer(
Expand All @@ -365,7 +365,7 @@ class Portal extends Component {
this.portal = this.node.firstElementChild

this.portal.addEventListener('mouseleave', this.handlePortalMouseLeave)
this.portal.addEventListener('mouseover', this.handlePortalMouseOver)
this.portal.addEventListener('mouseenter', this.handlePortalMouseEnter)
}

mountPortal = () => {
Expand Down Expand Up @@ -399,7 +399,7 @@ class Portal extends Component {
this.node.parentNode.removeChild(this.node)

this.portal.removeEventListener('mouseleave', this.handlePortalMouseLeave)
this.portal.removeEventListener('mouseover', this.handlePortalMouseOver)
this.portal.removeEventListener('mouseenter', this.handlePortalMouseEnter)

this.node = null
this.portal = null
Expand All @@ -421,7 +421,7 @@ class Portal extends Component {
onClick: this.handleTriggerClick,
onFocus: this.handleTriggerFocus,
onMouseLeave: this.handleTriggerMouseLeave,
onMouseOver: this.handleTriggerMouseOver,
onMouseEnter: this.handleTriggerMouseEnter,
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/modules/Popup/Popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,11 @@ export default class Popup extends Component {
portalProps.openOnTriggerFocus = true
portalProps.closeOnTriggerBlur = true
} else if (on === 'hover') {
portalProps.openOnTriggerMouseOver = true
portalProps.openOnTriggerMouseEnter = true
portalProps.closeOnTriggerMouseLeave = true
// Taken from SUI: https://git.io/vPmCm
portalProps.mouseLeaveDelay = 70
portalProps.mouseOverDelay = 50
portalProps.mouseEnterDelay = 50
}

return portalProps
Expand Down
40 changes: 20 additions & 20 deletions test/specs/addons/Portal/Portal-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -271,43 +271,43 @@ describe('Portal', () => {
})
})

describe('openOnTriggerMouseOver', () => {
it('should not open portal on mouseover when not set', (done) => {
describe('openOnTriggerMouseEnter', () => {
it('should not open portal on mouseenter when not set', (done) => {
const spy = sandbox.spy()
const trigger = <button onMouseOver={spy}>button</button>
const mouseOverDelay = 100
wrapperMount(<Portal trigger={trigger} mouseOverDelay={mouseOverDelay}><p>Hi</p></Portal>)
const trigger = <button onMouseEnter={spy}>button</button>
const mouseEnterDelay = 100
wrapperMount(<Portal trigger={trigger} mouseEnterDelay={mouseEnterDelay}><p>Hi</p></Portal>)

wrapper.find('button').simulate('mouseover')
wrapper.find('button').simulate('mouseenter')
document.body.childElementCount.should.equal(0)
spy.should.have.been.calledOnce()

setTimeout(() => {
document.body.childElementCount.should.equal(0)
spy.should.have.been.calledOnce()
done()
}, mouseOverDelay + 1)
}, mouseEnterDelay + 1)
})

it('should open portal on mouseover when set', (done) => {
it('should open portal on mouseenter when set', (done) => {
const spy = sandbox.spy()
const trigger = <button onMouseOver={spy}>button</button>
const mouseOverDelay = 100
const trigger = <button onMouseEnter={spy}>button</button>
const mouseEnterDelay = 100
wrapperMount(
<Portal trigger={trigger} openOnTriggerMouseOver mouseOverDelay={mouseOverDelay}><p>Hi</p></Portal>
<Portal trigger={trigger} openOnTriggerMouseEnter mouseEnterDelay={mouseEnterDelay}><p>Hi</p></Portal>
)

wrapper.find('button').simulate('mouseover')
wrapper.find('button').simulate('mouseenter')
setTimeout(() => {
document.body.childElementCount.should.equal(0)
spy.should.have.been.calledOnce()
}, mouseOverDelay - 1)
}, mouseEnterDelay - 1)

setTimeout(() => {
document.body.lastElementChild.should.equal(wrapper.instance().node)
spy.should.have.been.calledOnce()
done()
}, mouseOverDelay + 1)
}, mouseEnterDelay + 1)
})
})

Expand Down Expand Up @@ -383,7 +383,7 @@ describe('Portal', () => {
})

describe('closeOnTriggerMouseLeave + closeOnPortalMouseLeave', () => {
it('should close portal on trigger mouseleave even when portal receives mouseover within limit', (done) => {
it('should close portal on trigger mouseleave even when portal receives mouseenter within limit', (done) => {
const trigger = <button>button</button>
const delay = 100
wrapperMount(
Expand All @@ -392,9 +392,9 @@ describe('Portal', () => {

wrapper.find('button').simulate('mouseleave')

// Fire a mouseOver on the portal within the time limit
// Fire a mouseEnter on the portal within the time limit
setTimeout(() => {
domEvent.mouseOver(wrapper.instance().node.firstElementChild)
domEvent.mouseEnter(wrapper.instance().node.firstElementChild)
}, delay - 1)

// The portal should close because closeOnPortalMouseLeave not set
Expand All @@ -404,7 +404,7 @@ describe('Portal', () => {
}, delay + 1)
})

it('should not close portal on trigger mouseleave when portal receives mouseover within limit', (done) => {
it('should not close portal on trigger mouseleave when portal receives mouseenter within limit', (done) => {
const trigger = <button>button</button>
const delay = 100
wrapperMount(
Expand All @@ -415,9 +415,9 @@ describe('Portal', () => {

wrapper.find('button').simulate('mouseleave')

// Fire a mouseOver on the portal within the time limit
// Fire a mouseEnter on the portal within the time limit
setTimeout(() => {
domEvent.mouseOver(wrapper.instance().node.firstElementChild)
domEvent.mouseEnter(wrapper.instance().node.firstElementChild)
}, delay - 1)

// The portal should not have closed
Expand Down
2 changes: 1 addition & 1 deletion test/specs/modules/Popup/Popup-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ describe('Popup', () => {
const trigger = <button>foo</button>
wrapperMount(<Popup content='foo' trigger={trigger} />)

wrapper.find('button').simulate('mouseover', nativeEvent)
wrapper.find('button').simulate('mouseenter', nativeEvent)
setTimeout(() => {
assertInBody('.ui.popup.visible')
done()
Expand Down
9 changes: 9 additions & 0 deletions test/utils/domEvent.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ export const fire = (node, eventType, data = {}) => {
*/
export const keyDown = (node, data) => fire(node, 'keydown', data)

/**
* Dispatch a 'mouseenter' event on a DOM node.
* @param {String|Object} node A querySelector string or DOM node.
* @param {Object} [data] Additional event data.
* @returns {Object} The event
*/
export const mouseEnter = (node, data) => fire(node, 'mouseenter', data)

/**
* Dispatch a 'mouseleave' event on a DOM node.
* @param {String|Object} node A querySelector string or DOM node.
Expand Down Expand Up @@ -60,6 +68,7 @@ export const click = (node, data) => fire(node, 'click', data)

export default {
fire,
mouseEnter,
mouseLeave,
mouseOver,
mouseUp,
Expand Down

0 comments on commit e017826

Please sign in to comment.