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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add password visibility toggle #15605

Merged
merged 9 commits into from
Jun 26, 2024

Conversation

joeyksclark
Copy link
Contributor

First contribution 馃コ

Issue: #14321

Add ability to toggle password visibility by clicking an eye icon. This functionality is available from both the login and registration pages. Tested on Firefox and Safari. Screenshots are from Firefox.

Screenshot 2024-06-26 at 2 02 32鈥疉M
Screenshot 2024-06-26 at 2 02 44鈥疉M
Screenshot 2024-06-26 at 2 02 58鈥疉M
Screenshot 2024-06-26 at 2 03 06鈥疉M

@ornicar
Copy link
Collaborator

ornicar commented Jun 26, 2024

I wish the browsers would do that natively, but... they don't. So let's see this

background-color: currentColor;
transform: rotate(45deg);
top: 50%;
left: 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry about that, I appreciate the handcrafted solution here, but as the person who would be maintaining it for years, I prefer a oneliner instead :)

such as /account/twofactor, /account/close, /account/passwd and more
@ornicar ornicar merged commit 2ae4c6e into lichess-org:master Jun 26, 2024
4 checks passed
@joeyksclark
Copy link
Contributor Author

Thanks for reviewing and updating my PR. I noticed the changes made the code quite different from my original. Is this common for Lichess PRs? I usually prefer iterating on changes based on feedback but understand if this approach is preferred.

@ornicar
Copy link
Collaborator

ornicar commented Jun 26, 2024

I often find that feedback as code is quicker and more precise.

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.

None yet

2 participants