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(Dropdown): retains focus after selection #3452

Merged
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, () => {})
Copy link
Member

Choose a reason for hiding this comment

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

cruft?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is on purpose.
callback the 2nd argument in close method has default value.
But not using default value in this case.

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 }, callback)
    }
  }

}

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