-
Notifications
You must be signed in to change notification settings - Fork 54
feat(Dropdown): add clearable
prop
#885
Changes from 7 commits
1cc2977
afa62f3
2f7bae2
8acec0f
346493d
b8c5aa6
30967d0
3a604a8
1c54f09
230544e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
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 = [ | ||
layershifter marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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'), | ||
] | ||
|
||
export default steps |
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" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should add additional example for showing customization of the There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,7 @@ import { | |
ShorthandRenderFunction, | ||
ShorthandValue, | ||
ComponentEventHandler, | ||
ShorthandCollection, | ||
} from '../../types' | ||
import { ComponentSlotStylesInput, ComponentVariablesInput } from '../../themes/types' | ||
import Downshift, { | ||
|
@@ -38,9 +39,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 | ||
|
@@ -50,14 +53,20 @@ 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 | ||
|
||
/** The initial value for the search query, if the dropdown is also a search. */ | ||
defaultSearchQuery?: string | ||
|
||
/** The initial value or value array, if the array has multiple selection. */ | ||
defaultValue?: ShorthandValue | ShorthandValue[] | ||
defaultValue?: ShorthandValue | ShorthandCollection | ||
|
||
/** A dropdown can take the width of its container. */ | ||
fluid?: boolean | ||
|
@@ -86,7 +95,7 @@ export interface DropdownProps extends UIComponentProps<DropdownProps, DropdownS | |
inline?: boolean | ||
|
||
/** Array of props for generating list options (Dropdown.Item[]) and selected item labels(Dropdown.SelectedItem[]), if it's a multiple selection. */ | ||
items?: ShorthandValue[] | ||
items?: ShorthandCollection | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this changed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Never mind I was reading the changes mixed. Good change! 👍 |
||
|
||
/** | ||
* Function to be passed to create string from selected item, if it's a shorthand object. Used when dropdown also has a search function. | ||
|
@@ -141,7 +150,7 @@ export interface DropdownProps extends UIComponentProps<DropdownProps, DropdownS | |
renderSelectedItem?: ShorthandRenderFunction | ||
|
||
/** A dropdown can have a search field instead of trigger button. Can receive a custom search function that will replace the default equivalent. */ | ||
search?: boolean | ((items: ShorthandValue[], searchQuery: string) => ShorthandValue[]) | ||
search?: boolean | ((items: ShorthandCollection, searchQuery: string) => ShorthandCollection) | ||
|
||
/** Component for the search input query. */ | ||
searchInput?: ShorthandValue | ||
|
@@ -156,7 +165,7 @@ export interface DropdownProps extends UIComponentProps<DropdownProps, DropdownS | |
triggerButton?: ShorthandValue | ||
|
||
/** Sets currently selected value(s) (controlled mode). */ | ||
value?: ShorthandValue | ShorthandValue[] | ||
value?: ShorthandValue | ShorthandCollection | ||
} | ||
|
||
export interface DropdownState { | ||
|
@@ -165,7 +174,7 @@ export interface DropdownState { | |
focused: boolean | ||
isOpen?: boolean | ||
searchQuery?: string | ||
value: ShorthandValue | ShorthandValue[] | ||
value: ShorthandValue | ShorthandCollection | ||
} | ||
|
||
/** | ||
|
@@ -192,6 +201,8 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
content: false, | ||
}), | ||
activeSelectedIndex: PropTypes.number, | ||
clearable: PropTypes.bool, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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([ | ||
|
@@ -226,6 +237,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
|
||
static defaultProps: DropdownProps = { | ||
as: 'div', | ||
clearIndicator: 'close', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Opened #896. |
||
itemToString: item => { | ||
if (!item || React.isValidElement(item)) { | ||
return '' | ||
|
@@ -263,8 +275,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}> | ||
|
@@ -293,6 +313,8 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
{ refKey: 'innerRef' }, | ||
{ suppressRefError: true }, | ||
) | ||
const showClearIndicator = clearable && !this.isValueEmpty(value) | ||
|
||
return ( | ||
<Ref innerRef={innerRef}> | ||
<div | ||
|
@@ -315,13 +337,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, | ||
layershifter marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
@@ -392,7 +423,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
const { searchQuery, value } = this.state | ||
|
||
const noPlaceholder = | ||
searchQuery.length > 0 || (multiple && (value as ShorthandValue[]).length > 0) | ||
searchQuery.length > 0 || (multiple && (value as ShorthandCollection).length > 0) | ||
|
||
return DropdownSearchInput.create(searchInput || {}, { | ||
defaultProps: { | ||
|
@@ -510,7 +541,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
|
||
private renderSelectedItems(variables, rtl: boolean) { | ||
const { renderSelectedItem } = this.props | ||
const value = this.state.value as ShorthandValue[] | ||
const value = this.state.value as ShorthandCollection | ||
|
||
if (value.length === 0) { | ||
return null | ||
|
@@ -570,10 +601,10 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
} | ||
} | ||
|
||
private getItemsFilteredBySearchQuery = (): ShorthandValue[] => { | ||
private getItemsFilteredBySearchQuery = (): ShorthandCollection => { | ||
const { items, itemToString, multiple, search } = this.props | ||
const { searchQuery, value } = this.state | ||
const filteredItems = multiple ? _.difference(items, value as ShorthandValue[]) : items | ||
const filteredItems = multiple ? _.difference(items, value as ShorthandCollection) : items | ||
|
||
if (search) { | ||
if (_.isFunction(search)) { | ||
|
@@ -627,7 +658,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
this.handleSelectedItemRemove(e, item, predefinedProps, DropdownSelectedItemProps) | ||
}, | ||
onClick: (e: React.SyntheticEvent, DropdownSelectedItemProps: DropdownSelectedItemProps) => { | ||
const { value } = this.state as { value: ShorthandValue[] } | ||
const { value } = this.state as { value: ShorthandCollection } | ||
this.trySetState({ | ||
activeSelectedIndex: value.indexOf(item), | ||
}) | ||
|
@@ -729,7 +760,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
) { | ||
return | ||
} | ||
const { value } = this.state as { value: ShorthandValue[] } | ||
const { value } = this.state as { value: ShorthandCollection } | ||
if (value.length > 0) { | ||
this.trySetState({ activeSelectedIndex: value.length - 1 }) | ||
} | ||
|
@@ -742,12 +773,21 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
if ( | ||
multiple && | ||
(searchQuery === '' || this.inputRef.current.selectionStart === 0) && | ||
(value as ShorthandValue[]).length > 0 | ||
(value as ShorthandCollection).length > 0 | ||
) { | ||
this.removeItemFromValue() | ||
} | ||
} | ||
|
||
private handleClear = () => { | ||
const initialState = this.getInitialAutoControlledState(this.props) | ||
|
||
this.setState({ value: initialState.value }) | ||
|
||
this.tryFocusSearchInput() | ||
this.tryFocusTriggerButton() | ||
} | ||
|
||
private handleContainerClick = () => { | ||
this.tryFocusSearchInput() | ||
} | ||
|
@@ -797,7 +837,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
private handleSelectedChange = (item: ShorthandValue) => { | ||
const { items, multiple, getA11ySelectionMessage } = this.props | ||
const newState = { | ||
value: multiple ? [...(this.state.value as ShorthandValue[]), item] : item, | ||
value: multiple ? [...(this.state.value as ShorthandCollection), item] : item, | ||
searchQuery: this.getSelectedItemAsString(item), | ||
} | ||
|
||
|
@@ -834,7 +874,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
) { | ||
const { activeSelectedIndex, value } = this.state as { | ||
activeSelectedIndex: number | ||
value: ShorthandValue[] | ||
value: ShorthandCollection | ||
} | ||
const previousKey = rtl ? keyboardKey.ArrowRight : keyboardKey.ArrowLeft | ||
const nextKey = rtl ? keyboardKey.ArrowLeft : keyboardKey.ArrowRight | ||
|
@@ -894,7 +934,7 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
|
||
private removeItemFromValue(item?: ShorthandValue) { | ||
const { getA11ySelectionMessage } = this.props | ||
let value = this.state.value as ShorthandValue[] | ||
let value = this.state.value as ShorthandCollection | ||
let poppedItem = item | ||
|
||
if (poppedItem) { | ||
|
@@ -932,9 +972,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 | ||
} | ||
|
||
|
@@ -944,10 +983,15 @@ class Dropdown extends AutoControlledComponent<Extendable<DropdownProps>, Dropdo | |
|
||
return itemToString(value) | ||
} | ||
|
||
private isValueEmpty = (value: ShorthandValue | ShorthandCollection) => { | ||
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`, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these changes here? Bad merge?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't generated before 🤔