feat(input): add showPasswordToggle, hidePasswordIcon and showPasswordIcon properties #29141

OS-giulianasilva · 2024-03-12T16:09:28Z

Issue number: Internal

What is the current behavior?

The ion-input does not provide an ability to toggle password visibility on input

What is the new behavior?

Add three new properties:
- showPasswordToggle to enable the passwordToggle button in the input that when clicked toggles the input type between "text" and "password", allowing the user to view or hide the input value.
- showPasswordIcon to set the icon to be used to represent showing a password.
- hidePasswordIcon to set the icon to be used to represent hiding a password.

Does this introduce a breaking change?

Yes
No

Other information

Behavior on MD:

Behavior on iOS:

core/src/components/input/test/password-toggle/input.e2e.ts

sean-perkins

Couple of initial questions, great work so far!

I've updated the targeted branch to feature-8.0 since we will not be releasing any additional minor release changes other than what we've already committed and v8 is close 🎉

You will need to either merge feature-8.0 into your branch and resolve the merge conflicts or rebase. api.txt, components.d.ts and the proxies.ts are auto-generated from npm run build, so you can take whatever diff and then re-generate the results.

Once we are up to date with feature-8.0 I can help get screenshots generated and merged into your fork. The team is actively working to remove this restraint in the near future so you can update screenshots locally.

sean-perkins · 2024-03-12T21:06:10Z

core/src/components/input/input.tsx

@@ -836,6 +857,30 @@ export class Input implements ComponentInterface {
                <ion-icon aria-hidden="true" icon={mode === 'ios' ? closeCircle : closeSharp}></ion-icon>
              </button>
            )}
+            {this.showPasswordToggle && !readonly && !disabled && (


The design document described rendering this content as the default content of the end slot.

Did we experience an issue or additional consideration to not render it as the default slot content?

Actually, this is something that I overlooked in the design document. But now testing it as the default content of the end slot, I noticed that the password toggle button will be hidden when the end slot is used.

I think this would impact some use-cases. For example, if the developer wants to use the password toggle and also add a green checkmark icon to the end slot to represent the validation of the strength of the password, it will not be possible to achieve.

Please let me know your thoughts on it.

Let's talk about this internally.

core/src/components/input/input.tsx

JoaoFerreira-FrontEnd · 2024-03-13T16:05:57Z

core/src/components/input/input.scss

  appearance: none;
 }

-:host(.in-item-color) .input-clear-icon {
+:host(.in-item-color) .input-clear-icon,
+:host(.in-item-color) .input-password-toggle {


let's break this into

:host(.in-item-color){ .input-clear-icon, .input-password-toggle { } }

I would like to keep it as it is, because it is more readable and also matches the current CSS structure of the code.

@sean-perkins what do you think about this topic?

Most CSS we've written in Ionic avoids nested to keep the code readable. However, I think either would be fine -- really comes down to a matter of preference.

liamdebeasi

Great work so far!

core/src/components/input/input.tsx

core/src/components/input/test/password-toggle/input.e2e.ts

liamdebeasi · 2024-03-14T15:41:47Z

core/src/components/input/input.scss

  appearance: none;
 }

-:host(.in-item-color) .input-clear-icon {
+:host(.in-item-color) .input-clear-icon,
+:host(.in-item-color) .input-password-toggle {


Most CSS we've written in Ionic avoids nested to keep the code readable. However, I think either would be fine -- really comes down to a matter of preference.

liamdebeasi · 2024-03-14T15:42:02Z

core/src/components/input/input.scss

+:host(:not(.has-value)) .input-clear-icon {
+  visibility: hidden;


Why was this change made?

Because otherwise, the clear icon would not disappear when the input is erased.

The clear icon currently disappears when the input is erased on the latest stable version of Ionic: https://ionicframework.com/docs/api/input#clear-options

If that's not the case in this branch, then there may be other code impacting that.

liamdebeasi · 2024-03-14T15:44:06Z

core/src/components/input/test/password-toggle/input.e2e.ts

+      const input = page.locator('ion-input');
+      const nativeInput = page.locator('.native-input');
+      const toggle = input.locator('.input-password-toggle');
+
+      await toggle.click();
+      await page.waitForChanges();
+
+      await expect(nativeInput).toHaveAttribute('type', 'text');
+
+      await toggle.click();
+      await page.waitForChanges();
+
+      await expect(nativeInput).toHaveAttribute('type', 'password');


This could be a spec test. We don't necessarily benefit from having cross-browser testing here.

Makes sense! I will take care of this tomorrow.

liamdebeasi · 2024-03-14T15:44:27Z

core/src/components/input/test/password-toggle/input.e2e.ts

+      const input = page.locator('ion-input');
+      const toggle = input.locator('.input-password-toggle');
+      const icon = toggle.locator('ion-icon');
+
+      await expect(icon).toHaveJSProperty('icon', 'bus');
+
+      await toggle.click();
+      await page.waitForChanges();
+
+      await expect(icon).toHaveJSProperty('icon', 'cafe');


Same as previous comment, we don't really value from having cross-browser testing here so this could be a spec test.

Same as above.

liamdebeasi · 2024-03-14T15:50:59Z

Also you'll want to run npm run lint.fix from core and push your changes so the test-core-lint step passes.

Issue number: Internal ---------   ## What is the current behavior?  When given a password input it is hard to know what users are typing as the contents of the input are obscured. As a result, it is a common pattern to have a button that lets users temporarily toggle the visibility of the password so they can correct any mistakes. Ionic currently has the infrastructure for developers to implement this on their own, but this use case is so common that the team thinks it is worth having this functionality built-in to Ionic. ## What is the new behavior?  - Introduces the `ion-input-password-toggle` component. This component is a button that toggles the visibility of the text in the input it is slotted into. ## Does this introduce a breaking change? - [ ] Yes - [x] No  ## Other information  ⚠️ Give co-author credit to #29141 on merge. Docs PR: ionic-team/ionic-docs#3541 Note: We did not do the approach listed in the other PR due to #29141 (comment). --------- Co-authored-by: OS-giulianasilva <[email protected]>

thetaPC · 2024-04-03T17:38:49Z

Thank you for contributing! I'm going to close this PR since the work was implemented through PR #29141.

BenOsodrac and others added 5 commits January 31, 2024 17:49

Added new props to ion-input

e55060c

Merge branch 'main' into ROU-4802-2

659a147

add toggle password icon props and test

ae17eb4

update props names and tests

bdbe93b

build and lint

b4abe2b

OS-giulianasilva requested review from liamdebeasi, thetaPC, sean-perkins and a team as code owners March 12, 2024 16:09

github-actions bot added package: core @ionic/core package package: angular @ionic/angular package package: vue @ionic/vue package labels Mar 12, 2024

OS-giulianasilva commented Mar 12, 2024

View reviewed changes

core/src/components/input/test/password-toggle/input.e2e.ts Show resolved Hide resolved

sean-perkins changed the base branch from main to feature-8.0 March 12, 2024 21:03

sean-perkins reviewed Mar 12, 2024

View reviewed changes

OS-giulianasilva and others added 5 commits March 13, 2024 14:20

Merge branch 'feature-8.0' into ROU-4802-2

6013e12

update components.d.ts

962d01a

update proxies files and api.txt

54a10f6

deconstruct type property in render function

b71a31d

Merge branch 'feature-8.0' into ROU-4802-2

91ae531

JoaoFerreira-FrontEnd requested changes Mar 13, 2024

View reviewed changes

liamdebeasi suggested changes Mar 14, 2024

View reviewed changes

OS-giulianasilva added 7 commits March 14, 2024 17:39

refactor(input): change the function name

c5111b2

refactor(input): change aria-controls to use the local variable

61d6453

style(input): change default showPasswordIcon

cde7710

test(input): convert password toggle e2e tests to spec tests

60ab311

style(input): apply lint fixes

2183017

fix(input): remove unnecessary tag from index.html

4694c12

style(input): apply lint fixes

db970d8

liamdebeasi mentioned this pull request Mar 19, 2024

feat(input): add input-password-toggle component #29175

Merged

2 tasks

thetaPC closed this Apr 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(input): add showPasswordToggle, hidePasswordIcon and showPasswordIcon properties #29141

feat(input): add showPasswordToggle, hidePasswordIcon and showPasswordIcon properties #29141

OS-giulianasilva commented Mar 12, 2024

sean-perkins left a comment

sean-perkins Mar 12, 2024

OS-giulianasilva Mar 13, 2024

OS-giulianasilva Mar 15, 2024

JoaoFerreira-FrontEnd Mar 13, 2024

OS-giulianasilva Mar 13, 2024

JoaoFerreira-FrontEnd Mar 14, 2024

liamdebeasi Mar 14, 2024

liamdebeasi left a comment

liamdebeasi Mar 14, 2024

liamdebeasi Mar 14, 2024

OS-giulianasilva Mar 14, 2024

liamdebeasi Mar 14, 2024

liamdebeasi Mar 14, 2024

OS-giulianasilva Mar 14, 2024

OS-giulianasilva Mar 15, 2024

liamdebeasi Mar 14, 2024

OS-giulianasilva Mar 14, 2024

OS-giulianasilva Mar 15, 2024

liamdebeasi commented Mar 14, 2024

thetaPC commented Apr 3, 2024

		:host(:not(.has-value)) .input-clear-icon {
		visibility: hidden;

feat(input): add showPasswordToggle, hidePasswordIcon and showPasswordIcon properties #29141

feat(input): add showPasswordToggle, hidePasswordIcon and showPasswordIcon properties #29141

Conversation

OS-giulianasilva commented Mar 12, 2024

What is the current behavior?

What is the new behavior?

Does this introduce a breaking change?

Other information

sean-perkins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liamdebeasi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liamdebeasi commented Mar 14, 2024

thetaPC commented Apr 3, 2024