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

Support multiple dropdown type #254

Merged
merged 21 commits into from
Jun 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e8bdd84
refactor(Label): rename click listeners, cleanup
levithomason Jun 3, 2016
be277ef
feat(Dropdown): support multiple type
levithomason Jun 3, 2016
a107fa7
docs(Dropdown): show multiple usage
levithomason Jun 3, 2016
58d93e7
feat(Dropdown): spread getUnhandledProps
levithomason Jun 4, 2016
8adc585
test(Dropdown): multiple type label remove
levithomason Jun 4, 2016
b236294
fix(DropdownItem): remove cruft __onClick
levithomason Jun 4, 2016
2b00efc
refactor(Dropdown): use array for state.value
levithomason Jun 4, 2016
f4f064b
test(Dropdown): fix placeholder, add tests
levithomason Jun 4, 2016
20352c6
test(Dropdown): test removing items
levithomason Jun 5, 2016
2dd7e80
feat(Dropdown): onChange callback with e, val
levithomason Jun 5, 2016
a3993db
docs(Dropdown): use empty initial value
levithomason Jun 5, 2016
dd3b171
feat(Dropdown): use value array for multiple only
levithomason Jun 8, 2016
ed58000
test(Dropdown): fix and test open prop toggle
levithomason Jun 8, 2016
868756c
fix(Dropdown): combine placeholder and text
levithomason Jun 8, 2016
1ff2d3d
fix(Dropdown): fix lint errors
levithomason Jun 8, 2016
511a51d
refactor(Dropdown): simplify initial selected item
levithomason Jun 8, 2016
d01a72c
refactor(Dropdown): remove comment
levithomason Jun 8, 2016
d88ddfa
refactor(Dropdown): cleanup cruft
levithomason Jun 8, 2016
6346739
refactor(Dropdown): address PR comments
levithomason Jun 9, 2016
2ad2c81
docs(Dropdown): fix fetch/random buttons
levithomason Jun 9, 2016
4cd81a9
test(Dropdown): placeholder on multiple type
levithomason Jun 9, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 27 additions & 12 deletions docs/app/Examples/modules/Dropdown/Content/AsyncOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ const getOptions = () => _.times(5, () => {
export default class DropdownAsyncOptions extends Component {
componentWillMount() {
const options = getOptions()
const value = _.sample(options).value
this.setState({ isFetching: false, search: true, value, options })
this.setState({ isFetching: false, search: true, multiple: true, value: [], options })
}

handleChange = value => this.setState({ value })
handleChange = (e, value) => this.setState({ value })

// fake api call
fetchOptions = () => {
Expand All @@ -27,31 +26,47 @@ export default class DropdownAsyncOptions extends Component {
}, 500)
}

selectRandom = () => this.setState({ value: _.sample(this.state.options).value })
selectRandom = () => {
const { multiple, options } = this.state
const value = _.sample(options).value
this.setState({ value: multiple ? [value] : value })
}
toggleSearch = (e) => this.setState({ search: e.target.checked })
toggleMultiple = (e) => {
const { value } = this.state
// convert the value to/from an array
const newValue = e.target.checked ? _.compact([value]) : value[0]
this.setState({ multiple: e.target.checked, value: newValue })
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Since this example allows toggling the multiple prop, we need to convert the value to and from an array corresponding to the dropdown type.


render() {
const { options, isFetching, search, value } = this.state
const { multiple, options, isFetching, search, value } = this.state

return (
<div>
<Button onClick={this.fetchOptions}>Fetch</Button>
<Button onClick={this.selectRandom} disabled={_.isEmpty(options)}>Random</Button>
<p>
<Button onClick={this.fetchOptions}>Fetch</Button>
<Button onClick={this.selectRandom} disabled={_.isEmpty(options)}>Random</Button>
<label>
<input type='checkbox' checked={search} onChange={this.toggleSearch} /> Search
</label>
{' '}
<label>
<input type='checkbox' checked={multiple} onChange={this.toggleMultiple} /> Multiple
</label>
</p>
<Dropdown
options={options}
value={value}
placeholder='Add Users'
multiple={multiple}
search={search}
selection
fluid
onChange={this.handleChange}
disabled={isFetching}
loading={isFetching}
/>
{' '}
<label>
<input type='checkbox' checked={search} onChange={this.toggleSearch} />
{' '}Search
</label>
<pre>{JSON.stringify(this.state, null, 2)}</pre>
</div>
)
Expand Down
53 changes: 26 additions & 27 deletions src/elements/Label/Label.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/* eslint-disable valid-jsdoc, complexity */
import cx from 'classnames'
import React, { createElement, PropTypes } from 'react'
import React, { PropTypes } from 'react'
import createFragment from 'react-addons-create-fragment'

import META from '../../utils/Meta'
import * as sui from '../../utils/semanticUtils'
import { someChildType } from '../../utils/childrenUtils'
import {
getUnhandledProps,
iconPropRenderer,
imagePropRenderer,
useKeyOnly,
Expand All @@ -21,16 +21,13 @@ import Image from '../Image/Image'
*/
function Label(props) {
const {
attached, children, color, corner, className, circular, detail, detailLink, floating, horizontal, icon,
image, link, onClick, onClickDetail, onClickRemove, pointing, removable, ribbon, size, tag, text,
...rest,
attached, children, color, corner, className, circular, detail, detailLink, floating, horizontal,
icon, image, link, onClick, onDetailClick, onRemove, pointing, removable, ribbon, size, tag, text,
} = props

const handleClick = e => onClick && onClick(e, props)
const handleClickRemove = e => onClickRemove && onClickRemove(e, props)
const handleClickDetail = e => onClickDetail && onClickDetail(e, props)

const _component = image || link || onClick ? 'a' : 'div'
const handleRemove = e => onRemove && onRemove(e, props)
const handleDetailClick = e => onDetailClick && onDetailClick(e, props)

const classes = cx('sd-label ui',
size,
Expand All @@ -49,27 +46,29 @@ function Label(props) {
className
)

const _props = {
className: classes,
onClick: handleClick,
...rest,
}

const _detail = detail || detailLink
const detailComponent = (detailLink || onClickDetail) && 'a' || detail && 'div'
const DetailComponent = (detailLink || onDetailClick) && 'a' || 'div'
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to wrap the latter check in () for similar clarity to the initial expression.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not seeing where the extra parens can be added here. Here is another way of writing this:

let DetailComponent = 'div'
if (detailLink || onDetailClick) DetailComponent = 'a'


const _children = createFragment({
icon: iconPropRenderer(icon),
image: imagePropRenderer(image),
text,
children,
detail: _detail && createElement(detailComponent, { className: 'detail', onClick: handleClickDetail }, _detail),
remove: (removable || onClickRemove) && <Icon className='delete' onClick={handleClickRemove} />,
detail: detail && (
<DetailComponent className='detail' onClick={handleDetailClick}>{detail}</DetailComponent>
),
remove: (removable || onRemove) && (
<Icon className='delete' onClick={handleRemove} />
),
})

// Do not destructure createElement import
// react-docgen only recognizes a stateless component when React.createElement is returned
return React.createElement(_component, _props, _children)
const LabelComponent = image || link || onClick ? 'a' : 'div'
Copy link
Member

@dvdzkwsk dvdzkwsk Jun 8, 2016

Choose a reason for hiding this comment

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

Just stating that this is not immediately clear to me, or at least leaves me questioning if it works exactly as I expect. Take that as you will.

(image || link || onClick) ? 'a' : 'div'

const rest = getUnhandledProps(Label, props)

return (
<LabelComponent className={classes} onClick={handleClick} {...rest}>
{_children}
</LabelComponent>
)
}

Label._meta = {
Expand Down Expand Up @@ -106,7 +105,7 @@ Label.propTypes = {
/** Additional text with less emphasis. */
detail: PropTypes.string,

/** Same as detail but formatted as a link. */
/** Format the detail as a link. */
detailLink: PropTypes.string,
Copy link
Member Author

Choose a reason for hiding this comment

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

Updated this behavior to match the link behavior. Both just format the component as a link.


/** Format a label to align better alongside text. */
Expand Down Expand Up @@ -136,13 +135,13 @@ Label.propTypes = {
/** Adds the link style when present, called with (event, props). */
onClick: PropTypes.func,

/** Click callback for detail, called with (event, props). */
onClickDetail: PropTypes.func,
/** Click callback for detail, called with (event, props). Formats the detail as a link. */
onDetailClick: PropTypes.func,

/** Adds an "x" icon, called with (event, props) when "x" is clicked. */
onClickRemove: PropTypes.func,
onRemove: PropTypes.func,
Copy link
Member Author

Choose a reason for hiding this comment

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

Updated callbacks and click handlers to make more sense and be consistent with our others (on*Click)


/** Add an "x" icon that calls onClickRemove when clicked. */
/** Add an "x" icon that calls onRemove when clicked. */
removable: PropTypes.bool,

/** Point to content next to it. */
Expand Down
Loading