search

Orasund / elm-ui-widgets

Collection of reusable views for elm-ui.
https://orasund.github.io/elm-ui-widgets/
BSD 3-Clause "New" or "Revised" License
85 stars 11 forks source link

Add PasswordInput widget #55

Closed cdevienne closed 3 years ago

Orasund commented 3 years ago

I basically copied TextInput, removing its "chips" attribute and adding the "show" attribute. Adding "chips" back may be better, I would welcome feedback.

Thanks for pointing this out, somehow the fact that Elm-UI even has this function went past me. I must think about what the best solution for this could be. Because it does not only apply to passwords, also to the username, email, search field.

You might be right that we do not want to allow chips to be part of a password field. In that case it might be best to add a second textInput widget without chips but with something like "roles".

I will leave your PR untouched for know. Until I know how I want to deal with this and until I have some spare time to try a few things out.

Until then please be patient.

cdevienne commented 3 years ago

I would keep the API close to the Elm-UI one and avoid having a 'Role' argument. The password widget could have a show/hide action, may be not as a chip but a simple icon. The username, email or search fields could take a TextInputStyleand TextInput same as 'text'.

Orasund commented 3 years ago

Okey, so here is my current take on this:

Elm-ui has 6 autofill inputs that should be implemented at some point: username, newPassword, currentPassword, email, search, spellChecked.

Your Issue is about newPassword and currentPassword, and your PR implements them perfectly. (with one small exception, see PR).

My current plan is to slowly add these autofill functions as independent widgets. I'd like to see them in practice before Version 4.0.0 comes around. At that point we can still decide to group them together into something like a autoFillInput or rename the current textInput into chipInput.