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

feat: add password visibility button to authentication pages #572

Merged
merged 14 commits into from
Nov 24, 2023

Conversation

Darguima
Copy link
Contributor

@Darguima Darguima commented Nov 10, 2023

Added see Password Button at all login/signup forms.

I didn't checked the changes at pages/register/[uuid].js (I'm wating for feedback in how access this page seems to be unused and maybe will be deleted).

I also needed edit Input Component to accept a right children component.

Issue related: #527

image

Copy link

netlify bot commented Nov 10, 2023

Deploy Preview for seium-stg ready!

Name Link
🔨 Latest commit 076966d
🔍 Latest deploy log https://app.netlify.com/sites/seium-stg/deploys/65610587dbcb7200080eaba5
😎 Deploy Preview https://deploy-preview-572--seium-stg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@joaodiaslobo joaodiaslobo changed the title Add see password button to authentication pages feat: add password visibility button to authentication pages Nov 10, 2023
pages/register/[uuid].js Outdated Show resolved Hide resolved
pages/register/[uuid].js Outdated Show resolved Hide resolved
@ruilopesm ruilopesm added the enhancement New feature or request label Nov 11, 2023
@ruilopesm ruilopesm linked an issue Nov 11, 2023 that may be closed by this pull request
Copy link
Contributor

@tiago-bacelar tiago-bacelar 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 this is a great opportunity to create a PasswordInput component. There's a lot of duplicated code running around, and the extra useStates everywhere just to show/hide the eye are really cluttery.
You could even use the regular Input to create the PasswordInput, although I think it would be best to separate the two. That way you could also remove the right children from the Input, which would literally never be used for anything other than the eye icon on a password input.

@Darguima
Copy link
Contributor Author

@tiago-bacelar @ruilopesm , I agree with you that creating a new component is a good idea.

You mentioned not using the Input component, so should I create a new independent component, right? In that case, I'll need to duplicate some code, mainly the styles. Is that not a problem?

I understand the idea of removing the 'right' prop, and I think it's better to have the styles repeated than that right prop. However, is there a way to avoid repeating the styles? Because if, for some reason, somebody changes the Input styles, it would also be necessary to edit them in PasswordInput, and this can be forgotten.

(by now I will create the component from the right prop, to already replace everything with the component, then I edit consonant your feedback)

ruilopesm
ruilopesm previously approved these changes Nov 13, 2023
Copy link
Member

@ruilopesm ruilopesm left a comment

Choose a reason for hiding this comment

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

Great work 🙌🏼

layout/SignUp/components/SignUpForm/index.tsx Outdated Show resolved Hide resolved
pages/register/[uuid].js Outdated Show resolved Hide resolved
layout/SignUp/components/SignUpForm/index.tsx Show resolved Hide resolved
@diogogmatos
Copy link
Member

@Darguima
Copy link
Contributor Author

Darguima commented Nov 21, 2023

I already fix the requested changes about the id/name prop. I added the id/name prop again, and fix it from password to confirm on the password confirmations inputs.

̶A̶l̶s̶o̶,̶ ̶t̶o̶ ̶d̶e̶l̶e̶t̶e̶ ̶t̶h̶e̶ ̶r̶i̶g̶h̶t̶ ̶p̶r̶o̶p̶ ̶f̶r̶o̶m̶ ̶t̶h̶e̶ ̶I̶n̶p̶u̶t̶ ̶c̶o̶m̶p̶o̶n̶e̶n̶t̶,̶ ̶I̶ ̶c̶r̶e̶a̶t̶e̶d̶ ̶t̶h̶e̶ ̶P̶a̶s̶s̶w̶o̶r̶d̶I̶n̶p̶u̶t̶ ̶w̶i̶t̶h̶o̶u̶t̶ ̶a̶n̶y̶ ̶d̶e̶p̶e̶n̶d̶e̶n̶c̶y̶ ̶o̶n̶ ̶t̶h̶e̶ ̶I̶n̶p̶u̶t̶,̶ ̶b̶u̶t̶ ̶t̶o̶ ̶a̶v̶o̶i̶d̶ ̶r̶e̶p̶e̶a̶t̶e̶d̶ ̶s̶t̶y̶l̶e̶s̶,̶ ̶t̶h̶a̶t̶ ̶c̶o̶u̶l̶d̶ ̶c̶a̶u̶s̶e̶ ̶d̶i̶f̶f̶e̶r̶e̶n̶t̶ ̶s̶t̶y̶l̶e̶s̶,̶ ̶a̶f̶t̶e̶r̶ ̶a̶ ̶m̶o̶d̶i̶f̶i̶c̶a̶t̶i̶o̶n̶ ̶a̶t̶ ̶o̶n̶l̶y̶ ̶o̶n̶e̶ ̶o̶f̶ ̶t̶h̶e̶ ̶c̶o̶m̶p̶o̶n̶e̶n̶t̶,̶ ̶I̶ ̶e̶x̶p̶o̶r̶t̶e̶d̶ ̶t̶h̶e̶ ̶I̶n̶p̶u̶t̶ ̶s̶t̶y̶l̶e̶s̶ ̶f̶r̶o̶m̶ ̶t̶h̶e̶ ̶I̶n̶p̶u̶t̶ ̶c̶o̶m̶p̶o̶n̶e̶n̶t̶.̶ ̶W̶h̶a̶t̶ ̶d̶o̶ ̶y̶o̶u̶ ̶t̶h̶i̶n̶k̶ ̶o̶f̶ ̶t̶h̶e̶ ̶r̶e̶s̶u̶l̶t̶?̶ ̶(̶I̶ ̶k̶n̶o̶w̶ ̶t̶h̶a̶t̶ ̶i̶s̶n̶'̶t̶ ̶t̶h̶e̶ ̶m̶o̶s̶t̶ ̶c̶o̶m̶m̶o̶n̶ ̶t̶h̶i̶n̶g̶,̶ ̶s̶o̶ ̶I̶'̶m̶ ̶w̶a̶i̶t̶i̶n̶g̶ ̶f̶e̶e̶d̶b̶a̶c̶k̶)̶

@Darguima
Copy link
Contributor Author

To prevent repeated styles across the project were created a InputBase wrapper to be used internally when creating Input components.

Copy link
Contributor

@tiago-bacelar tiago-bacelar left a comment

Choose a reason for hiding this comment

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

Good job! 💪

@Darguima Darguima merged commit 4923c9c into main Nov 24, 2023
5 checks passed
@Darguima Darguima deleted the dg/password_visibility branch November 24, 2023 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add see password button to authentication pages
4 participants