Skip to content

Commit

Permalink
fix(Dropdown): fix minCharacter prop behaviour (#1722)
Browse files Browse the repository at this point in the history 
* wip

* test(Dropdown): add missing tests
  • Loading branch information
@layershifter @levithomason
layershifter authored and levithomason committed Jun 22, 2017
1 parent 65fd10c commit 9287f9c
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 68 deletions.
105 changes: 50 additions & 55 deletions src/modules/Dropdown/Dropdown.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,19 +331,19 @@ export default class Dropdown extends Component {
static defaultProps = {
additionLabel: 'Add ',
additionPosition: 'top',
closeOnBlur: true,
icon: 'dropdown',
minCharacters: 1,
noResultsMessage: 'No results found.',
openOnFocus: true,
renderLabel: ({ text }) => text,
selectOnBlur: true,
openOnFocus: true,
closeOnBlur: true,
minCharacters: 1,
}

static autoControlledProps = [
'open',
'value',
'selectedLabel',
'value',
]

static _meta = {
Expand Down Expand Up @@ -417,9 +417,11 @@ export default class Dropdown extends Component {
if (!prevState.focus && this.state.focus) {
debug('dropdown focused')
if (!this.isMouseDown) {
const { openOnFocus } = this.props
const { minCharacters, openOnFocus, search } = this.props
const openable = !search || (search && minCharacters === 1)

debug('mouse is not down, opening')
if (openOnFocus) this.open()
if (openOnFocus && openable) this.open()
}
if (!this.state.open) {
document.addEventListener('keydown', this.openOnArrow)
Expand Down Expand Up @@ -489,10 +491,8 @@ export default class Dropdown extends Component {
// onChange needs to receive a value
// can't rely on props.value if we are controlled
handleChange = (e, value) => {
debug('handleChange()')
debug(value)
const { onChange } = this.props
if (onChange) onChange(e, { ...this.props, value })
debug('handleChange()', value)
_.invoke(this.props, 'onChange', e, { ...this.props, value })
}

closeOnChange = (e) => {
Expand Down Expand Up @@ -576,11 +576,13 @@ export default class Dropdown extends Component {
}

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

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

if (search && _.isEmpty(this.getMenuOptions())) return
this.makeSelectedItemActive(e)
this.closeOnChange(e)
}
Expand Down Expand Up @@ -620,29 +622,34 @@ export default class Dropdown extends Component {

handleMouseDown = (e) => {
debug('handleMouseDown()')
const { onMouseDown } = this.props
if (onMouseDown) onMouseDown(e, this.props)

this.isMouseDown = true
_.invoke(this.props, 'onMouseDown', e, this.props)
// Do not access document when server side rendering
if (!isBrowser) return
document.addEventListener('mouseup', this.handleDocumentMouseUp)
if (isBrowser) document.addEventListener('mouseup', this.handleDocumentMouseUp)
}

handleDocumentMouseUp = () => {
debug('handleDocumentMouseUp()')

this.isMouseDown = false
// Do not access document when server side rendering
if (!isBrowser) return
document.removeEventListener('mouseup', this.handleDocumentMouseUp)
if (isBrowser) document.removeEventListener('mouseup', this.handleDocumentMouseUp)
}

handleClick = (e) => {
handleClick = e => {
debug('handleClick()', e)
const { onClick } = this.props
if (onClick) onClick(e, this.props)

const { minCharacters, search } = this.props
const { open, searchQuery } = this.state

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

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

handleItemClick = (e, item) => {
Expand Down Expand Up @@ -679,14 +686,15 @@ export default class Dropdown extends Component {

handleFocus = (e) => {
debug('handleFocus()')
const { onFocus } = this.props
const { focus } = this.state

if (focus) return
if (onFocus) onFocus(e, this.props)

_.invoke(this.props, 'onFocus', e, this.props)
this.setState({ focus: true })
}

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

// Heads up! Don't remove this.
Expand All @@ -705,26 +713,28 @@ export default class Dropdown extends Component {
this.setState({ focus: false, searchQuery: '' })
}

handleSearchChange = (e) => {
debug('handleSearchChange()')
debug(e.target.value)
handleSearchChange = e => {
debug('handleSearchChange()', e)
// prevent propagating to this.props.onChange()
e.stopPropagation()
const { search, onSearchChange, minCharacters } = this.props
const { open } = this.state
const newQuery = e.target.value

if (onSearchChange) onSearchChange(e, newQuery)
const { minCharacters } = this.props
const { open } = this.state
const newQuery = _.get(e, 'target.value', '')

if (newQuery.length >= minCharacters) {
// open search dropdown on search query
if (search && newQuery && !open) this.open()
_.invoke(this.props, 'onSearchChange', e, newQuery)
this.setState({
selectedIndex: 0,
searchQuery: newQuery,
})

this.setState({
selectedIndex: 0,
searchQuery: newQuery,
})
// open search dropdown on search query
if (!open && newQuery.length >= minCharacters) {
this.open()
return
}
// close search dropdown if search query is too small
if (open && minCharacters !== 1 && newQuery.length < minCharacters) this.close()
}

// ----------------------------------------
Expand Down Expand Up @@ -1009,22 +1019,7 @@ export default class Dropdown extends Component {
this.setState({ focus: hasFocus })
}

toggle = (e) => {
if (!this.state.open) {
this.open(e)
return
}

const { search } = this.props
const options = this.getMenuOptions()

if (search && _.isEmpty(options)) {
e.preventDefault()
return
}

this.close(e)
}
toggle = e => this.state.open ? this.close(e) : this.open(e)

// ----------------------------------------
// Render
Expand Down
118 changes: 105 additions & 13 deletions test/specs/modules/Dropdown/Dropdown-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1428,6 +1428,76 @@ describe('Dropdown', () => {
})
})

describe('onClick', () => {
it('is called with (event, props)', () => {
const onClick = sandbox.spy()
wrapperMount(<Dropdown onClick={onClick} options={options} />)
wrapper.simulate('click', { stopPropagation: _.noop })

onClick.should.have.been.calledOnce()
onClick.should.have.been.calledWithMatch({ }, { options })
})

it("toggles the dropdown when it's not searchable", () => {
wrapperMount(<Dropdown options={options} />)

wrapper.simulate('click')
dropdownMenuIsOpen()

wrapper.simulate('click')
dropdownMenuIsClosed()
})

it("opens the dropdown when it's searchable, but don't close", () => {
wrapperMount(<Dropdown options={options} search />)

wrapper.simulate('click')
dropdownMenuIsOpen()

wrapper.simulate('click')
dropdownMenuIsOpen()
})

it("don't open the dropdown when it's searchable and minCharacters is more that default value", () => {
wrapperMount(<Dropdown minCharacters={3} options={options} search />)

wrapper.simulate('click')
dropdownMenuIsClosed()
})
})

describe('onFocus', () => {
it('is called with (event, props)', () => {
const onFocus = sandbox.spy()
wrapperMount(<Dropdown onFocus={onFocus} options={options} />)
wrapper.simulate('focus')

onFocus.should.have.been.calledOnce()
onFocus.should.have.been.calledWithMatch({ }, { options })
})

it("opens the dropdown when it's not searchable", () => {
wrapperMount(<Dropdown options={options} />)

wrapper.simulate('focus')
dropdownMenuIsOpen()
})

it("opens the dropdown when it's searchable", () => {
wrapperMount(<Dropdown options={options} search />)

wrapper.simulate('focus')
dropdownMenuIsOpen()
})

it("don't open the dropdown when it's searchable and minCharacters is more that default value", () => {
wrapperMount(<Dropdown minCharacters={3} options={options} search />)

wrapper.simulate('focus')
dropdownMenuIsClosed()
})
})

describe('onSearchChange', () => {
it('is called with (event, value) on search input change', () => {
const spy = sandbox.spy()
Expand All @@ -1438,6 +1508,30 @@ describe('Dropdown', () => {
spy.should.have.been.calledOnce()
spy.should.have.been.calledWithMatch({ target: { value: 'a' } }, 'a')
})

it("don't open the menu on change if query's length is less than minCharacters", () => {
wrapperMount(<Dropdown minCharacters={3} options={options} selection search />)

dropdownMenuIsClosed()

// simulate search with query's length is less than minCharacters
wrapper
.find('input.search')
.simulate('change', { target: { value: 'a' } })

dropdownMenuIsClosed()
})

it("closes the opened menu on change if query's length is less than minCharacters", () => {
wrapperMount(<Dropdown minCharacters={3} options={options} selection search />)
const input = wrapper.find('input.search')

input.simulate('change', { target: { value: 'abc' } })
dropdownMenuIsOpen()

input.simulate('change', { target: { value: 'a' } })
dropdownMenuIsClosed()
})
})

describe('options', () => {
Expand Down Expand Up @@ -1577,19 +1671,6 @@ describe('Dropdown', () => {
dropdownMenuIsOpen()
})

it("Don't open the menu on change if query's length is less than minCharacters", () => {
wrapperMount(<Dropdown options={options} selection search minCharacters={4} />)

dropdownMenuIsClosed()

// simulate search with query's length is less than minCharacters
wrapper
.find('input.search')
.simulate('change', { target: { value: faker.hacker.noun().substring(0, 1) } })

dropdownMenuIsClosed()
})

it('does not call onChange on query change', () => {
const onChangeSpy = sandbox.spy()
wrapperMount(<Dropdown options={options} selection search onChange={onChangeSpy} />)
Expand Down Expand Up @@ -1696,6 +1777,17 @@ describe('Dropdown', () => {
.at(1)
.should.not.have.prop('selected', true)
})

it('does not close the menu when options are empty', () => {
wrapperMount(<Dropdown options={options} search selection />)
wrapper.simulate('click')

wrapper.find('input.search')
.simulate('change', { target: { value: 'foo' } })
domEvent.keyDown(document, { key: 'Enter' })

dropdownMenuIsOpen()
})
})

describe('no results message', () => {
Expand Down

0 comments on commit 9287f9c

Please sign in to comment.