Skip to content

Adding Password Component - #1946

Open
shravani-0811 wants to merge 2 commits into
masterfrom
password-component
Open

Adding Password Component#1946
shravani-0811 wants to merge 2 commits into
masterfrom
password-component

Conversation

@shravani-0811

@shravani-0811 shravani-0811 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Description

Adds a new **Adaptive Form Password Input ** core component: a masked <input type="password"> field with an optional show/hide visibility toggle button.

Related Issue

Motivation and Context

Adaptive Forms didn't have a first-class password field: authors had to repurpose Text Input, which offers no masking, no visibility toggle, and no autofill-attribute control suited to credentials. This adds that as a proper, accessible core component while closing off the plaintext-exposure trap that comes from naively inheriting Text Input's "default value" behavior for a password-typed field.

How Has This Been Tested?

  • Java unit tests (TextInputImplTest): field type resolution (fieldType=passwordgetFieldType()/getExportedType()), isShowHidePasswordEnabled() default-true and author-disabled cases, and .model.json export shape via testJSONExport.
  • Cypress authoring tests (passwordinput.authoring.cy.js): dropping the component into a form container / responsive grid via the editor sidebar, and opening its edit dialog.
  • Cypress runtime tests (passwordinput.runtime.cy.js): model/view initialization, masked-by-default rendering, clicking the eye icon reveals plaintext and toggles aria-pressed/aria-label (and back), toggle button absent when showHidePassword is disabled, value submits correctly regardless of toggle state, minLength/mandatory validation messages, description/tooltip toggling, and aria-disabled absence when disabled.
  • Manual verification: deployed to a local AEM SDK author instance; confirmed the component appears in the standard Adaptive Form component sidebar and renders/toggles correctly (masked ↔ plaintext, aria-pressed flips) in the disabled/preview rendering path.

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 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 and the overall coverage did not decrease.
  • All unit tests pass on CircleCi.
  • I ran all tests locally and they pass.

@adobe-bot

Copy link
Copy Markdown

Accessibility Violations Found

Id Impact
aria-required-attr critical
empty-heading minor
label-title-only serious
target-size serious

1 similar comment
@adobe-bot

Copy link
Copy Markdown

Accessibility Violations Found

Id Impact
aria-required-attr critical
empty-heading minor
label-title-only serious
target-size serious

* @since com.adobe.cq.forms.core.components.models.form 2.0.0
*/
@JsonIgnore
default boolean isShowHidePasswordEnabled() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should not be json ignore, might be required in headless, this should be fd: prefixed, since it is OOTB property

### Note on styling the show/hide toggle button
This component only renders the `.cmp-adaptiveform-passwordinput__toggle-visibility` button
element and its accessibility attributes (`aria-pressed`, `aria-label`). It intentionally
ships with **no visual/icon CSS** for that button — consistent with how every other core

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How is the cypress test currently clicking this, since this is invisible. How did you test this ?


@ValueMapValue(injectionStrategy = InjectionStrategy.OPTIONAL, name = ReservedProperties.PN_SHOW_HIDE_PASSWORD)
@Default(booleanValues = true)
protected boolean showHidePassword;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should create something specific for password, with time, if we enhance password, contract might be leaked into text/telephone etc

if (this.widget.value !== '') {
this.setModelValue(this.widget.value);
}
this.widget.addEventListener('blur', (e) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No input listener → no as-you-type minLength/pattern feedback, and I don't see any character restriction (was this supported in v1, can you check)

<jcr:root xmlns:cq="http://www.day.com/jcr/cq/1.0" xmlns:jcr="http://www.jcp.org/jcr/1.0"
jcr:primaryType="cq:ClientLibraryFolder"
allowProxy="{Boolean}true"
categories="[core.forms.components.textinput.v1.runtime]"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this should be password input why is it textinput.v1.runtime

if (this.widget.value !== '') {
this.setModelValue(this.widget.value);
}
this.widget.addEventListener('blur', (e) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we add more configuration in password like restrict to numeric digits or alpha numeric etc, this is a very common customer use-case

@rismehta rismehta left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

check comments

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.

3 participants