Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace hide/show password text with eye icons #9534

Conversation

rebecca-shoptaw
Copy link
Collaborator

@rebecca-shoptaw rebecca-shoptaw commented Jul 8, 2024

Closes #9533.

Feature. Switches out "Hide Password"/"Show Password" text for some simple eye open/closed icons.

Technical

  • Went with Material Design icons to match the mockup
  • Involved moving the password-visibility-toggle class from the a element to its img child so that the JavaScript would activate on the correct element
  • Would also be happy to do away with the a element entirely and just put a cursor: pointer on the img -- held off for now in case the image nested in link is the preferred pattern for icons

Testing

  1. Go to /account/create
  2. Begin typing a password -- by default the password should be hidden and you should see a closed-eye icon
  3. Click on the icon -- the password should appear and the icon should switch to an open eye

Screenshot

Hidden password Visible password

Stakeholders

@cdrini

@codecov-commenter
Copy link

codecov-commenter commented Jul 8, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 16.29%. Comparing base (1cd8f21) to head (b5f4c1b).
Report is 40 commits behind head on master.

Files Patch % Lines
...nlibrary/plugins/openlibrary/js/password-toggle.js 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9534      +/-   ##
==========================================
+ Coverage   16.17%   16.29%   +0.12%     
==========================================
  Files          90       90              
  Lines        4736     4761      +25     
  Branches      824      826       +2     
==========================================
+ Hits          766      776      +10     
- Misses       3453     3463      +10     
- Partials      517      522       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rebecca-shoptaw rebecca-shoptaw force-pushed the 9533/feature/add-password-eye-icons branch from 5de58f3 to 7b63dea Compare July 8, 2024 20:43
@rebecca-shoptaw rebecca-shoptaw marked this pull request as ready for review July 8, 2024 20:47
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Looks great! One small accessibility fix.

@cdrini cdrini self-assigned this Jul 10, 2024
@cdrini cdrini added Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing labels Jul 10, 2024
@github-actions github-actions bot removed the Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] label Jul 11, 2024
static/css/page-signup.less Outdated Show resolved Hide resolved
Copy link
Collaborator

@cdrini cdrini left a comment

Choose a reason for hiding this comment

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

Looks great! 👁️

@cdrini cdrini merged commit 9a9069b into internetarchive:master Jul 11, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
On testing.openlibrary.org This PR has been deployed to testing.openlibrary.org for testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add icons for registration hide/show password toggle
3 participants