Skip to content

Show/Hide password CSS - #112

Open
shravani-0811 wants to merge 3 commits into
mainfrom
password-component
Open

shravani-0811 wants to merge 3 commits into
mainfrom
password-component

Conversation

@shravani-0811

Copy link
Copy Markdown
Collaborator

Description

Added Files for showing and hiding password as a toggle button

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Comment thread src/components/passwordinput/_passwordinput.scss Outdated
Comment thread src/components/passwordinput/_passwordinput.scss Outdated
Comment thread src/components/passwordinput/_passwordinput.scss Outdated
Comment thread src/components/passwordinput/_passwordinput.scss Outdated
Comment thread src/components/passwordinput/_passwordinput.scss Outdated
Comment thread src/components/passwordinput/_passwordinput.scss Outdated
Comment thread src/components/passwordinput/_passwordinput.scss Outdated

@rismehta rismehta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

check comments

@sakshi-arora1 sakshi-arora1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The commit is confusing. As we are touching all the areas.

  1. Create a new commit to make variables, if you want to make it more variable driven. But you need to make sure the values remain same, as any change can actually break production forms.
  2. Create a separate commit for password

@iamsudhanshu

iamsudhanshu commented Oct 1, 2026 •

Copy link
Copy Markdown

The commit is confusing. As we are touching all the areas.

  1. Create a new commit to make variables, if you want to make it more variable driven. But you need to make sure the values remain same, as any change can actually break production forms.
  2. Create a separate commit for password

Checked the PR has 2 commits - with migration to variable in a separate commit - so we should be good. Also validated the variable replacement with copilot:
image

so if we do not squash and merge - we should be good, and we can share the specific commit with customers to integrate the password changes in their repos. @sakshi-arora1 could you confirm if we are good to merge in that case.

cc: @shravani-0811

@armaang1729 armaang1729 mentioned this pull request Oct 1, 2026
2 of 10 tasks
shravani-0811 and others added 2 commits October 1, 2026 15:13
Add $field-border-radius, $field-border-width, $validation-border-width,
and $validation-inset-padding to _variables.scss, and migrate passwordinput
and all other affected components to use them in place of repeated magic
numbers (0.25rem, 1px, 4px, 5px) and the hardcoded #969696 color. This is a
value-preserving refactor: rendered output is unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Correct the show/hide toggle button to reference the actual committed
icon assets (showPassword.svg / hidePassword.svg) instead of the
non-existent eye.svg / eye-off.svg, and align the toggle button's
border-radius with the shared $field-border-radius value (0.2rem -> 0.25rem)
to match the rest of the field styling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

4 participants