FIX: password visibility toggle with input mask https://github.com/Crocoblock/issues-tracker/issues/14919 - #737

🤖 AI PR Review

Risk level: low

Review

Summary

• This PR centralises the eye/visibility toggle logic into a new addEyeIconListeners helper (modules/blocks-v2/text-field/assets/src/frontend/field/TextFieldData.js) and re-uses it from the masked text field (TextFieldMaskedData). Built assets were updated accordingly.

What I looked at

• Source: modules/blocks-v2/text-field/assets/src/frontend/field/TextFieldData.js
• Masked: modules/blocks-v2/text-field/assets/src/frontend/field.mask/TextFieldMaskedData.js
• Built assets: modules/blocks-v2/text-field/assets/build/frontend/field*.js and .asset.php files

Positive

• Good refactor to avoid duplication: eye icon listeners are implemented once and reused.
• Uses optional chaining to guard DOM lookups which is safe and reduces console errors when eye icon is absent.
• aria-pressed state is updated, key handlers added for keyboard activation — accessibility considerations are present.

Security

• No server-side changes. The JS changes operate on DOM only and do not introduce new XSS or data handling paths.
• Event handlers use plain DOM APIs and do not insert HTML, so no new sanitization concerns.

Compatibility / integration concerns

• Backward compatibility: TextFieldData still exports the default TextFieldData, and addEyeIconListeners was added as a named export. That should not break existing imports that use the default. Ensure no other code relied on a named export that has changed — I see only the masked file imports the helper.
• Masked input + inputmask: toggling the input.type may interact with inputmask (some mask libraries reinitialize or stop working when type changes). The change intentionally attaches the toggle to masked inputs; please test the following cases manually: 1) masked password field with inputmask present, 2) masked field when clearMaskOnSubmit enabled/disabled, 3) switching visibility before and after the mask is applied (fast user interactions). Ensure inputmask behaviour remains correct after toggling (no loss of mask/unmasked value) and the form submits the expected value.
• Browser compatibility / build output: The built/minified file for masked field introduced the nullish coalescing operator (??) in the built asset (field.mask.js built). Confirm your bundler/transpilation step actually targets the same set of browsers supported by JetFormBuilder (WordPress historically supports older browsers). If these are distributed built assets, make sure they're transpiled to ES5 if required by supported WordPress/browser targets.

Potential issues / suggestions

1. Toggle logic inversion — please verify intended semantics of the "seen" class:

   • In addEyeIconListeners node.type is set with:
     node.type = button.classList.contains('seen') ? 'password' : 'text';
     This means when the button has class 'seen' the input becomes type="password", otherwise type="text". That works if the class name describes the eye icon state where the eye being "seen" means the value is hidden. The previous minified code (in earlier built file) used the same expression. Make sure this matches UI/UX expectation in CSS and markup. If the icon CSS uses "seen" to indicate visible content, this conditional should be inverted. I recommend adding a short comment or renaming the class (or using data-state) to avoid future confusion.

2. Accessibility improvements

   • aria-pressed is toggled but ensure the element that receives the listeners is a real button with an accessible name/label (e.g. aria-label="Toggle password visibility") or contains accessible text. If it’s a non-button element, it should still have role="button" and be focusable. If those attributes are added elsewhere in markup, ignore this.

3. Testing

   • Add a small manual test matrix (or automated end-to-end test if available) for: normal password field, masked password field (inputmask present), keyboard toggling (Space/Enter), and form submit with/without clearMaskOnSubmit on.

4. Built files

   • The PR updates built assets (field.js, field.mask.js) and their asset.php version hashes. Make sure the repo’s workflow for building assets was followed and that CI runs linting/tests on the built files; otherwise reviewers might want to see only src changes and the canonical build commit.

Minor

• The new helper is exported as a named export and the default remains. That’s good. Ensure any other code consuming the default import isn’t accidentally changed or relying on side-effects.

Conclusion

• The change is small and focused. Main risk is the interaction between toggling input.type and the inputmask plugin — please verify the mask is preserved and submission values remain correct after toggling, and confirm the semantics of the "seen" class are intentional. Also double-check the build/transpile step for backward browser support.

Recommended follow-ups before merge

• Manual QA of masked password fields (see Testing above).
• Confirm built assets are transpiled for supported browsers (or remove modern syntax from built files if not).
• Optionally add a short comment near the class.contains('seen') line documenting expected semantics to avoid confusion later.
• Consider adding an automated E2E test that covers the mask + visibility toggle behaviour if the repo has such tests.

Suggested changelog entry

- FIX: password visibility toggle for masked text-field (input mask) in text-field block