Skip to content

Commit

Permalink
fix(Dropdown): open/close on focus/blur (#500)
Browse files Browse the repository at this point in the history
* refactor(config): use full sourcemaps

* fix(Dropdown): open/close on focus/blur if !mouseDown

* fix(Dropdown-test): fix some tests

* refactor(Selection): revert dropdown selection example

* fix(Dropdown-test): fix remaining tests

* fix(Dropdown): remove item onMouseDown

* test(Dropdown): add/update tests

* test(Dropdown): fix/add more coverage

* refactor(Dropdown): restore eslint comments
  • Loading branch information
levithomason authored Sep 16, 2016
1 parent de2b2fa commit 704697a
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 98 deletions.
2 changes: 1 addition & 1 deletion config.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ config = Object.assign({}, config, {
// ----------------------------------
// Compiler Configuration
// ----------------------------------
compiler_devtool: __DEV__ && 'eval-cheap-module-source-map'
compiler_devtool: __DEV__ && 'source-map'
|| __TEST__ && 'source-map'
|| __STAGING__ && 'source-map',
compiler_hash_type: __PROD__ ? 'chunkhash' : 'hash',
Expand Down
104 changes: 54 additions & 50 deletions src/modules/Dropdown/Dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const _meta = {
name: 'Dropdown',
type: META.TYPES.MODULE,
props: {
pointing: ['bottom left', 'bottom right'],
pointing: ['left', 'right', 'top', 'top left', 'top right', 'bottom', 'bottom left', 'bottom right'],
additionPosition: ['top', 'bottom'],
},
}
Expand Down Expand Up @@ -159,6 +159,9 @@ export default class Dropdown extends Component {
/** Called with the React Synthetic Event on Dropdown focus. */
onFocus: PropTypes.func,

/** Called with the React Synthetic Event on Dropdown mouse down. */
onMouseDown: PropTypes.func,

// ------------------------------------
// Style
// ------------------------------------
Expand Down Expand Up @@ -246,14 +249,14 @@ export default class Dropdown extends Component {
super.componentWillReceiveProps(nextProps)
debug('componentWillReceiveProps()')
// TODO objectDiff still runs in prod, stop it
debug('changed props:', objectDiff(nextProps, this.props))
debug('to props:', objectDiff(this.props, nextProps))

/* eslint-disable no-console */
if (process.env.NODE_ENV !== 'production') {
// in development, validate value type matches dropdown type
const isNextValueArray = Array.isArray(nextProps.value)
const hasValue = _.has(nextProps, 'value')

/* eslint-disable no-console */
if (hasValue && nextProps.multiple && !isNextValueArray) {
console.error(
'Dropdown `value` must be an array when `multiple` is set.' +
Expand All @@ -265,8 +268,8 @@ export default class Dropdown extends Component {
' Either set `multiple={true}` or use a string or number value.'
)
}
/* eslint-enable no-console */
}
/* eslint-enable no-console */

if (!_.isEqual(nextProps.value, this.props.value)) {
debug('value changed, setting', nextProps.value)
Expand All @@ -277,10 +280,15 @@ export default class Dropdown extends Component {
componentDidUpdate(prevProps, prevState) { // eslint-disable-line complexity
debug('componentDidUpdate()')
// TODO objectDiff still runs in prod, stop it
debug('changed state:', objectDiff(this.state, prevState))
debug('to state:', objectDiff(prevState, this.state))

// focused / blurred
if (!prevState.focus && this.state.focus) {
debug('dropdown focused')
if (!this.isMouseDown) {
debug('mouse is not down, opening')
this.open()
}
if (!this.state.open) {
document.addEventListener('keydown', this.openOnArrow)
document.addEventListener('keydown', this.openOnSpace)
Expand All @@ -290,16 +298,21 @@ export default class Dropdown extends Component {
document.addEventListener('keydown', this.removeItemOnBackspace)
}
} else if (prevState.focus && !this.state.focus) {
debug('dropdown blurred')
if (!this.isMouseDown) {
debug('mouse is not down, closing')
this.close()
}
document.removeEventListener('keydown', this.openOnArrow)
document.removeEventListener('keydown', this.openOnSpace)
document.removeEventListener('keydown', this.moveSelectionOnKeyDown)
document.removeEventListener('keydown', this.selectItemOnEnter)
document.removeEventListener('keydown', this.removeItemOnBackspace)
this.close()
}

// opened / closed
if (!prevState.open && this.state.open) {
debug('dropdown opened')
this.open()
document.addEventListener('keydown', this.closeOnEscape)
document.addEventListener('keydown', this.moveSelectionOnKeyDown)
Expand All @@ -309,6 +322,7 @@ export default class Dropdown extends Component {
document.removeEventListener('keydown', this.openOnArrow)
document.removeEventListener('keydown', this.openOnSpace)
} else if (prevState.open && !this.state.open) {
debug('dropdown closed')
this.close()
document.removeEventListener('keydown', this.closeOnEscape)
document.removeEventListener('keydown', this.moveSelectionOnKeyDown)
Expand Down Expand Up @@ -370,19 +384,19 @@ export default class Dropdown extends Component {
}

openOnSpace = (e) => {
debug('openOnSpace()')
if (keyboardKey.getCode(e) !== keyboardKey.Spacebar) return
if (this.state.open) return
e.preventDefault()
// TODO open/close should only change state, open/closeMenu should be called on did update
this.trySetState({ open: true })
}

openOnArrow = (e) => {
const code = keyboardKey.getCode(e)
debug('openOnArrow()')
if (!_.includes([keyboardKey.ArrowDown, keyboardKey.ArrowUp], code)) return
if (this.state.open) return
e.preventDefault()
// TODO open/close should only change state, open/closeMenu should be called on did update
this.trySetState({ open: true })
}

Expand Down Expand Up @@ -447,9 +461,22 @@ export default class Dropdown extends Component {
// Component Event Handlers
// ----------------------------------------

handleMouseDown = (e) => {
debug('handleMouseDown()')
const { onMouseDown } = this.props
if (onMouseDown) onMouseDown(e)
this.isMouseDown = true
document.addEventListener('mouseup', this.handleDocumentMouseUp)
}

handleDocumentMouseUp = () => {
debug('handleDocumentMouseUp()')
this.isMouseDown = false
document.removeEventListener('mouseup', this.handleDocumentMouseUp)
}

handleClick = (e) => {
debug('handleClick()')
debug(e)
debug('handleClick()', e)
const { onClick } = this.props
if (onClick) onClick(e)
// prevent closeOnDocumentClick()
Expand Down Expand Up @@ -492,11 +519,6 @@ export default class Dropdown extends Component {
const { onFocus } = this.props
if (onFocus) onFocus(e)
this.setState({ focus: true })
// TODO
// handleFocus() is called on mouse down
// handleClick() it called on mouse up
// open() here is immediately toggled to close() by click
// this.open()
}

handleBlur = (e) => {
Expand Down Expand Up @@ -693,29 +715,15 @@ export default class Dropdown extends Component {

open = () => {
debug('open()')
debug(`dropdown is ${this.state.open ? 'already' : 'not yet'} open`)
const { search } = this.props
if (search) this.refs.search.focus()
if (search) this._search.focus()

this.trySetState({
open: true,
}, {
dropdownClasses: 'active visible',
menuClasses: 'visible',
})
this.trySetState({ open: true })
}

close = () => {
debug('close()')
debug(`dropdown is ${!this.state.open ? 'already' : 'not yet'} closed`)
if (!this.state.open) return

this.trySetState({
open: false,
}, {
dropdownClasses: '',
menuClasses: '',
})
this.trySetState({ open: false })
}

toggle = () => this.state.open ? this.close() : this.open()
Expand Down Expand Up @@ -784,15 +792,13 @@ export default class Dropdown extends Component {
return (
<input
value={searchQuery}
onBlur={this.handleBlur}
onChange={this.handleSearchChange}
onFocus={this.handleFocus}
className='search'
name={[name, 'search'].join('-')}
autoComplete='off'
tabIndex='0'
style={{ width: searchWidth }}
ref='search'
ref={c => (this._search = c)}
/>
)
}
Expand Down Expand Up @@ -849,16 +855,17 @@ export default class Dropdown extends Component {
active={isActive(opt.value)}
onClick={this.handleItemClick}
selected={selectedIndex === i}
onMouseDown={e => e.preventDefault()} // prevent default to allow item select without closing on blur
{...opt}
style={{ ...opt.style, pointerEvents: 'all' }} // Needed for handling click events on disabled items
// Needed for handling click events on disabled items
style={{ ...opt.style, pointerEvents: 'all' }}
/>
))
}

renderMenu = () => {
const { children } = this.props
const { menuClasses } = this.state
const { open } = this.state
const menuClasses = open ? 'visible' : ''

// single menu child
if (children) {
Expand All @@ -879,7 +886,7 @@ export default class Dropdown extends Component {
debug('render()')
debug('props', this.props)
debug('state', this.state)
const { dropdownClasses } = this.state
const { open } = this.state

const {
button,
Expand All @@ -906,7 +913,7 @@ export default class Dropdown extends Component {
// Classes
const classes = cx(
'ui',
dropdownClasses,
open && 'active visible',
useKeyOnly(disabled, 'disabled'),
useKeyOnly(error, 'error'),
useKeyOnly(loading, 'loading'),
Expand Down Expand Up @@ -934,23 +941,20 @@ export default class Dropdown extends Component {
'dropdown',
)

const conditionalProps = search ? null : {
onBlur: this.handleBlur,
onClick: this.handleClick,
onFocus: this.handleFocus,
tabIndex: 0,
}

const rest = getUnhandledProps(Dropdown, this.props)
const ElementType = getElementType(Dropdown, this.props)

return (
<ElementType
{...rest}
{...conditionalProps}
onChange={this.onChange}
onClick={this.handleClick}
className={classes}
onBlur={this.handleBlur}
onClick={this.handleClick}
onMouseDown={this.handleMouseDown}
onFocus={this.handleFocus}
onChange={this.onChange}
tabIndex={search ? undefined : 0}
ref={c => (this._dropdown = c)}
>
{this.renderHiddenInput()}
{this.renderLabels()}
Expand Down
Loading

0 comments on commit 704697a

Please sign in to comment.