Skip to content

Commit

Permalink
refactor(Dropdown): remove hidden select
Browse files Browse the repository at this point in the history
  • Loading branch information
@levithomason
levithomason committed Jun 1, 2017
1 parent 2ec4510 commit 78b522c
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 102 deletions.
3 changes: 0 additions & 3 deletions src/modules/Dropdown/Dropdown.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,6 @@ export interface DropdownProps {
/** A selection dropdown can allow multiple selections. */
multiple?: boolean;

/** Name of the hidden input which holds the value. */
name?: string;

/** Message to display when there are no results. */
noResultsMessage?: string;

Expand Down
27 changes: 1 addition & 26 deletions src/modules/Dropdown/Dropdown.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,6 @@ export default class Dropdown extends Component {
/** A selection dropdown can allow multiple selections. */
multiple: PropTypes.bool,

/** Name of the hidden input which holds the value. */
name: PropTypes.string,

/** Message to display when there are no results. */
noResultsMessage: PropTypes.string,

Expand Down Expand Up @@ -1060,28 +1057,8 @@ export default class Dropdown extends Component {
return <div className={classes}>{_text}</div>
}

renderHiddenInput = () => {
debug('renderHiddenInput()')
const { value } = this.state
const { multiple, name, options, selection } = this.props
debug(`name: ${name}`)
debug(`selection: ${selection}`)
debug(`value: ${value}`)
if (!selection) return null

// a dropdown without an active item will have an empty string value
return (
<select type='hidden' aria-hidden='true' name={name} value={value} multiple={multiple}>
<option value='' />
{_.map(options, (option, i) => (
<option key={getKeyOrValue(option.key, option.value)} value={option.value}>{option.text}</option>
))}
</select>
)
}

renderSearchInput = () => {
const { disabled, search, name, tabIndex } = this.props
const { disabled, search, tabIndex } = this.props
const { searchQuery } = this.state

if (!search) return null
Expand All @@ -1107,7 +1084,6 @@ export default class Dropdown extends Component {
aria-autocomplete='list'
onChange={this.handleSearchChange}
className='search'
name={[name, 'search'].join('-')}
autoComplete='off'
tabIndex={computedTabIndex}
style={{ width: searchWidth }}
Expand Down Expand Up @@ -1285,7 +1261,6 @@ export default class Dropdown extends Component {
tabIndex={computedTabIndex}
ref={this.handleRef}
>
{this.renderHiddenInput()}
{this.renderLabels()}
{this.renderSearchInput()}
{this.renderSearchSizer()}
Expand Down
89 changes: 16 additions & 73 deletions test/specs/modules/Dropdown/Dropdown-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1247,21 +1247,20 @@ describe('Dropdown', () => {
})
describe('removing items', () => {
it('calls onChange without the clicked value', () => {
const name = 'my-dropdown'
const value = _.map(options, 'value')
const randomIndex = _.random(options.length - 1)
const randomValue = value[randomIndex]
const expected = _.without(value, randomValue)
const spy = sandbox.spy()
wrapperMount(<Dropdown options={options} selection name={name} value={value} multiple onChange={spy} />)
wrapperMount(<Dropdown options={options} selection value={value} multiple onChange={spy} />)

wrapper
.find('.delete.icon')
.at(randomIndex)
.simulate('click')

spy.should.have.been.calledOnce()
spy.should.have.been.calledWithMatch({}, { name, value: expected })
spy.should.have.been.calledWithMatch({}, { value: expected })
})
})
})
Expand All @@ -1283,25 +1282,23 @@ describe('Dropdown', () => {
spy.should.not.have.been.called()
})
it('removes the last item when there is no search query', () => {
const name = 'my-dropdown'
const value = _.map(options, 'value')
const expected = _.dropRight(value)
wrapperMount(<Dropdown options={options} selection name={name} value={value} multiple search onChange={spy} />)
wrapperMount(<Dropdown options={options} selection value={value} multiple search onChange={spy} />)

// open
wrapper.simulate('click')

domEvent.keyDown(document, { key: 'Backspace' })

spy.should.have.been.calledOnce()
spy.should.have.been.calledWithMatch({}, { name, value: expected })
spy.should.have.been.calledWithMatch({}, { value: expected })
})
it('removes the last item when there is no search query when uncontrolled', () => {
const name = 'my-dropdown'
const value = _.map(options, 'value')
const expected = _.dropRight(value)
wrapperMount(
<Dropdown options={options} selection name={name} defaultValue={value} multiple search onChange={spy} />
<Dropdown options={options} selection defaultValue={value} multiple search onChange={spy} />
)

// open
Expand All @@ -1310,7 +1307,7 @@ describe('Dropdown', () => {
domEvent.keyDown(document, { key: 'Backspace' })

spy.should.have.been.calledOnce()
spy.should.have.been.calledWithMatch({}, { name, value: expected })
spy.should.have.been.calledWithMatch({}, { value: expected })

wrapper
.state('value')
Expand Down Expand Up @@ -1351,38 +1348,35 @@ describe('Dropdown', () => {
})

it('is called with event and value on item click', () => {
const name = 'my-dropdown'
const randomIndex = _.random(options.length - 1)
const randomValue = options[randomIndex].value
wrapperMount(<Dropdown options={options} selection onChange={spy} name={name} />)
wrapperMount(<Dropdown options={options} selection onChange={spy} />)
.simulate('click')
.find('DropdownItem')
.at(randomIndex)
.simulate('click')

spy.should.have.been.calledOnce()
spy.should.have.been.calledWithMatch({}, { name, value: randomValue })
spy.should.have.been.calledWithMatch({}, { value: randomValue })
})
it('is called with event and value when pressing enter on a selected item', () => {
const name = 'my-dropdown'
const firstValue = options[0].value
wrapperMount(<Dropdown options={options} selection onChange={spy} name={name} />)
wrapperMount(<Dropdown options={options} selection onChange={spy} />)
.simulate('click')

domEvent.keyDown(document, { key: 'Enter' })

spy.should.have.been.calledOnce()
spy.should.have.been.calledWithMatch({}, { name, value: firstValue })
spy.should.have.been.calledWithMatch({}, { value: firstValue })
})
it('is called with event and value when blurring', () => {
const name = 'my-dropdown'
const firstValue = options[0].value
wrapperMount(<Dropdown options={options} selection onChange={spy} name={name} />)
wrapperMount(<Dropdown options={options} selection onChange={spy} />)
.simulate('focus') // open, highlights first item
.simulate('blur') // blur should activate selected item

spy.should.have.been.calledOnce()
spy.should.have.been.calledWithMatch({}, { name, value: firstValue })
spy.should.have.been.calledWithMatch({}, { value: firstValue })
})
it('is not called on blur when closed', () => {
wrapperMount(<Dropdown options={options} selection open={false} onChange={spy} />)
Expand Down Expand Up @@ -1513,48 +1507,6 @@ describe('Dropdown', () => {
})
})

describe('selection', () => {
it('does not add a hidden select by default', () => {
wrapperMount(<Dropdown />)
.should.not.have.descendants('select[type="hidden"]')
})
it('adds a hidden select', () => {
wrapperShallow(<Dropdown options={options} selection />)
.should.have.exactly(1).descendants('select[type="hidden"]')
})
it('passes the name prop to the hidden select', () => {
wrapperShallow(<Dropdown name='foo' options={options} selection />)
.find('select[type="hidden"]').should.have.prop('name', 'foo')
})
it('passes the value prop to the hidden select', () => {
const value = _.values(options)

wrapperShallow(<Dropdown name='foo' value={value} options={options} selection />)
.find('select[type="hidden"][name="foo"]').should.have.prop('value', value)
})
it('passes the multiple prop to the hidden select', () => {
const selector = 'select[type="hidden"][name="foo"]'

wrapperShallow(<Dropdown name='foo' multiple options={options} selection />)
.find(selector).should.have.prop('multiple', true)

wrapperShallow(<Dropdown name='foo' multiple={false} options={options} selection />)
.find(selector).should.have.prop('multiple', false)
})
it('adds options to the hidden select', () => {
wrapperShallow(<Dropdown options={options} selection />)
.should.have.exactly(options.length + 1).descendants('option')

options.forEach((opt, i) => {
// index is incremented to exclude the empty option value
const optionElement = wrapper.find('option').at(i + 1)

optionElement.should.have.prop('value', opt.value)
optionElement.should.have.text(opt.text)
})
})
})

describe('search', () => {
it('does not add a search input when not defined', () => {
wrapperShallow(<Dropdown options={options} selection />)
Expand All @@ -1566,13 +1518,6 @@ describe('Dropdown', () => {
.should.have.exactly(1).descendants('input.search')
})

it('adds the name prop to the search input', () => {
wrapperShallow(<Dropdown name='foo' options={options} selection search />)
.should.have.descendants('input.search')

wrapper.find('input.search').should.have.prop('name', 'foo-search')
})

it('sets focus to the search input on open', () => {
wrapperMount(<Dropdown options={options} selection search />)
.simulate('click')
Expand Down Expand Up @@ -2122,9 +2067,8 @@ describe('Dropdown', () => {

it('calls onAddItem prop when clicking new value', () => {
const spy = sandbox.spy()
const name = 'my-dropdown'
const search = wrapperMount(
<Dropdown options={customOptions} selection search allowAdditions onAddItem={spy} name={name} />
<Dropdown options={customOptions} selection search allowAdditions onAddItem={spy} />
)
.find('input.search')

Expand All @@ -2136,22 +2080,21 @@ describe('Dropdown', () => {
.simulate('click')

spy.should.have.been.calledOnce()
spy.should.have.been.calledWithMatch({}, { name, value: 'boo' })
spy.should.have.been.calledWithMatch({}, { value: 'boo' })
})

it('calls onAddItem prop when pressing enter on new value', () => {
const spy = sandbox.spy()
const name = 'my-dropdown'
const search = wrapperMount(
<Dropdown options={customOptions} selection search allowAdditions onAddItem={spy} name={name} />
<Dropdown options={customOptions} selection search allowAdditions onAddItem={spy} />
)
.find('input.search')

search.simulate('change', { target: { value: 'boo' } })
domEvent.keyDown(document, { key: 'Enter' })

spy.should.have.been.calledOnce()
spy.should.have.been.calledWithMatch({}, { name, value: 'boo' })
spy.should.have.been.calledWithMatch({}, { value: 'boo' })
})
})

Expand Down

0 comments on commit 78b522c

Please sign in to comment.