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

Add closeOnSelect prop to SelectMenu #588

Merged
merged 2 commits into from
Jun 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions docs/src/pages/components/select-menu.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@ and uses `react-tiny-virtual-list` for the rendering of the virtualized list of
The `SelectMenu` is unopinionated in how many items are selected in the list.
Pass an array to the `selected` prop to select more items.

## Close on select

The `SelectMenu` by default will stay open when an option is selected.
This can be configured so that the menu closes on selection.
This will not apply for Multiselect menus.

## Options prop structure

```js static
Expand Down
12 changes: 11 additions & 1 deletion src/select-menu/src/OptionsList.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,17 @@ export default class OptionsList extends PureComponent {
*/
isMultiSelect: PropTypes.bool,

/**
* When true, menu closes on option selection.
*/
closeOnSelect: PropTypes.bool,

/**
* This holds the values of the options
*/
selected: PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.string, PropTypes.number])),
selected: PropTypes.arrayOf(
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you configure your editor? I don't think these lines should be formatted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using VS Code. It's picking up the prettier settings in the package.json and as there is no printWidth setting is going with the default of 80. This line is longer than that, which is why it is being formatted.
I've left it as is as the prettier settings would need to explicitly state a larger printWidth. Is this ok? Would you rather I change it back?

Copy link
Contributor

Choose a reason for hiding this comment

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

Weird. Maybe nobody had touched the file in a while. Thanks for checking!

PropTypes.oneOfType([PropTypes.string, PropTypes.number])
),
onSelect: PropTypes.func,
onDeselect: PropTypes.func,
onFilterChange: PropTypes.func,
Expand Down Expand Up @@ -201,6 +208,9 @@ export default class OptionsList extends PureComponent {

handleSelect = item => {
this.props.onSelect(item)
if (!this.props.isMultiSelect && this.props.closeOnSelect) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I realized that hitting the Enter key has a similar behavior (closing the menu). It makes me wonder if we should also have the check for multiSelect and closeOnSelect in the handleEnter method (you can always close the menu by hitting ESC in that case). Doing this would be a change of contract in the interface. I'm not sure if we want to do that now.

@mshwery - would love some thoughts!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could save that for a separate PR if the current behavior is problematic. I think it's fine to leave alone for now TBH, though I agree with your idea!

this.props.close()
}
}

handleDeselect = item => {
Expand Down
12 changes: 10 additions & 2 deletions src/select-menu/src/SelectMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,12 @@ export default class SelectMenu extends PureComponent {
* Can be a function that returns a node, or a node itself, that is
* rendered instead of the options list when there are no options.
*/
emptyView: PropTypes.oneOfType([PropTypes.func, PropTypes.node])
emptyView: PropTypes.oneOfType([PropTypes.func, PropTypes.node]),

/*
* When true, menu closes on option selection.
*/
closeOnSelect: PropTypes.bool
}

static defaultProps = {
Expand All @@ -116,7 +121,8 @@ export default class SelectMenu extends PureComponent {
position: Position.BOTTOM_LEFT,
isMultiSelect: false,
filterPlaceholder: 'Filter...',
filterIcon: 'search'
filterIcon: 'search',
closeOnSelect: false
}

getDetailView = (close, detailView) => {
Expand Down Expand Up @@ -163,6 +169,7 @@ export default class SelectMenu extends PureComponent {
emptyView,
titleView,
isMultiSelect,
closeOnSelect,
...props
} = this.props

Expand Down Expand Up @@ -196,6 +203,7 @@ export default class SelectMenu extends PureComponent {
close={close}
{...this.getDetailView(close, detailView)}
{...this.getEmptyView(close, emptyView)}
closeOnSelect={closeOnSelect}
/>
)}
{...props}
Expand Down
9 changes: 8 additions & 1 deletion src/select-menu/src/SelectMenuContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ export default class SelectMenuContent extends PureComponent {
*/
isMultiSelect: PropTypes.bool,

/*
* When true, menu closes on option selection.
*/
closeOnSelect: PropTypes.bool,

/**
* Node that is placed in the header section, above the options.
*/
Expand Down Expand Up @@ -84,7 +89,8 @@ export default class SelectMenuContent extends PureComponent {
titleView,
detailView,
emptyView,
isMultiSelect
isMultiSelect,
closeOnSelect
} = this.props

const headerHeight = 40
Expand Down Expand Up @@ -114,6 +120,7 @@ export default class SelectMenuContent extends PureComponent {
options={options}
isMultiSelect={isMultiSelect}
close={close}
closeOnSelect={closeOnSelect}
{...listProps}
/>
)}
Expand Down
13 changes: 13 additions & 0 deletions src/select-menu/stories/index.stories.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,19 @@ storiesOf('select-menu', module).add('SelectMenu', () => (
</SelectMenu>
)}
</Manager>
<Manager>
{({ setState, state }) => (
<SelectMenu
title="Select name"
options={options}
selected={state.selected}
onSelect={item => setState({ selected: item.value })}
closeOnSelect
>
<Button>Menu will close on select</Button>
</SelectMenu>
)}
</Manager>
<Manager>
{({ setState, state }) => (
<Pane display="inline-block">
Expand Down