Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Conversation

@ericcornelissen
Copy link
Contributor

Description of the Change

Added a button to the credentials dialog to toggle the input type of the password between "password" and "text" so the user can see which credentials they entered, you can find a preview below.

I did this by adding a state variable named showPassword to the CredentialDialog component that keeps track of the current state (showing or hiding the password), and a button that manipulates this value.

  • If true the <input> type is set to "text" and the <button> text to "Hide".
  • If false the <input> type is set to "password" and the <button> text to "Show"

The button has an onClick listener which is the new method toggleShowPassword, which simply sets the value of the state variable showPassword to !showPassword.

As far as I can tell this is the only place in Atom (core) where a <button> is overlayed on an <input>, hence I added a new class named .github-DialogLabelButton which positions the <button> over the <input> and uses theme-based colors.

preview

Alternate Designs

The implementation of the <button> in HTML (JSX) and CSS is somewhat arbitrary and I'm open to changing it. Note however that I placed it inside the <label> so it can be positioned relative to that.

Currently the button is text-based, alternatively it could be icon-based. However, I did not implement this as the relevant icons are not present/incorrect, see:

eye_icons_preview

Benefits

Better user experience for users that make typos when entering their credentials, i.e. everyone 😛

Possible Drawbacks

No real drawbacks, arguably Shoulder Surfing but as the default still hides the credentials I don't consider this a problem.

Applicable Issues

#1736

Metrics

N/A

Tests

I tested the feature using two tests:

  1. Verify the input type is "text" when the toggle button is pressed once.
  2. Verify the input type is "password" when the toggle button is pressed twice.

Arguably another test verifying the input type when the button was not pressed can be added, I omitted this as the combination of the two tests above imply that assertion.

Documentation

N/A

Release Notes

The GitHub package now provides a button to show/hide the input values of passwords.

User Experience Research (Optional)

No field research was done for this feature, however there is research supporting the addition of this feature. For example NIST specifies that "... [verifiers] SHOULD offer an option to display the secret..." [1] (where secret refers to the password in this situation).

If desired I can try to find more supporting research.

@ericcornelissen ericcornelissen changed the title Papercut/show credentials Add button to show/hide password in credentials dialog Nov 25, 2018
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 85.52% when pulling dfccca2 on ericcornelissen:papercut/show-credentials into 6e07816 on atom:master.

Copy link
Contributor

@smashwilson smashwilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me. Thanks!

Copy link

@annthurium annthurium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

@annthurium annthurium merged commit a70b713 into atom:master Nov 28, 2018
@ericcornelissen ericcornelissen deleted the papercut/show-credentials branch November 29, 2018 13:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants