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 input #2772

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

maicongodinho
Copy link
Contributor

@maicongodinho maicongodinho commented Jan 28, 2024

Closes #1930

This update introduces a new password input component, encompassing all essential features available in the base text input component.

@maicongodinho maicongodinho requested a review from a team as a code owner January 28, 2024 16:22
Copy link

netlify bot commented Jan 28, 2024

Deploy Preview for carbon-components-angular ready!

Name Link
🔨 Latest commit 4e06bec
🔍 Latest deploy log https://app.netlify.com/sites/carbon-components-angular/deploys/664fa8a7b059610008e8a335
😎 Deploy Preview https://deploy-preview-2772--carbon-components-angular.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.

@maicongodinho
Copy link
Contributor Author

maicongodinho commented Jan 28, 2024

In the context of React components, where a "directive-like" solution is not employed, my approach to the new component involves maintaining the use of projection. A type flag is utilized to seamlessly switch between templates representing different input types, thus ensuring native HTML5 password handling.

To implement this, users are required to utilize two inputs accompanied by the cdsPassword or ibmPassword directive, each configured with distinct types (text and password). The PasswordInputLabelComponent is then responsible for managing these inputs effectively.

Building upon the React component reference found at https://react.carbondesignsystem.com/?path=/story/components-textinput--toggle-password-visibility

Hope that covers all 😄

@hubertmalkowski
Copy link

hubertmalkowski commented May 14, 2024

Sorry to butt in, but is there any update on this PR?

@Akshat55
Copy link
Contributor

Sorry, been preoccupied, let me prioritize this for this week and we can hopefully merge it sometimes next week. @maicongodinho, I'm going to make a few simplifications to this. I want to be able to toggle input property type so users don't have to project both input[type="text"] and input[type="password"].

@maicongodinho
Copy link
Contributor Author

Hey @Akshat55, indeed, this is an older implementation. I aimed to leverage the directive style, integrating best practices in security and accessibility from the React library. While exploring simplification within our existing structure, I couldn't find an optimal solution. However, we could consider developing a new, consolidated component that combines the functionality of labels and password features seamlessly. Let me know what you think and what you need from my side.

Signed-off-by: Akshat Patel <akshat@live.ca>
Signed-off-by: Akshat Patel <akshat@live.ca>
Signed-off-by: Akshat Patel <akshat@live.ca>
Signed-off-by: Akshat Patel <akshat@live.ca>
Signed-off-by: Akshat Patel <akshat@live.ca>
@maicongodinho
Copy link
Contributor Author

Hi @Akshat55, I just saw the approach you took. Excellent use of setting the property to the native element. I hadn't considered this method myself. Really good work!

src/input/password-input-label.component.ts Outdated Show resolved Hide resolved
src/input/password-input-label.component.ts Outdated Show resolved Hide resolved
Akshat55
Akshat55 previously approved these changes May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Featured
Development

Successfully merging this pull request may close these issues.

Password Component Needed
4 participants