Material-ui: [Select] Remove the React.cloneElement usage

[bgits]

I have a form with two text fields. The first text field works as expected in selecting and closing on click, but the second does and not have have gone in to very hacky territory in order to try to get it to work but it does not.

• [X ] This is not a v0.x issue.
• [X ] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

The menu option should blur on click

Current Behavior 😯

It stays open and does not trigger a handleChange or handleBlur event in the parent. In the code sandbox it doesn't even open.

Steps to Reproduce 🕹

Link: https://codesandbox.io/s/94ql31xn9p

Context 🔦

I need wrap the menu in an EnhancedComponent in order to Observe certain fields on the model (async)

Your Environment 🌎

| Tech | Version |
|--------------|---------|
| Material-UI | v3.6.0 |
| React | v16.8.0 |
| Browser | chrome 72.0.3626.121 |
| TypeScript | no |
| etc. | |

[oliviertassinari]

@bgits Your codesandbox crash. I can't use it.

[bgits]

@bgits Your codesandbox crash. I can't use it.

I think it maybe related to the bug based on the console error when clicking. I posted this as well which is the code snippet related. https://spectrum.chat/material-ui/help/textfield-does-not-blur-when-menuitem-is-clicked~58c7e043-7ec2-4bb4-ac97-f36dc4002a02

[joshwooding]

@bgits you aren’t destructuring props in EnhancedMenuItems.

[vitkon]

yep, should be const EnhancedMenuItems = ({ profiles }) => (

[oliviertassinari]

@bgits You can't create an intermediary component. We rely on the React.cloneElement() API:

        <TextField
          className={classes.textField}
          id="delegateProfile"
          name="delegateProfile"
          select
          label="Select Delegate Profile"
          placeholder="Select Delegate Profile"
          margin="normal"
          variant="outlined"
          onChange={console.log}
          onBlur={console.log}
          value={""}
        >
          {profiles.map(profile => (
            <MenuItem
              style={{ display: "flex", alignItems: "center" }}
              key={profile.name}
              value={profile.name}
            >
              {profile.name}
            </MenuItem>
          ))}
        </TextField>
      </div>

https://codesandbox.io/s/wz72o1v365

We are working on the problem. @joshwooding has started to remove the React.cloneElement() requirement for a few of our components. But It's not always feasible, we have to evaluate the bundle size increase vs the simplicity gain it provides to people.

[bgits]

@bgits You can't create an intermediary component. We rely on the React.cloneElement() API:

https://codesandbox.io/s/wz72o1v365

We are working on the problem. @joshwooding has started to remove the React.cloneElement() requirement for a few of our components. But It's not always feasible, we have to evaluate the bundle size increase vs the simplicity gain it provides to people.

So for now the only workaround is to have the data prefetched for each MenuItem and display them as a direct child?

[oliviertassinari]

@bgits Yes. Alternatively, we could provide a different API, it might be our best hope actually. I think that we should wait for more people upvotes and use cases before prioritizing the problem.

[oliviertassinari]

A new constraint to take into account, the usage of cloneElement prevents virtualization: #16364.

[eps1lon]

With a priority on customization we should use context by default. Even if it looks like a overengineered cloneElement for the basic case it prevents so much subtle bugs with custom components.