Add password visibility toggle

[simison]

A simple button to allow toggling password visibility at the sign-in
form.

Because life's too short to waste time re-typing passwords.

[mleanos]

LGTM

[jloveland]

looks good. would it make sense to add this to the following?

• modules/users/client/views/password/reset-password.client.view.html
• modules/users/client/views/settings/change-password.client.view.html
• modules/users/client/views/authentication/signup.client.view.html

[simison]

@jloveland true, I'll add.

Then UI wise it would make sense to remove password double check when changing the password:

— just like it's now missing from the signup form, too. Thoughts?

[mleanos]

@simison I think that's a very good point. If the visibility toggle is on the Change Password form, then there's no need to have the Verify Password field. Although, it might be fine to leave it.

Either way, I think it would be nice to have the toggle on each of the forms that have the Password input.

[simison]

Added this to those other pages but didn't remove verifications.

[lirantal]

@simison you had me at life's too short to waste them on re-typing passwords line lol

[lirantal]

@simison kudus on the screenshots, thanks for making it an example for every other PR. Keep it up.
@ilanbiala I'm ok with this

[simison]

👍

@lirantal @ilanbiala do you recon it would make sense to remove double-checking passwords and should it be included at this PR, or separately? (I'd prefer small PRs)

[simison]

(squashed)

[codydaig]

LGTM

[lirantal]

I think we can leave the double passwords checking for now, unless there's some UX evidence that it should be removed.

[simison]

@lirantal some food for thought:

• http://ux.stackexchange.com/questions/20953/why-should-we-ask-the-password-twice-during-registration
• http://ux.stackexchange.com/questions/484/is-a-repeat-password-field-necessary-in-a-signup-page

Basically the problem that asking password twice is trying to solve, gets already solved with the visibility toggle.

[lirantal]

@mleanos I scanned through it, and I'm generally ok with continuing with this change

[mleanos]

@lirantal Did you tag the wrong person in your last comment? I don't think this was waiting for anything from my end.

[lirantal]

my bad :)
@simison I pinged you...

[ilanbiala]

@simison since this seems somewhat redundant, can you make it a directive with an option to have the toggle and add tests for the directive?

[simison]

@ilanbiala I was thinking of a directive at first but since most of it is just some HTML and the markup can be very specific depending on visuals/css framework/icon framework (here it's very tight Bootstrap+Glyphicons), it didn't make sense to me. The rest is just one variable and two if-else's.

Now that you asked... maybe it would serve as an example DOM manipulating directive.

I really like small and focused modules...

[ilanbiala]

@simison any update on the directive refactor?

[ilanbiala]

@simison any update on this or should this be moved back?

[ilanbiala]

Moved to 0.4.x so we don't hold up 0.4.2 for this unless we make progress.

[rhutchison]

@simison any luck changing this to a directive? I like the idea and I am in favor of removing the verify check.

I would prefer if the show/hide is per-input (current and new) with verify removed.

[ilanbiala]

Moving to backlog so this doesn't hold up 0.5.0 unless @simison or someone else wants to finish up the PR.

[lirantal]

@simison @mleanos would you like to see this through please?

[mleanos]

@lirantal If @simison doesn't have time, I'll pick this back up.

[simison]

@mleanos feel free to, I'm pretty much booked next ~month or so.

[lirantal]

👍

[ilanbiala]

@mleanos I'll review it and take a look, let me know which PR it's in.

[lirantal]

@simison and @mleanos would you guys like to bring this PR up to date and we'll merge it in?

[lirantal]

@mleanos another ping in-case you missed it in all the previous update.
If you'd like to push this in...

[mleanos]

@simison @lirantal I'll work on getting this merged.

@hyperreality Is this something you'd be interested in taking over? Either way, I'll keep this at the top of my priority list, so it doesn't get lost again.

Whoever, ends up finishing the development with this can pull down @simison's branch & make changes there & then submit a new PR (which would include Mikael's commits) from your local branch by pushing to your own fork (origin). That would work.. right?

[hyperreality]

@mleanos Personally in my project I won't be using this feature and removal of the password re-type field - it's surely less effective at ensuring that users haven't made a typo plus I haven't seen it used in many other places.

[itelo]

@mleanos Are you working in this feature?

[mleanos]

@itelo No, I'm not. Feel free to submit a PR for this.