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

Jump to bottom

breaking(Portal|Popup|Modal): do not autofocus #1341

Merged
merged 1 commit into from
Feb 17, 2017
Merged

breaking(Portal|Popup|Modal): do not autofocus #1341

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
19 changes: 1 addition & 18 deletions src/addons/Portal/Portal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ class Portal extends Component {
if (!this.state.open) return
debug('renderPortal()')

const { children, className, closeOnTriggerBlur, openOnTriggerFocus } = this.props
const { children, className } = this.props

this.mountPortal()

Expand All @@ -358,22 +358,6 @@ class Portal extends Component {

this.portalNode = this.rootNode.firstElementChild

// don't take focus away from for focus based portal triggers
if (!this.didInitialRender && !(openOnTriggerFocus || closeOnTriggerBlur)) {
this.didInitialRender = true

// add a tabIndex so we can focus it, remove outline
this.portalNode.tabIndex = -1
this.portalNode.style.outline = 'none'

// Wait a tick for things like popups which need to calculate where the popup shows up.
// Otherwise, the element is focused at its initial position, scrolling the browser, then
// it is immediately repositioned at the proper location.
setTimeout(() => {
if (this.portalNode) this.portalNode.focus()
})
}

this.portalNode.addEventListener('mouseleave', this.handlePortalMouseLeave)
this.portalNode.addEventListener('mouseenter', this.handlePortalMouseEnter)
}
Expand Down Expand Up @@ -405,7 +389,6 @@ class Portal extends Component {

unmountPortal = () => {
if (!isBrowser || !this.rootNode) return
this.didInitialRender = false

debug('unmountPortal()')

Expand Down
59 changes: 32 additions & 27 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 @@ -463,56 +463,61 @@ describe('Portal', () => {
})
})

// Heads Up!
// Portals used to take focus on mount and restore focus to the original activeElement on unMount.
// One by one, these auto set/remove focus features were removed and the assertions negated.
// Leave these tests here to ensure we aren't ever stealing focus.
describe('focus', () => {
it('takes focus on first render', (done) => {
it('does not take focus onMount', (done) => {
wrapperMount(<Portal defaultOpen><p /></Portal>)

setTimeout(() => {
const { portalNode } = wrapper.instance()
document.activeElement.should.equal(portalNode)
expect(portalNode.getAttribute('tabindex')).to.equal('-1')
expect(portalNode.style.outline).to.equal('none')
document.activeElement.should.not.equal(portalNode)
done()
}, 0)
})

it('does not take focus when mounted on portals that closeOnTriggerBlur', (done) => {
wrapperMount(<Portal defaultOpen closeOnTriggerBlur><p /></Portal>)
it('does not take focus on unMount', (done) => {
const input = document.createElement('input')
document.body.appendChild(input)

setTimeout(() => {
const { portalNode } = wrapper.instance()
document.activeElement.should.not.equal(portalNode)
expect(portalNode.getAttribute('tabindex')).to.not.equal('-1')
expect(portalNode.style.outline).to.not.equal('none')
done()
}, 0)
})
input.focus()
document.activeElement.should.equal(input)

it('does not take focus when mounted on portals that openOnTriggerFocus', (done) => {
wrapperMount(<Portal defaultOpen openOnTriggerFocus><p /></Portal>)
wrapperMount(<Portal open><p /></Portal>)
document.activeElement.should.equal(input)

setTimeout(() => {
const { portalNode } = wrapper.instance()
document.activeElement.should.not.equal(portalNode)
expect(portalNode.getAttribute('tabindex')).to.not.equal('-1')
expect(portalNode.style.outline).to.not.equal('none')
document.activeElement.should.equal(input)

wrapper.setProps({ open: false })
wrapper.unmount()

document.activeElement.should.equal(input)

document.body.removeChild(input)
done()
}, 0)
})

it('does not take focus on subsequent renders', (done) => {
wrapperMount(<Portal defaultOpen><input data-focus-me /></Portal>)
it('does not take focus on re-render', (done) => {
const input = document.createElement('input')
document.body.appendChild(input)

setTimeout(() => {
const { portalNode } = wrapper.instance()
document.activeElement.should.equal(portalNode)
input.focus()
document.activeElement.should.equal(input)

wrapperMount(<Portal defaultOpen><p /></Portal>)
document.activeElement.should.equal(input)

const input = document.querySelector('input[data-focus-me]')
input.focus()
setTimeout(() => {
document.activeElement.should.equal(input)

wrapper.render()
document.activeElement.should.equal(input)

document.body.removeChild(input)
done()
}, 0)
})
Expand Down