Skip to content

Commit

Permalink
fix(Dropdown): retains focus after selection (#3452)
Browse files Browse the repository at this point in the history
* fix(Dropdown): retains focus after selection

* remove the state argument in trySetState method to match React's API

* add assertions for 'focus' state

* update Dropdown.js

* restore style

* restore style

* use _.noop instead
  • Loading branch information
jongsue authored and layershifter committed Aug 4, 2019
1 parent 0d66fa7 commit 2193182
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 11 deletions.
18 changes: 11 additions & 7 deletions src/modules/Dropdown/Dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,6 @@ export default class Dropdown extends Component {
this.scrollSelectedItemIntoView()
} else if (prevState.open && !this.state.open) {
debug('dropdown closed')
this.handleClose()
}
}

Expand All @@ -498,7 +497,7 @@ export default class Dropdown extends Component {
const { closeOnChange, multiple } = this.props
const shouldClose = _.isUndefined(closeOnChange) ? !multiple : closeOnChange

if (shouldClose) this.close(e)
if (shouldClose) this.close(e, _.noop)
}

closeOnEscape = (e) => {
Expand Down Expand Up @@ -707,13 +706,18 @@ export default class Dropdown extends Component {
}

this.clearSearchQuery(value)

if (search) {
_.invoke(this.searchRef.current, 'focus')
} else {
_.invoke(this.ref.current, 'focus')
}

this.closeOnChange(e)

// Heads up! This event handler should be called after `onChange`
// Notify the onAddItem prop if this is a new value
if (isAdditionItem) _.invoke(this.props, 'onAddItem', e, { ...this.props, value })

if (search) _.invoke(this.searchRef.current, 'focus')
}

handleFocus = (e) => {
Expand Down Expand Up @@ -1147,13 +1151,13 @@ export default class Dropdown extends Component {
this.scrollSelectedItemIntoView()
}

close = (e) => {
close = (e, callback = this.handleClose) => {
const { open } = this.state
debug('close()', { open })

if (open) {
_.invoke(this.props, 'onClose', e, this.props)
this.trySetState({ open: false })
this.trySetState({ open: false }, callback)
}
}

Expand All @@ -1164,7 +1168,7 @@ export default class Dropdown extends Component {
// https://github.com/Semantic-Org/Semantic-UI-React/issues/627
// Blur the Dropdown on close so it is blurred after selecting an item.
// This is to prevent it from re-opening when switching tabs after selecting an item.
if (!hasSearchFocus) {
if (!hasSearchFocus && this.ref.current) {
this.ref.current.blur()
}

Expand Down
31 changes: 27 additions & 4 deletions test/specs/modules/Dropdown/Dropdown-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1331,6 +1331,7 @@ describe('Dropdown', () => {
dropdownMenuIsOpen()

// select item
item.simulate('mousedown')
item.simulate('click')
dropdownMenuIsClosed()
})
Expand Down Expand Up @@ -2210,21 +2211,43 @@ describe('Dropdown', () => {
})

it('sets focus to the search input after selection', () => {
wrapperMount(<Dropdown open options={options} selection search />)

// random item, skip the first as it's selected by default
const randomIndex = 1 + _.random(options.length - 2)

wrapper
wrapperMount(<Dropdown options={options} selection search />)
.simulate('click', nativeEvent)
.find('DropdownItem')
.at(randomIndex)
.simulate('click')
.simulate('click', nativeEvent)

dropdownMenuIsClosed()

const activeElement = document.activeElement
const searchIsFocused = activeElement === document.querySelector('input.search')
searchIsFocused.should.be.true(
`Expected "input.search" to be the active element but found ${activeElement} instead.`,
)
wrapper.should.have.state('focus', true)
})

it('sets focus to the dropdown after selection', () => {
const randomIndex = _.random(options.length - 1)

wrapperMount(<Dropdown options={options} selection />)
.simulate('click', nativeEvent)
.find('DropdownItem')
.at(randomIndex)
.simulate('click', nativeEvent)

dropdownMenuIsClosed()

const activeElement = document.activeElement
const dropdownIsFocused = activeElement === document.querySelector('div.dropdown')
dropdownIsFocused.should.be.true(
`Expected Dropdown to be the active element but found ${activeElement} instead.`,
)

wrapper.should.have.state('focus', true)
})
})

Expand Down

0 comments on commit 2193182

Please sign in to comment.