Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

feat(Dropdown): add clearable prop #885

Merged
merged 10 commits into from
Feb 13, 2019
Merged

feat(Dropdown): add clearable prop #885

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1cc2977
feat(Dropdown): add `clearable` prop
layershifter Feb 11, 2019
afa62f3
update steps
layershifter Feb 12, 2019
2f7bae2
add toc
layershifter Feb 12, 2019
8acec0f
fix focusing
layershifter Feb 12, 2019
346493d
fix `handleClear` method
layershifter Feb 12, 2019
b8c5aa6
extract styles
layershifter Feb 12, 2019
30967d0
use ShorthandCollection
layershifter Feb 12, 2019
3a604a8
Merge branches 'feat/dropdown-clearable' and 'master' of https://gith…
layershifter Feb 13, 2019
1c54f09
add changelog entry
layershifter Feb 13, 2019
230544e
use overrideProps
layershifter Feb 13, 2019
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
16 changes: 16 additions & 0 deletions docs/src/examples/components/Dropdown/Types/DropdownExampleClearable.shorthand.steps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { Dropdown } from '@stardust-ui/react'

const selectors = {
clearIndicator: `.${Dropdown.slotClassNames.clearIndicator}`,
triggerButton: `.${Dropdown.slotClassNames.triggerButton}`,
item: (itemIndex: number) => `.${Dropdown.slotClassNames.itemsList} li:nth-child(${itemIndex})`,
}

const steps = [
steps => steps.click(selectors.triggerButton).snapshot('Shows list'),
steps => steps.click(selectors.item(3)).snapshot('Selects an item'),
steps => steps.click(selectors.clearIndicator).snapshot('Clears the value'),
steps => steps.click(selectors.triggerButton).snapshot('Closes the list'),
]

export default steps
20 changes: 20 additions & 0 deletions docs/src/examples/components/Dropdown/Types/DropdownExampleClearable.shorthand.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { Dropdown } from '@stardust-ui/react'
import * as React from 'react'

const inputItems = [
'Bruce Wayne',
'Natasha Romanoff',
'Steven Strange',
'Alfred Pennyworth',
`Scarlett O'Hara`,
'Imperator Furiosa',
'Bruce Banner',
'Peter Parker',
'Selina Kyle',
]

const DropdownClearableExample = () => (
<Dropdown clearable items={inputItems} placeholder="Select your hero" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should add additional example for showing customization of the clearableIndicator?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we will have request for it, we can introduce it. For now, I am not sure that we need to add example for an every slot, it can make docs unusable ⛸

)

export default DropdownClearableExample
5 changes: 5 additions & 0 deletions docs/src/examples/components/Dropdown/Types/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ const Types = () => (
description="A dropdown can be searchable and allow a multiple selection."
examplePath="components/Dropdown/Types/DropdownExampleSearchMultiple"
/>
<ComponentExample
title="Clearable"
description="A dropdown can be clearable and let users remove their selection."
examplePath="components/Dropdown/Types/DropdownExampleClearable"
/>
<ComponentExample
title="Inline"
description="A dropdown can be used inline with text."
Expand Down
62 changes: 51 additions & 11 deletions packages/react/src/components/Dropdown/Dropdown.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ import DropdownSearchInput, { DropdownSearchInputProps } from './DropdownSearchI
import Button from '../Button/Button'
import { screenReaderContainerStyles } from '../../lib/accessibility/Styles/accessibilityStyles'
import ListItem from '../List/ListItem'
import Icon from '../Icon/Icon'

export interface DropdownSlotClassNames {
container: string
clearIndicator: string
triggerButton: string
itemsList: string
selectedItems: string
Expand All @@ -50,6 +52,12 @@ export interface DropdownProps extends UIComponentProps<DropdownProps, DropdownS
/** The index of the currently active selected item, if dropdown has a multiple selection. */
activeSelectedIndex?: number

/** A dropdown can be clearable and let users remove their selection. */
clearable?: boolean

/** A slot for a clearing indicator. */
clearIndicator?: ShorthandValue

/** The initial value for the index of the currently active selected item, in a multiple selection. */
defaultActiveSelectedIndex?: number

Expand Down Expand Up @@ -192,6 +200,8 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
content: false,
}),
activeSelectedIndex: PropTypes.number,
clearable: PropTypes.bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like that we have two new props in the API, but I don't have any better proposal for now...

clearIndicator: customPropTypes.itemShorthand,
defaultActiveSelectedIndex: PropTypes.number,
defaultSearchQuery: PropTypes.string,
defaultValue: PropTypes.oneOfType([
Expand Down Expand Up @@ -226,6 +236,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo

static defaultProps: DropdownProps = {
as: 'div',
clearIndicator: 'close',
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, don't assume that some icon exists, as in some theme they may not. That's the reason we introduced the Indicator component, instead of using the chevron icons inside the components. Can we use here some unicode char by default if no icon i provided?

Copy link
Member Author

Choose a reason for hiding this comment

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

https://github.com/stardust-ui/react/blob/master/packages/react/src/components/Input/Input.tsx#L149

Input component does the same thing actually. We can refactor them separately later.
The main issue with unicode chars, that I was not able to find a good symbol. Do you have a proposal about it?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am aware that the Input does the same thing, agreed to tackle this in separate PR. This brings me back to the fact that maybe we should have some icons in the base theme, at least for the things we need in the components (close, arrows etc..) Let's create separate issue for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Opened #896.

itemToString: item => {
if (!item || React.isValidElement(item)) {
return ''
Expand Down Expand Up @@ -263,8 +274,16 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
unhandledProps,
rtl,
}: RenderResultConfig<DropdownProps>) {
const { search, multiple, getA11yStatusMessage, itemToString, toggleIndicator } = this.props
const { defaultHighlightedIndex, searchQuery } = this.state
const {
clearable,
clearIndicator,
search,
multiple,
getA11yStatusMessage,
itemToString,
toggleIndicator,
} = this.props
const { defaultHighlightedIndex, searchQuery, value } = this.state

return (
<ElementType className={classes.root} {...unhandledProps}>
Expand Down Expand Up @@ -293,6 +312,8 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
{ refKey: 'innerRef' },
{ suppressRefError: true },
)
const showClearIndicator = clearable && !this.isValueEmpty(value)

return (
<Ref innerRef={innerRef}>
<div
Expand All @@ -315,13 +336,22 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
)
: this.renderTriggerButton(styles, rtl, getToggleButtonProps)}
</div>
{Indicator.create(toggleIndicator, {
defaultProps: {
direction: isOpen ? 'top' : 'bottom',
onClick: getToggleButtonProps().onClick,
styles: styles.toggleIndicator,
},
})}
{showClearIndicator
? Icon.create(clearIndicator, {
defaultProps: {
className: Dropdown.slotClassNames.clearIndicator,
onClick: this.handleClear,
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't onClick be on the override props? Then you can do all necessary for the Dropdown, and then invoke the user's onClick if provided.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch 👍

styles: styles.clearIndicator,
xSpacing: 'none',
},
})
: Indicator.create(toggleIndicator, {
defaultProps: {
direction: isOpen ? 'top' : 'bottom',
onClick: getToggleButtonProps().onClick,
styles: styles.toggleIndicator,
},
})}
{this.renderItemsList(
styles,
variables,
Expand Down Expand Up @@ -748,6 +778,12 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
}
}

private handleClear = () => {
const { multiple } = this.props

this.setState({ value: multiple ? [] : '' })
}

private handleContainerClick = () => {
this.tryFocusSearchInput()
}
Expand Down Expand Up @@ -932,9 +968,8 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo
*/
private getSelectedItemAsString = (value: ShorthandValue): string => {
const { itemToString, multiple, placeholder } = this.props
const isValueEmpty = _.isArray(value) ? value.length < 1 : !value

if (isValueEmpty) {
if (this.isValueEmpty(value)) {
return placeholder
}

Expand All @@ -944,10 +979,15 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo

return itemToString(value)
}

private isValueEmpty = (value: ShorthandValue | ShorthandValue[]) => {
return _.isArray(value) ? value.length < 1 : !value
}
}

Dropdown.slotClassNames = {
container: `${Dropdown.className}__container`,
clearIndicator: `${Dropdown.className}__clear-indicator`,
triggerButton: `${Dropdown.className}__trigger-button`,
itemsList: `${Dropdown.className}__items-list`,
selectedItems: `${Dropdown.className}__selected-items`,
Expand Down
16 changes: 16 additions & 0 deletions packages/react/src/themes/teams/components/Dropdown/dropdownStyles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,22 @@ const dropdownStyles: ComponentSlotStylesInput<DropdownPropsAndState, DropdownVa
}),
}),

clearIndicator: ({ variables: v }): ICSSInJSStyle => ({
alignItems: 'center',
display: 'flex',
justifyContent: 'center',

backgroundColor: 'transparent',
cursor: 'pointer',
userSelect: 'none',

margin: 0,
position: 'absolute',
right: pxToRem(5),
height: v.toggleIndicatorSize,
width: v.toggleIndicatorSize,
}),

container: ({ props: p, variables: v }): ICSSInJSStyle => ({
display: 'flex',
flexWrap: 'wrap',
Expand Down