-
Notifications
You must be signed in to change notification settings - Fork 13.5k
feat(input): add showPasswordToggle, hidePasswordIcon and showPasswordIcon properties #29141
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
Changes from all commits
e55060c
659a147
ae17eb4
bdbe93b
b4abe2b
6013e12
962d01a
54a10f6
b71a31d
91ae531
c5111b2
61d6453
cde7710
60ab311
2183017
4694c12
db970d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -180,7 +180,8 @@ | |
// Clear Input Icon | ||
// -------------------------------------------------- | ||
|
||
.input-clear-icon { | ||
.input-clear-icon, | ||
.input-password-toggle { | ||
@include margin(auto); | ||
@include padding(0); | ||
@include background-position(center); | ||
|
@@ -202,11 +203,11 @@ | |
|
||
color: $text-color-step-400; | ||
|
||
visibility: hidden; | ||
appearance: none; | ||
} | ||
|
||
:host(.in-item-color) .input-clear-icon { | ||
:host(.in-item-color) .input-clear-icon, | ||
:host(.in-item-color) .input-password-toggle { | ||
color: inherit; | ||
} | ||
|
||
|
@@ -217,12 +218,13 @@ | |
* However, the clear button always disappears after | ||
* being activated, so we never get to that state. | ||
*/ | ||
.input-clear-icon:focus { | ||
.input-clear-icon:focus, | ||
.input-password-toggle:focus { | ||
opacity: 0.5; | ||
} | ||
|
||
:host(.has-value) .input-clear-icon { | ||
visibility: visible; | ||
:host(:not(.has-value)) .input-clear-icon { | ||
visibility: hidden; | ||
Comment on lines
+226
to
+227
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this change made? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because otherwise, the clear icon would not disappear when the input is erased. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
} | ||
|
||
// Input Wrapper | ||
|
@@ -399,7 +401,7 @@ | |
*/ | ||
max-width: 200px; | ||
|
||
transition: color 150ms cubic-bezier(.4, 0, .2, 1), transform 150ms cubic-bezier(.4, 0, .2, 1); | ||
transition: color 150ms cubic-bezier(0.4, 0, 0.2, 1), transform 150ms cubic-bezier(0.4, 0, 0.2, 1); | ||
|
||
/** | ||
* This ensures that double tapping this text | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ import { inheritAriaAttributes, debounceEvent, inheritAttributes, componentOnRea | |
import { createSlotMutationController } from '@utils/slot-mutation-controller'; | ||
import type { SlotMutationController } from '@utils/slot-mutation-controller'; | ||
import { createColorClasses, hostContext } from '@utils/theme'; | ||
import { closeCircle, closeSharp } from 'ionicons/icons'; | ||
import { closeCircle, closeSharp, eyeOff, eye } from 'ionicons/icons'; | ||
|
||
import { getIonMode } from '../../global/ionic-global'; | ||
import type { AutocompleteTypes, Color, StyleEventDetail, TextFieldTypes } from '../../interface'; | ||
|
@@ -162,6 +162,11 @@ export class Input implements ComponentInterface { | |
*/ | ||
@Prop() helperText?: string; | ||
|
||
/** | ||
* Set the icon that can be used to represent hiding a password. Defaults to `eyeOff`. | ||
*/ | ||
@Prop() hidePasswordIcon?: string; | ||
|
||
/** | ||
* The visible label associated with the input. | ||
* | ||
|
@@ -238,6 +243,17 @@ export class Input implements ComponentInterface { | |
*/ | ||
@Prop() shape?: 'round'; | ||
|
||
/** | ||
* Set the icon that can be used to represent showing a password. Defaults to `eye`. | ||
*/ | ||
@Prop() showPasswordIcon?: string; | ||
OS-giulianasilva marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
/** | ||
* If `true`, a password toggle icon will appear in the input. | ||
* Clicking the icon toggles the input type between `"text"` and `"password"`, allowing the user to view or hide the input value. | ||
*/ | ||
@Prop() showPasswordToggle = false; | ||
|
||
/** | ||
* If `true`, the element will have its spelling and grammar checked. | ||
*/ | ||
|
@@ -252,7 +268,7 @@ export class Input implements ComponentInterface { | |
/** | ||
* The type of control to display. The default type is text. | ||
*/ | ||
@Prop() type: TextFieldTypes = 'text'; | ||
@Prop({ mutable: true }) type: TextFieldTypes = 'text'; | ||
|
||
/** | ||
* The value of the input. | ||
|
@@ -451,6 +467,7 @@ export class Input implements ComponentInterface { | |
if (input) { | ||
this.value = input.value || ''; | ||
} | ||
|
||
this.emitInputChange(ev); | ||
}; | ||
|
||
|
@@ -541,6 +558,10 @@ export class Input implements ComponentInterface { | |
this.emitInputChange(ev); | ||
}; | ||
|
||
private togglePasswordVisibility = () => { | ||
this.type = this.type === 'text' ? 'password' : 'text'; | ||
}; | ||
|
||
private hasValue(): boolean { | ||
return this.getValue().length > 0; | ||
} | ||
|
@@ -562,7 +583,6 @@ export class Input implements ComponentInterface { | |
|
||
return <div class="counter">{getCounterText(value, maxlength, counterFormatter)}</div>; | ||
} | ||
|
||
/** | ||
* Responsible for rendering helper text, | ||
* error text, and counter. This element should only | ||
|
@@ -665,11 +685,13 @@ export class Input implements ComponentInterface { | |
} | ||
|
||
render() { | ||
const { disabled, fill, readonly, shape, inputId, labelPlacement, el, hasFocus } = this; | ||
const { disabled, fill, readonly, shape, inputId, labelPlacement, el, hasFocus, type } = this; | ||
const mode = getIonMode(this); | ||
const value = this.getValue(); | ||
const inItem = hostContext('ion-item', this.el); | ||
const shouldRenderHighlight = mode === 'md' && fill !== 'outline' && !inItem; | ||
const showPasswordIcon = this.showPasswordIcon || eye; | ||
const hidePasswordIcon = this.hidePasswordIcon || eyeOff; | ||
|
||
const hasValue = this.hasValue(); | ||
const hasStartEndSlots = el.querySelector('[slot="start"], [slot="end"]') !== null; | ||
|
@@ -693,7 +715,6 @@ export class Input implements ComponentInterface { | |
*/ | ||
const labelShouldFloat = | ||
labelPlacement === 'stacked' || (labelPlacement === 'floating' && (hasValue || hasFocus || hasStartEndSlots)); | ||
|
||
return ( | ||
<Host | ||
class={createColorClasses(this.color, { | ||
|
@@ -742,7 +763,7 @@ export class Input implements ComponentInterface { | |
required={this.required} | ||
spellcheck={this.spellcheck} | ||
step={this.step} | ||
type={this.type} | ||
type={type} | ||
value={value} | ||
onInput={this.onInput} | ||
onChange={this.onChange} | ||
|
@@ -771,6 +792,27 @@ export class Input implements ComponentInterface { | |
<ion-icon aria-hidden="true" icon={mode === 'ios' ? closeCircle : closeSharp}></ion-icon> | ||
</button> | ||
)} | ||
{this.showPasswordToggle && !readonly && !disabled && ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The design document described rendering this content as the default content of the Did we experience an issue or additional consideration to not render it as the default slot content? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's talk about this internally. |
||
<button | ||
aria-label="show password" | ||
liamdebeasi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
aria-checked={type === 'text' ? 'true' : 'false'} | ||
aria-controls={inputId} | ||
role="switch" | ||
type="button" | ||
class="input-password-toggle" | ||
onPointerDown={(ev) => { | ||
/** | ||
* This prevents mobile browsers from | ||
* blurring the input when the password toggle | ||
* button is activated. | ||
*/ | ||
ev.preventDefault(); | ||
}} | ||
onClick={this.togglePasswordVisibility} | ||
> | ||
<ion-icon aria-hidden="true" icon={type === 'text' ? hidePasswordIcon : showPasswordIcon}></ion-icon> | ||
</button> | ||
)} | ||
<slot name="end"></slot> | ||
</div> | ||
{shouldRenderHighlight && <div class="input-highlight"></div>} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
<!DOCTYPE html> | ||
<html lang="en" dir="ltr"> | ||
<head> | ||
<meta charset="UTF-8" /> | ||
<title>Input - Password toggle</title> | ||
<meta | ||
name="viewport" | ||
content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no" | ||
/> | ||
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet" /> | ||
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet" /> | ||
<script src="../../../../../scripts/testing/scripts.js"></script> | ||
<script nomodule src="../../../../../dist/ionic/ionic.js"></script> | ||
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script> | ||
<style> | ||
.grid { | ||
display: grid; | ||
grid-template-columns: repeat(5, minmax(250px, 1fr)); | ||
grid-row-gap: 20px; | ||
grid-column-gap: 20px; | ||
} | ||
h2 { | ||
font-size: 12px; | ||
font-weight: normal; | ||
|
||
color: #6f7378; | ||
|
||
margin-top: 10px; | ||
} | ||
@media screen and (max-width: 800px) { | ||
.grid { | ||
grid-template-columns: 1fr; | ||
padding: 0; | ||
} | ||
} | ||
</style> | ||
</head> | ||
|
||
<body> | ||
<ion-app> | ||
<ion-header> | ||
<ion-toolbar> | ||
<ion-title>Input - Password</ion-title> | ||
</ion-toolbar> | ||
</ion-header> | ||
|
||
<ion-content id="content" class="ion-padding"> | ||
<div class="grid"> | ||
<div class="grid-item"> | ||
<ion-input | ||
clear-input="true" | ||
placeholder="Password" | ||
show-password-toggle="true" | ||
type="password" | ||
value="password-test" | ||
> | ||
</ion-input> | ||
</div> | ||
</div> | ||
</ion-content> | ||
</ion-app> | ||
</body> | ||
</html> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
import { expect } from '@playwright/test'; | ||
import { configs, test } from '@utils/test/playwright'; | ||
|
||
configs().forEach(({ title, screenshot, config }) => { | ||
test.describe(title('input: password toggle button'), () => { | ||
test('should render password icon when show-password-toggle', async ({ page }) => { | ||
OS-giulianasilva marked this conversation as resolved.
Show resolved
Hide resolved
|
||
await page.setContent( | ||
` | ||
<ion-input value="abc" type="password" clear-input="true" show-password-toggle="true"></ion-input> | ||
`, | ||
config | ||
); | ||
|
||
const input = page.locator('ion-input'); | ||
await expect(input).toHaveScreenshot(screenshot(`input-with-password-toggle`)); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's break this into
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to keep it as it is, because it is more readable and also matches the current CSS structure of the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sean-perkins what do you think about this topic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.