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

#65 - toggle password visibility #164

Merged
merged 5 commits into from
Nov 8, 2023
Merged

Conversation

Lee0z
Copy link
Collaborator

@Lee0z Lee0z commented Nov 7, 2023

Added button to toggle password visibility.

  • Invisible:
    image

  • Visible:
    image

Same goes to register form.

  • Invisible:
    image

  • Visible:
    image

It should close #65.

@Lee0z Lee0z requested a review from a team as a code owner November 7, 2023 12:44
@Lee0z Lee0z linked an issue Nov 7, 2023 that may be closed by this pull request
Copy link
Contributor

@EwelinaSkrzypacz EwelinaSkrzypacz left a comment

Choose a reason for hiding this comment

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

I think I will get rid of filed "password confirmation" because now we have toggle to see password, but maybe it's a task for another pr?

@Lee0z
Copy link
Collaborator Author

Lee0z commented Nov 8, 2023

I think I will get rid of filed "password confirmation" because now we have toggle to see password, but maybe it's a task for another pr?

So the button in registration form doesn't reveal "Confirm Password" field? Just the "Password" one?

@EwelinaSkrzypacz
Copy link
Contributor

I think I will get rid of filed "password confirmation" because now we have toggle to see password, but maybe it's a task for another pr?

So the button in registration form doesn't reveal "Confirm Password" field? Just the "Password" one?

No, I mean that we should delete input "Confirm password". This field is unnecessary due to potential user errors, the inconvenience of entering complex passwords twice, the availability of more advanced verification methods, and the desire for a smoother user experience.

image

@Lee0z
Copy link
Collaborator Author

Lee0z commented Nov 8, 2023

I think I will get rid of filed "password confirmation" because now we have toggle to see password, but maybe it's a task for another pr?

So the button in registration form doesn't reveal "Confirm Password" field? Just the "Password" one?

No, I mean that we should delete input "Confirm password". This field is unnecessary due to potential user errors, the inconvenience of entering complex passwords twice, the availability of more advanced verification methods, and the desire for a smoother user experience.

image

That is great idea, so I changed registration form:
image

Is there any backend code to support that password confirmation? As far as I know it is only frontend feature. Please check commit after this comment.

@EwelinaSkrzypacz
Copy link
Contributor

I think I will get rid of filed "password confirmation" because now we have toggle to see password, but maybe it's a task for another pr?

So the button in registration form doesn't reveal "Confirm Password" field? Just the "Password" one?

No, I mean that we should delete input "Confirm password". This field is unnecessary due to potential user errors, the inconvenience of entering complex passwords twice, the availability of more advanced verification methods, and the desire for a smoother user experience.
image

That is great idea, so I changed registration form: image

Is there any backend code to support that password confirmation? As far as I know it is only frontend feature. Please check commit after this comment.

In files there are some use of password_confirmation, for example

  • Feature/SignupTest.php
  • Exceptions/ExceptionHandler.php
  • Http/Middleware/TrimStrings.php

I will recommend use Ctrl + Shift + F in Phpstorm to search all of occurrences of text

@Lee0z
Copy link
Collaborator Author

Lee0z commented Nov 8, 2023

I think I will get rid of filed "password confirmation" because now we have toggle to see password, but maybe it's a task for another pr?

So the button in registration form doesn't reveal "Confirm Password" field? Just the "Password" one?

No, I mean that we should delete input "Confirm password". This field is unnecessary due to potential user errors, the inconvenience of entering complex passwords twice, the availability of more advanced verification methods, and the desire for a smoother user experience.
image

That is great idea, so I changed registration form: image
Is there any backend code to support that password confirmation? As far as I know it is only frontend feature. Please check commit after this comment.

In files there are some use of password_confirmation, for example

  • Feature/SignupTest.php
  • Exceptions/ExceptionHandler.php
  • Http/Middleware/TrimStrings.php

I will recommend use Ctrl + Shift + F in Phpstorm to search all of occurrences of text

There was indeed written tests for signup feature, so I've removed password_confirmation field.
Since we don't need "password_confirmation" to be trimmed through middleware, I've deleted that here too, same goes to exceptions.

In backend register request there were also rules specified, which required "confirmation", deleted.

I've manualy tested account creation few times and after modifications it worked.

Also noticed small bug - on error event (wrong password input etc.) the password visibility button moved down to match both error and password field. It's fixed.

@Lee0z Lee0z merged commit 5c93e11 into main Nov 8, 2023
3 checks passed
@Lee0z Lee0z deleted the #65-toggle-password-visibility branch November 8, 2023 15:30
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.

Toggle Password Visibility
4 participants