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

[Select] Allow adding non-option children #26356

Open
1 task done
Floriferous opened this issue May 18, 2021 · 6 comments
Open
1 task done

[Select] Allow adding non-option children #26356

Floriferous opened this issue May 18, 2021 · 6 comments
Labels
component: select This is the name of the generic UI component, not the React module! new feature New feature or request waiting for 👍 Waiting for upvotes

Comments

@Floriferous
Copy link
Contributor

Many of our first-time users get confused by multi-selects, as they don't often quickly understand that they need to click outside of the select menu to close it so they can continue filling in their form. This makes onboarding new users with multi-selects a bit uncomfortable.

We'd like to add a visible button to the Menu that lets them close it, however it seems like this isn't trivial to do, as all the children of the Menu trigger the handleItemClick function, adding undefined to the array of values.

Here's an example of what I'm trying to achieve, another option would be to have an IconButton top-right that's clickable for example:
https://codesandbox.io/s/dry-wind-jdfbe

Screenshot 2021-05-18 at 10 26 11

As you can see in the demo, the "Close" label is added to the values array as undefined.

  • I have searched the issues of this repository and believe that this is not a duplicate.
@Floriferous Floriferous added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label May 18, 2021
@eps1lon
Copy link
Member

eps1lon commented Jun 30, 2021

Thanks for the feedback.

as they don't often quickly understand that they need to click outside of the select menu to close it so they can continue filling in their form.

I totally get that. I would even go further and say that it doesn't make any sense that the only affordance for closing is clicking outside. Especially without any signifier.

Here's an example of what I'm trying to achieve, another option would be to have an IconButton top-right that's clickable for example:
codesandbox.io/s/dry-wind-jdfbe

I wouldn't recommend this approach to begin with since it already has broken behavior (clicking it will be like selecting a value).

The good news is that you can implement the behavior right now: https://codesandbox.io/s/select-multi-with-close-button-j8to8

Though this isn't as straight forward as it should be here. So the problem is how we can provide a better API. This API needs to make sure that the position of the button is also customizeable.

@mnajdova The customization in https://codesandbox.io/s/dry-wind-jdfbe?file=/src/Demo.tsx:1073-1403 is slightly broken since it overrides existing styles completely due to https://github.com/mui-org/material-ui/blob/1406f5137b57a14c2623affd6bac52d08fd17e1b/packages/material-ui/src/Menu/Menu.js#L169-L170. I think this used to be possible since we didn't pass a custom component. But now we do so the question how one would compose component props.

@eps1lon eps1lon added component: select This is the name of the generic UI component, not the React module! new feature New feature or request and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 30, 2021
@aizela
Copy link

aizela commented Jul 16, 2021

Thanks for the feedback.

as they don't often quickly understand that they need to click outside of the select menu to close it so they can continue filling in their form.

I totally get that. I would even go further and say that it doesn't make any sense that the only affordance for closing is clicking outside. Especially without any signifier.

Here's an example of what I'm trying to achieve, another option would be to have an IconButton top-right that's clickable for example:
codesandbox.io/s/dry-wind-jdfbe

I wouldn't recommend this approach to begin with since it already has broken behavior (clicking it will be like selecting a value).

The good news is that you can implement the behavior right now: https://codesandbox.io/s/select-multi-with-close-button-j8to8

Though this isn't as straight forward as it should be here. So the problem is how we can provide a better API. This API needs to make sure that the position of the button is also customizeable.

@mnajdova The customization in https://codesandbox.io/s/dry-wind-jdfbe?file=/src/Demo.tsx:1073-1403 is slightly broken since it overrides existing styles completely due to

https://github.com/mui-org/material-ui/blob/1406f5137b57a14c2623affd6bac52d08fd17e1b/packages/material-ui/src/Menu/Menu.js#L169-L170

. I think this used to be possible since we didn't pass a custom component. But now we do so the question how one would compose component props.

According to the code that you share in codesandbox, when I click the [none], [object Object] appears in the input.
and when i add <MenuItem value="">clearAll</MenuItem> in my Select,it's undefined,Neither of them has cleared all of the value

@aizela
Copy link

aizela commented Jul 16, 2021

const [names, setNames] = useState([])
const nameList= [
    'Clear',
    'CT',
    'DX',
    'MI',
    'CR',
    'ES'
  ]

 const handleSelect = (e) => {
    const value = e.target.value
    value.filter((item) => {
      if (item !== 'Clear') {
        return setNames(e.target.value)
      } else {
        return setNames([])
      }
    })
  }


                   <Select
                      labelId="mutiple-checkbox-label"
                      id="mutiple-checkbox"
                      multiple
                      value={names}
                      onChange={handleSelect}
                      input={<Input />}
                      renderValue={(selected) => selected.join(', ')}>
                      {apparatuses.map((item) => (
                        <MenuItem key={item} value={item}>
                          {item !== 'Clear' ? (
                            <Checkbox checked={names.indexOf(item) > -1} />
                          ) : null}
                          <ListItemText primary={item} />
                        </MenuItem>
                      ))}
                    </Select>

by this, it worked. Although different from my original scenario

@jackiesweet
Copy link

Is this going to get any love?

@Jovells
Copy link

Jovells commented Aug 1, 2023

bump

@aprilmintacpineda
Copy link

aprilmintacpineda commented Nov 1, 2023

I think I sorta figured, it comes down to how you generate the menu items, essentially what you want is not to wrap it on an element other than the select, i.e., they have to be direct descendants of the select component. So instead of doing:

<select {...props}>
  <>
    {menuItems}
    <Button ...props>Fetch more</Button>
  </>
</select>

You should do

<select {...props}>
    {menuItems}
    <Button {...props}>Fetch more</Button>
</select>

And if you have conditions and generate them through a loop, you should do:

<select {...props}>
    {
        someCondition
        ? renderSomethingElse()
        : items.map(() => ...)
                .concat(<Button {...props}>Fetch more</Button>)
    }
</select>

The .concat is the solution that allows you to add additional child in the end without needing to wrap them in fragments.

Caveat:

A caveat I noticed is when I click the button I added to the children, the dropdown options will close, even when you add ev.preventDefault(); and/or ev.stopPropagation();. I haven't found a way to get around this without needing to handle the open state of the select component myself.

EDIT: To get around the caveat, wrap the button on a parent component like Box or something, like so:

<select {...props}>
    {
        someCondition
        ? renderSomethingElse()
        : items.map(() => ...)
                .concat(
                    <Box key={-1}>
                        <Button {...props}>Fetch more</Button>
                    </Box>
                )
    }
</select>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: select This is the name of the generic UI component, not the React module! new feature New feature or request waiting for 👍 Waiting for upvotes
Projects
None yet
Development

No branches or pull requests

8 participants