Skip to content

Add view password functionality to login form#4714

Merged
dorner merged 2 commits into
rubyforgood:mainfrom
princekumarg12:4671-add-view-password-functionality
Oct 11, 2024
Merged

Add view password functionality to login form#4714
dorner merged 2 commits into
rubyforgood:mainfrom
princekumarg12:4671-add-view-password-functionality

Conversation

@princekumarg12
Copy link
Copy Markdown
Contributor

Resolves #4671

Description

This change implements an "eyeball" icon functionality on the login page, allowing users to toggle password visibility. This feature aims to improve user experience by enabling users to confirm their password entry, reducing login errors due to typos.

This solution balances usability with security, ensuring the password is hidden by default.

No additional dependencies are required for this change.

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

I implemented unit tests to verify the password toggle functionality:

  • Confirmed that clicking the "eyeball" icon switches the password field from masked to visible.
  • Verified the icon changes correctly between eye and eye-slash states.

@cielf cielf requested a review from dorner October 10, 2024 18:48
@cielf
Copy link
Copy Markdown
Collaborator

cielf commented Oct 10, 2024

Functionality looks fine

Comment thread app/views/users/sessions/new.html.erb Outdated
<% end %>

<script>
document.getElementById('toggle_password').onclick = function () {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we package this into a Stimulus controller? That's the way we want to encourage sprinkling JavaScript into our pages. You can see an example here: https://github.com/rubyforgood/human-essentials/blob/8839e19eb67ed92fb4c96fa7a98beab2ddf9e09f/app/javascript/controllers/highchart_controller.js#L32-L32 and it's used

<button type="button" class="btn btn-sm btn-secondary" data-action="click->highchart#selectAll">Select All</button>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi Dorner,
I've refactored the password toggle into a Stimulus controller as requested. It’s now encapsulated neatly in the password_visibility_controller.js, making our JS integration more structured.

@dorner
Copy link
Copy Markdown
Collaborator

dorner commented Oct 11, 2024

Also, erb lint is failing - see above.

@princekumarg12 princekumarg12 requested a review from dorner October 11, 2024 12:12
@dorner dorner merged commit 7e44962 into rubyforgood:main Oct 11, 2024
@dorner
Copy link
Copy Markdown
Collaborator

dorner commented Oct 11, 2024

Awesome, thanks!

@github-actions
Copy link
Copy Markdown
Contributor

@princekumarg12: Your PR Add view password functionality to login form is part of today's Human Essentials production release: 2024.10.13.
Thank you very much for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a view password functionality on login

3 participants