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.

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

feat(Portal): More flexible configuration #590

Merged
merged 1 commit into from
Oct 3, 2016
Merged

feat(Portal): More flexible configuration #590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
102 changes: 86 additions & 16 deletions src/addons/Portal/Portal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,19 @@ class Portal extends Component {
/** Controls whether or not the portal should close when escape is pressed is displayed. */
closeOnEscape: PropTypes.bool,

/**
* Controls whether or not the portal should close when mousing out of the portal.
* NOTE: This will prevent `closeOnTriggerMouseLeave` when mousing over the
* gap from the trigger to the portal.
*/
closeOnPortalMouseLeave: PropTypes.bool,

/** Controls whether or not the portal should close on blur of the trigger. */
closeOnTriggerBlur: PropTypes.bool,

/** Controls whether or not the portal should close on click of the trigger. */
closeOnTriggerClick: PropTypes.bool,

/** Controls whether or not the portal should close when mousing out of the trigger. */
closeOnTriggerMouseLeave: PropTypes.bool,

Expand All @@ -46,6 +56,12 @@ class Portal extends Component {
/** The node where the portal should mount.. */
mountNode: PropTypes.any,

/** Milliseconds to wait before closing on mouse leave */
mouseLeaveDelay: PropTypes.number,

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

/** Called when a close event happens */
onClose: PropTypes.func,

Expand Down Expand Up @@ -110,6 +126,10 @@ class Portal extends Component {

componentWillUnmount() {
this.unmountPortal()

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

// ----------------------------------------
Expand All @@ -118,6 +138,8 @@ class Portal extends Component {

closeOnDocumentClick = (e) => {
if (!this.props.closeOnDocumentClick) return

// If event happened in the portal, ignore it
if (this.portal.contains(e.target)) return

debug('closeOnDocumentClick()')
Expand All @@ -140,6 +162,26 @@ class Portal extends Component {
// Component Event Handlers
// ----------------------------------------

handlePortalMouseLeave = (e) => {
const { closeOnPortalMouseLeave, mouseLeaveDelay } = this.props

if (!closeOnPortalMouseLeave) return

debug('handlePortalMouseLeave()')
this.mouseLeaveTimer = this.closeWithTimeout(e, mouseLeaveDelay)
}

handlePortalMouseOver = (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()')
clearTimeout(this.mouseLeaveTimer)
}

handleTriggerBlur = (e) => {
const { trigger, closeOnTriggerBlur } = this.props

Expand All @@ -153,22 +195,24 @@ class Portal extends Component {
}

handleTriggerClick = (e) => {
const { trigger, openOnTriggerClick } = this.props
const { trigger, closeOnTriggerClick, openOnTriggerClick } = this.props
const { open } = this.state

// Call original event handler
_.invoke(trigger, 'props.onClick', e)

if (!openOnTriggerClick) return

debug('handleTriggerClick()')

e.stopPropagation()

// Prevents closeOnDocumentClick from closing the portal when
// openOnTriggerFocus is set. Focus shifts on mousedown so the portal opens
// before the click finishes so it may actually wind up on the document.
e.nativeEvent.stopImmediatePropagation()
this.open(e)
if (open && closeOnTriggerClick) {
e.stopPropagation()
this.close(e)
} else if (!open && openOnTriggerClick) {
// Prevents closeOnDocumentClick from closing the portal when
// openOnTriggerFocus is set. Focus shifts on mousedown so the portal opens
// before the click finishes so it may actually wind up on the document.
e.nativeEvent.stopImmediatePropagation()

e.stopPropagation()
this.open(e)
}
}

handleTriggerFocus = (e) => {
Expand All @@ -184,27 +228,31 @@ class Portal extends Component {
}

handleTriggerMouseLeave = (e) => {
const { trigger, closeOnTriggerMouseLeave } = this.props
clearTimeout(this.mouseOverTimer)

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

// Call original event handler
_.invoke(trigger, 'props.onMouseLeave', e)

if (!closeOnTriggerMouseLeave) return

debug('handleTriggerMouseLeave()')
this.close(e)
this.mouseLeaveTimer = this.closeWithTimeout(e, mouseLeaveDelay)
}

handleTriggerMouseOver = (e) => {
const { trigger, openOnTriggerMouseOver } = this.props
clearTimeout(this.mouseLeaveTimer)

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

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

if (!openOnTriggerMouseOver) return

debug('handleTriggerMouseOver()')
this.open(e)
this.mouseOverTimer = this.openWithTimeout(e, mouseOverDelay)
}

// ----------------------------------------
Expand All @@ -220,6 +268,14 @@ class Portal extends Component {
this.trySetState({ open: true })
}

openWithTimeout = (e, delay) => {
// React wipes the entire event object and suggests using e.persist() if
// you need the event for async access. However, even with e.persist
// certain required props (e.g. currentTarget) are null so we're forced to clone.
const eventClone = { ...e }
return setTimeout(() => this.open(eventClone), delay || 0)
}

close = (e) => {
debug('close()')

Expand All @@ -229,6 +285,14 @@ class Portal extends Component {
this.trySetState({ open: false })
}

closeWithTimeout = (e, delay) => {
// React wipes the entire event object and suggests using e.persist() if
// you need the event for async access. However, even with e.persist
// certain required props (e.g. currentTarget) are null so we're forced to clone.
const eventClone = { ...e }
Copy link
Member

@levithomason levithomason Oct 3, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Heads up, React wipes the entire event object, nullifying all values. That's because React reuses a single event object. In order to persist the event object for async access, use e.persist():

https://facebook.github.io/react/docs/events.html#event-pooling

I think what is here will technically work, but it still seems as though we should be using e.persist(), even if we're copying the persisted object.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tried e.persist initially but doesn't preserve the currentTarget, see facebook/react#2857. This is kinda gross but it's the only way to make a copy of the event as-is.

Copy link
Member

Choose a reason for hiding this comment

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

Womp womp, thanks for the link

return setTimeout(() => this.close(eventClone), delay || 0)
}

renderPortal() {
const { children, className } = this.props

Expand All @@ -241,6 +305,9 @@ class Portal extends Component {
Children.only(children),
this.node
)

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

mountPortal = () => {
Expand All @@ -264,6 +331,9 @@ class Portal extends Component {
ReactDOM.unmountComponentAtNode(this.node)
this.node.parentNode.removeChild(this.node)

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

this.node = null
this.portal = null

Expand Down
163 changes: 149 additions & 14 deletions test/specs/addons/Portal/Portal-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,47 +213,182 @@ describe('Portal', () => {
})
})

describe('closeOnTriggerClick', () => {
it('should not close portal on click', () => {
const spy = sandbox.spy()
const trigger = <button onClick={spy}>button</button>
wrapperMount(<Portal trigger={trigger} defaultOpen><p>Hi</p></Portal>)

wrapper.find('button').simulate('click', nativeEvent)
document.body.lastElementChild.should.equal(wrapper.instance().node)
spy.should.have.been.calledOnce()
})

it('should close portal on click when set', () => {
const spy = sandbox.spy()
const trigger = <button onClick={spy}>button</button>
wrapperMount(<Portal trigger={trigger} defaultOpen closeOnTriggerClick><p>Hi</p></Portal>)

wrapper.find('button').simulate('click', nativeEvent)
document.body.childElementCount.should.equal(0)
spy.should.have.been.calledOnce()
})
})

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

wrapper.find('button').simulate('mouseover')
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)
})

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

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

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

describe('closeOnTriggerMouseLeave', () => {
it('should not close portal on mouseleave when not set', () => {
it('should not close portal on mouseleave when not set', (done) => {
const spy = sandbox.spy()
const trigger = <button onMouseLeave={spy}>button</button>
wrapperMount(<Portal trigger={trigger} defaultOpen><p>Hi</p></Portal>)
const delay = 100
wrapperMount(<Portal trigger={trigger} defaultOpen mouseLeaveDelay={delay}><p>Hi</p></Portal>)

wrapper.find('button').simulate('mouseleave')
document.body.lastElementChild.should.equal(wrapper.instance().node)
spy.should.have.been.calledOnce()
setTimeout(() => {
document.body.lastElementChild.should.equal(wrapper.instance().node)
spy.should.have.been.calledOnce()
done()
}, delay + 1)
})

it('should close portal on mouseleave when set', () => {
it('should close portal on mouseleave when set', (done) => {
const spy = sandbox.spy()
const trigger = <button onMouseLeave={spy}>button</button>
wrapperMount(<Portal trigger={trigger} defaultOpen closeOnTriggerMouseLeave><p>Hi</p></Portal>)
const delay = 100
wrapperMount(
<Portal trigger={trigger} defaultOpen closeOnTriggerMouseLeave mouseLeaveDelay={delay}><p>Hi</p></Portal>
)

wrapper.find('button').simulate('mouseleave')
document.body.childElementCount.should.equal(0)
spy.should.have.been.calledOnce()
setTimeout(() => {
document.body.lastElementChild.should.equal(wrapper.instance().node)
spy.should.have.been.calledOnce()
}, delay - 1)

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

describe('closeOnPortalMouseLeave', () => {
it('should not close portal on mouseleave of portal when not set', (done) => {
const trigger = <button>button</button>
const delay = 100
wrapperMount(<Portal trigger={trigger} defaultOpen mouseLeaveDelay={delay}><p>Hi</p></Portal>)

domEvent.mouseLeave(wrapper.instance().node.firstElementChild)

setTimeout(() => {
document.body.lastElementChild.should.equal(wrapper.instance().node)
done()
}, delay + 1)
})

it('should close portal on mouseleave of portal when set', (done) => {
const trigger = <button>button</button>
const delay = 100
wrapperMount(
<Portal trigger={trigger} defaultOpen closeOnPortalMouseLeave mouseLeaveDelay={delay}><p>Hi</p></Portal>
)

domEvent.mouseLeave(wrapper.instance().node.firstElementChild)

setTimeout(() => {
document.body.lastElementChild.should.equal(wrapper.instance().node)
}, delay - 1)

setTimeout(() => {
document.body.childElementCount.should.equal(0)
done()
}, delay + 1)
})
})

describe('closeOnTriggerMouseLeave + closeOnPortalMouseLeave', () => {
it('should close portal on trigger mouseleave even when portal receives mouseover within limit', (done) => {
const trigger = <button>button</button>
const delay = 100
wrapperMount(
<Portal trigger={trigger} defaultOpen closeOnTriggerMouseLeave mouseLeaveDelay={delay}><p>Hi</p></Portal>
)

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

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

// The portal should close because closeOnPortalMouseLeave not set
setTimeout(() => {
document.body.childElementCount.should.equal(0)
done()
}, delay + 1)
})

it('should not close portal on trigger mouseleave when portal receives mouseover within limit', (done) => {
const trigger = <button>button</button>
const delay = 100
wrapperMount(
<Portal trigger={trigger} defaultOpen closeOnTriggerMouseLeave
closeOnPortalMouseLeave mouseLeaveDelay={delay}
><p>Hi</p></Portal>
)

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

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

// The portal should not have closed
setTimeout(() => {
document.body.lastElementChild.should.equal(wrapper.instance().node)
done()
}, delay + 1)
})
})

Expand Down
Loading