[TextField] Add eye to password type

[stuk88]

Description

The password field in android contains an eye icon that when clicked shows the password (until released)
Thought it should have been implemented in the material ui too.

Images & references

https://medium.com/@moyinoluwa/password-visibility-toggle-android-support-library-revision-24-2-0-98b422087e5a

Versions

• Material-UI: 0.17.0
• React: 15.4.2
• Browser: Chrome 57.0.2987.133 (64-bit)

[supra28]

Can use this in the meantime

[DragonEmperorG]

Are there any official solutions to this problem of 1.0.0-beta.2?

[kybarg]

@DragonEmperorG You can use composable field https://material-ui-1dab0.firebaseapp.com/component-demos/text-fields#components and add icon into your layout

[leMaik]

I'm starting to work on a material-ui@1.0.0 port of our component right now and I'll send a PR as soon as it's ready. This really should be included in material-ui itself. 😎

[leMaik]

@oliviertassinari If I add this to the TextField component itself, this would require to give it a state (toggle on/off) and thus make it a class or requiring the user to provide showPassword and onTogglePassword props, which is both undesireable. 🤔

Should this become a separate component instead?

[oliviertassinari]

or requiring the user to provide showPassword and onTogglePassword props

@leMaik I prefer this option over the state one, but I guess the best would be to have both options.
I think that the feature should be implemented at the Input level, not the TextField one.
We could implement the PasswordInput component as a slim wrapper.
How should we call this component?

[oliviertassinari]

I'm starting to work on a material-ui@1.0.0 port of our component

🎉

[leMaik]

@oliviertassinari I'd go with PasswordInput for a slim Input wrapper that adds showPassword and onTogglePassword and the toggle button. And then add a PasswordField for the stateful password field that "just works".

Edit: Right now, I don't see a way to inject any component into an Input component. Maybe we should allow it to have children? I don't want to have the toggle button hard-coded there…

[oliviertassinari]

Some more thoughts. Maybe we can support both controled and uncontroled behavior. I'm not sure we should introduce two new components. I'm fine with having people building the PasswordField from lower level components. We have 4 options for our PasswordInput:

1. Higher order Component
2. Component with dependency injection (cloneElement)
3. Component with hidden composition on Input
4. Compound component

I like option 2. What do you think?

[leMaik]

I'm fine with having people building the PasswordField from lower level components.

A password field is something that is so basic that users shouldn't be required to build it themselves. That would just end up in me building a new material-ui-password-field and using that everywhere. 😄

Regarding the options:

1. What? 😮
2. If you tell me how to insert a button into Input by using cloneElement, I'm all in. The Input still won't be able to render children.
3. That'd be my favorite, but see above.
4. ?

I don't want to over-engineer this. After all, it's just a button beside a text field. 😉

Edit: I would say that controlled behavior for password toggling isn't required at all.

[oliviertassinari]

I would say that controlled behavior for password toggling isn't required at all.

Let's start without it then.

A password field is something that is so basic that users shouldn't be required to build it themselves.

They will have the documentation demo, everybody is copy-pasting the docs demo. I don't know what's the best pattern but I want to avoid duplication and maximize flexibility.

1. withPassword(Input)

<PasswordInput>
  <Input />
</PasswordInput>

3. Like 2 but user don't have access to the Input component, the would need a proxy to change it.

<PasswordInput>
  {(props) => <Input {...props} />}
</PasswordInput>

[leMaik]

I'll give it a try later this week / next week. Now that I see the approaches (I knew what a HOC is, I just wondered why it would be useful here), I strongly prefer the second one, but again: Why should a user need more than one component to get something as basic as a password field?

I want a usage like this to get a toggle-able password field:

<PasswordField />

Just because the code can be copy-pasted from the docs doesn't mean that it shouldn't be as simple to use as possible. 😄

[oliviertassinari]

@leMaik I think that both 2 and 3 are potentially good candidates. Picking one over the other depends on the use cases we want to support, your call.

Why should a user need more than one component to get something as basic as a password field?

We have broken the TextField into multiples components in order to support multiples use cases. input with/without label, with/without helper message, and later maybe with a selectfield, datepicker, timepicker.

That would just end up in me building a new material-ui-password-field and using that everywhere.

If that's something you can do without touching lower-level components, that something I would personally label as a success and want to see more :).

[leMaik]

I'll try to come up with a way to extend Input with a toggle button for the password visibility. This would mean that #6693 (comment) could easily be done by users of material-ui.

Maybe I'm just still too much in the 0.x just-configure-it-by-setting-all-the-props mood...

[oliviertassinari]

Maybe I'm just still too much in the 0.x just-configure-it-by-setting-all-the-props mood...

@leMaik Yeah, we have definitely made a strategic switch.

1. build lower-level components first, expose that
2. see potential abstraction win and expose them

We are 80% into 1. and 20% into 2. If you find an elegant way to expose the password feature into the TextField component, let's go for it, but no rush, it can wait.

[oliviertassinari]

@leMaik Thanks for the discussion!

[leMaik]

I was thinking about adding the toggle state-functionality directly to the TextField and adding a {type === 'password' && <PasswordToggle onToggle={() => this.setState({ showPassword: !this.state.showPassword })} showPassword={this.state.showPassword} />}. This way, TextField would be the batteries-included component and PasswordToggle could be used by those who want to have full control over their component. 🤔

@oliviertassinari Thank you, always happy to have some exchange on how to do things in React.

[oliviertassinari]

@leMaik This means bloating the TextField with some extra code that users might not need. Yeah, I can't see better alternatives, that's where the TextField tradeoff reach his limitations. Sounds good to me as long as users can also escape the TextField abstraction if they need fine grain tunning.

[oliviertassinari]

Hence the PasswordInput component extending the Input one :). Sounds like we have a plan, I can't wait to see what you will come up with.

[eyn]

In line with the compostability of fields have you considered having an <InputAction>?

So a PasswordField would be

        <FormControl className={classes.formControl}>
          <InputLabel htmlFor="name-helper">Name</InputLabel>
          <InputAction>
            <IconButton onClick={this.toggleShowPassword}>
              {this.state.showPassword?<Hide />:<Eye />}
            </IconButton>
          </InputAction>
          <Input id="name-helper"  value={this.state.name} onChange={this.handleChange} />
          <FormHelperText>Some important helper text</FormHelperText>
        </FormControl>

This could then be reused for dropdowns etc..

[oliviertassinari]

@eyn The InputAction API looks good. You can also check some different implementations:

• https://codesandbox.io/s/LWMA9QZj
• https://codesandbox.io/s/QMGq4kAY

Both seem to be using the following pattern:

    <div className={...}>
      <TextField />
      <InputAction className={...} />
    </div>