Skip to content

Commit

Permalink
fix(Dropdown): fix multiple problems (#1795)
Browse files Browse the repository at this point in the history 
* fix(Dropdown): fix multiple problems with minCharacters

* fix(Dropdown): fix multiple problems with minCharacters

* test(Dropdown): add new tests for fixes

* test(shorthand): revert changes

* style(Dropdown): fix lint issue

* test(Dropdown): add new tests for fixes

* fix(Dropdown): more fixes
  • Loading branch information
@layershifter @levithomason
layershifter authored and levithomason committed Jul 29, 2017
1 parent ca139f5 commit 62507c2
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 52 deletions.
132 changes: 81 additions & 51 deletions src/modules/Dropdown/Dropdown.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,11 +367,17 @@ export default class Dropdown extends Component {
static Menu = DropdownMenu
static SearchInput = DropdownSearchInput

getInitialAutoControlledState() {
return { searchQuery: '' }
}

componentWillMount() {
debug('componentWillMount()')
const { open, value } = this.state

this.setValue(value)
this.setSelectedIndex(value)

if (open) this.open()
}

Expand Down Expand Up @@ -408,6 +414,7 @@ export default class Dropdown extends Component {
if (!_.isEqual(nextProps.value, this.props.value)) {
debug('value changed, setting', nextProps.value)
this.setValue(nextProps.value)
this.setSelectedIndex(nextProps.value)
}

if (!_.isEqual(nextProps.options, this.props.options)) {
Expand Down Expand Up @@ -562,6 +569,7 @@ export default class Dropdown extends Component {
makeSelectedItemActive = (e) => {
const { open } = this.state
const { multiple, onAddItem } = this.props

const item = this.getSelectedItem()
const value = _.get(item, 'value')

Expand All @@ -570,49 +578,48 @@ export default class Dropdown extends Component {
if (!value || !open) return

// notify the onAddItem prop if this is a new value
if (onAddItem && item['data-additional']) {
onAddItem(e, { ...this.props, value })
}
if (onAddItem && item['data-additional']) onAddItem(e, { ...this.props, value })

// state value may be undefined
const newValue = multiple ? _.union(this.state.value, [value]) : value

// notify the onChange prop that the user is trying to change value
if (multiple) {
// state value may be undefined
const newValue = _.union(this.state.value, [value])
this.setValue(newValue)
this.handleChange(e, newValue)
} else {
this.setValue(value)
this.handleChange(e, value)
}
this.setValue(newValue)
this.setSelectedIndex(newValue)
this.handleChange(e, newValue)
}

selectItemOnEnter = (e) => {
debug('selectItemOnEnter()', keyboardKey.getName(e))
const { search } = this.props
const { multiple, search } = this.props

if (keyboardKey.getCode(e) !== keyboardKey.Enter) return
e.preventDefault()

if (search && _.isEmpty(this.getMenuOptions())) return

this.makeSelectedItemActive(e)
this.closeOnChange(e)

if (!multiple) this.clearSearchQuery()
if (search && this.searchRef) this.searchRef.focus()
}

removeItemOnBackspace = (e) => {
debug('removeItemOnBackspace()')
debug(keyboardKey.getName(e))
if (keyboardKey.getCode(e) !== keyboardKey.Backspace) return
debug('removeItemOnBackspace()', keyboardKey.getName(e))

const { multiple, search } = this.props
const { searchQuery, value } = this.state

if (keyboardKey.getCode(e) !== keyboardKey.Backspace) return
if (searchQuery || !search || !multiple || _.isEmpty(value)) return

e.preventDefault()

// remove most recent value
const newValue = _.dropRight(value)

this.setValue(newValue)
this.setSelectedIndex(newValue)
this.handleChange(e, newValue)
}

Expand Down Expand Up @@ -659,39 +666,48 @@ export default class Dropdown extends Component {

if (!search) return this.toggle(e)
if (open) return
if (searchQuery.length >= minCharacters || minCharacters === 1) this.open(e)
if (searchQuery.length >= minCharacters || minCharacters === 1) {
this.open(e)
return
}
if (this.searchRef) this.searchRef.focus()
}

handleIconClick = e => {
debug('handleIconClick()', e)

_.invoke(this.props, 'onClick', e, this.props)
// prevent handleClick()
e.stopPropagation()
this.toggle(e)
}

handleItemClick = (e, item) => {
debug('handleItemClick()')
debug(item)
const { multiple, onAddItem } = this.props
debug('handleItemClick()', item)

const { multiple, onAddItem, search } = this.props
const { value } = item

// prevent toggle() in handleClick()
e.stopPropagation()
// prevent closeOnDocumentClick() if multiple or item is disabled
if (multiple || item.disabled) {
e.nativeEvent.stopImmediatePropagation()
}

if (multiple || item.disabled) e.nativeEvent.stopImmediatePropagation()
if (item.disabled) return

// notify the onAddItem prop if this is a new value
if (onAddItem && item['data-additional']) {
onAddItem(e, { ...this.props, value })
}
if (onAddItem && item['data-additional']) onAddItem(e, { ...this.props, value })

const newValue = multiple ? _.union(this.state.value, [value]) : value

// notify the onChange prop that the user is trying to change value
if (multiple) {
const newValue = _.union(this.state.value, [value])
this.setValue(newValue)
this.handleChange(e, newValue)
} else {
this.setValue(value)
this.handleChange(e, value)
}
this.setValue(newValue)
this.setSelectedIndex(value)
if (!multiple) this.clearSearchQuery()

this.handleChange(e, newValue)
this.closeOnChange(e)

if (multiple && search && this.searchRef) this.searchRef.focus()
}

handleFocus = (e) => {
Expand Down Expand Up @@ -720,7 +736,8 @@ export default class Dropdown extends Component {
this.makeSelectedItemActive(e)
if (closeOnBlur) this.close()
}
this.setState({ focus: false, searchQuery: '' })
this.setState({ focus: false })
this.clearSearchQuery()
}

handleSearchChange = (e, { value }) => {
Expand Down Expand Up @@ -829,7 +846,7 @@ export default class Dropdown extends Component {
return _.findIndex(options, ['value', value])
}

getDropdownAriaOptions = (ElementType) => {
getDropdownAriaOptions = () => {
const { loading, disabled, search, multiple } = this.props
const { open } = this.state
const ariaOptions = {
Expand Down Expand Up @@ -859,18 +876,14 @@ export default class Dropdown extends Component {
// Setters
// ----------------------------------------

setValue = (value) => {
debug('setValue()')
debug('value', value)
const newState = {
searchQuery: '',
}

const { multiple, search } = this.props
if (multiple && search && this.searchRef) this.searchRef.focus()
clearSearchQuery = () => {
debug('clearSearchQuery()')
this.setState({ searchQuery: '' })
}

this.trySetState({ value }, newState)
this.setSelectedIndex(value)
setValue = (value) => {
debug('setValue()', value)
this.trySetState({ value })
}

setSelectedIndex = (value = this.state.value, optionsProps = this.props.options) => {
Expand Down Expand Up @@ -935,6 +948,7 @@ export default class Dropdown extends Component {
debug('new value:', newValue)

this.setValue(newValue)
this.setSelectedIndex(newValue)
this.handleChange(e, newValue)
}

Expand All @@ -954,12 +968,26 @@ export default class Dropdown extends Component {
if (nextIndex > lastIndex) nextIndex = 0
else if (nextIndex < 0) nextIndex = lastIndex

if (options[nextIndex].disabled) return this.moveSelectionBy(offset, nextIndex)
if (options[nextIndex].disabled) {
this.moveSelectionBy(offset, nextIndex)
return
}

this.setState({ selectedIndex: nextIndex })
this.scrollSelectedItemIntoView()
}

// ----------------------------------------
// Overrides
// ----------------------------------------

handleIconOverrides = predefinedProps => ({
onClick: (e) => {
_.invoke(predefinedProps, 'onClick', e, predefinedProps)
this.handleIconClick(e)
},
})

// ----------------------------------------
// Refs
// ----------------------------------------
Expand Down Expand Up @@ -1275,7 +1303,9 @@ export default class Dropdown extends Component {
{this.renderSearchInput()}
{this.renderSearchSizer()}
{trigger || this.renderText()}
{Icon.create(icon)}
{Icon.create(icon, {
overrideProps: this.handleIconOverrides,
})}
{this.renderMenu()}
</ElementType>
)
Expand Down
57 changes: 56 additions & 1 deletion test/specs/modules/Dropdown/Dropdown-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,16 @@ describe('Dropdown', () => {
common.isConformant(Dropdown)
common.hasUIClassName(Dropdown)
common.hasSubComponents(Dropdown, [DropdownDivider, DropdownHeader, DropdownItem, DropdownMenu, DropdownSearchInput])
common.implementsIconProp(Dropdown)

common.implementsIconProp(Dropdown, {
assertExactMatch: false,
})
common.implementsShorthandProp(Dropdown, {
propKey: 'header',
ShorthandComponent: DropdownHeader,
mapValueToProps: val => ({ content: val }),
})

common.propKeyOnlyToClassName(Dropdown, 'disabled')
common.propKeyOnlyToClassName(Dropdown, 'error')
common.propKeyOnlyToClassName(Dropdown, 'loading')
Expand Down Expand Up @@ -431,6 +435,35 @@ describe('Dropdown', () => {
wrapperRender(<Dropdown />)
.should.contain.descendants('.dropdown.icon')
})

it('always opens a dropdown on click', () => {
wrapperMount(<Dropdown options={options} selection search />)
.find('i.icon')
.simulate('click')

dropdownMenuIsOpen()
})

it('always opens a dropdown on click', () => {
wrapperMount(<Dropdown options={options} selection search />)
.find('i.icon')
.simulate('click')

dropdownMenuIsOpen()
})

it('passes onClick handler', () => {
const event = { target: null }
const onClick = sandbox.spy()
const props = { name: 'user', onClick }

wrapperMount(<Dropdown icon={props} options={options} />)
.find('i.icon')
.simulate('click', event)

onClick.should.have.been.calledOnce()
onClick.should.have.been.calledWithMatch(event, props)
})
})

describe('selected item', () => {
Expand Down Expand Up @@ -728,6 +761,16 @@ describe('Dropdown', () => {
domEvent.keyDown(document, { key: 'Enter' })
dropdownMenuIsClosed()
})

it('keeps value of the searchQuery when selection is changed', () => {
wrapperMount(<Dropdown options={options} selection search />)

wrapper.setState({ searchQuery: 'foo' })
wrapper.simulate('click')
domEvent.keyDown(document, { key: 'ArrowDown' })

wrapper.should.have.state('searchQuery', 'foo')
})
})

describe('value', () => {
Expand Down Expand Up @@ -1638,6 +1681,18 @@ describe('Dropdown', () => {
)
})

it('sets focus to the search input on click on the placeholder', () => {
wrapperMount(<Dropdown minCharacters={3} options={options} placeholder='foo' selection search />)
.find('.default.text')
.simulate('click')

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.`
)
})

it('clears the search query when an item is selected', () => {
// search for random item
const searchQuery = _.sample(options).text
Expand Down

0 comments on commit 62507c2

Please sign in to comment.