Skip to content
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

Improve Confirm Password Error Validation #858

Merged
merged 10 commits into from
Nov 26, 2021
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/silver-comics-know.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@aws-amplify/ui-angular': patch
'@aws-amplify/ui-react': patch
'@aws-amplify/ui': patch
'@aws-amplify/ui-vue': patch
---

Updated the password validation logic, so errors are only display on blur, or when six or more characters is typed for both the confirm password and password fields.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
[disabled]="disabled"
[labelHidden]="labelHidden"
[autocomplete]="inferAutocomplete()"
(setBlur)="onBlur($event)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Angular linter complained here if I called this blur or onblur. So I changed the name to setBlur

></amplify-password-field>

<amplify-text-field
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ export class FormFieldComponent implements OnInit {
return validationError[this.name];
}

public onBlur($event: Event) {
let { name } = <HTMLInputElement>$event.target;

this.authenticator.updateBlur({ name });
}

inferLabel(): string {
const label = this.label || this.attributeMap[this.name]?.label;
return translate<string>(label);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
[value]="initialValue"
[attr.disabled]="disabled ? '' : null"
[autocomplete]="autocomplete"
(blur)="setBlur.emit($event)"
/>
<div class="amplify-field-group__outer-end">
<button
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, Input } from '@angular/core';
import { Component, EventEmitter, Input, Output } from '@angular/core';
import { translate } from '@aws-amplify/ui';
import { nanoid } from 'nanoid';

Expand All @@ -16,6 +16,7 @@ export class PasswordFieldComponent {
@Input() placeholder = '';
@Input() required = true;
@Input() labelHidden = false;
@Output() setBlur = new EventEmitter<Event>();

public type: 'text' | 'password' = 'password';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ export class AuthenticatorService implements OnDestroy {
return this._sendEventAliases.updateForm;
}

public get updateBlur() {
return this._sendEventAliases.updateBlur;
}

public get resendCode() {
return this._sendEventAliases.resendCode;
}
Expand Down
5 changes: 4 additions & 1 deletion packages/e2e/cypress/integration/common/sign-up.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ Given('I click {string}', (text: string) => {
});

When('I confirm my password', () => {
cy.findInputField('Confirm Password').type(Cypress.env('VALID_PASSWORD'));
cy.findInputField('Confirm Password')
.type(Cypress.env('VALID_PASSWORD'))
.blur()
.wait(100);
});
Comment on lines +10 to 14
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is something odd timing wise, that I cannot duplicate in real life only in cypress. Where if you send the blur at the exact same time as you click the acknowledgement button it causes the validation not to fire and the button won't light up. That's why I added a wait in here. Not sure if there is a better way of handling the blur...

Copy link
Contributor

@wlee221 wlee221 Nov 24, 2021

Choose a reason for hiding this comment

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

Weird, we can keep an eye on it. Fortunately (and unfortunately) this isn't human-reproducible, so I'm good with the changes!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍


Then('I see {string} as an input field', (name: string) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,15 @@ import { Button, Flex, Heading, PasswordField, Text } from '../../..';
import { isInputOrSelectElement, isInputElement } from '../../../helpers/utils';

export const ForceNewPassword = (): JSX.Element => {
const { _state, error, isPending, toSignIn, submitForm, updateForm } =
useAuthenticator();
const {
_state,
error,
isPending,
toSignIn,
submitForm,
updateForm,
updateBlur,
} = useAuthenticator();
const { validationError } = getActorContext(_state) as SignInContext;

const handleChange = (event: React.FormEvent<HTMLFormElement>) => {
Expand All @@ -29,6 +36,11 @@ export const ForceNewPassword = (): JSX.Element => {
submitForm();
};

const handleBlur = (event: React.FocusEvent<HTMLInputElement>) => {
const { name } = event.target;
updateBlur({ name });
};

return (
<form
data-amplify-form=""
Expand All @@ -49,6 +61,7 @@ export const ForceNewPassword = (): JSX.Element => {
label={translate('Password')}
labelHidden={true}
hasError={!!validationError['confirm_password']}
onBlur={handleBlur}
/>
<PasswordField
data-amplify-confirmpassword
Expand All @@ -58,6 +71,7 @@ export const ForceNewPassword = (): JSX.Element => {
label={translate('Confirm Password')}
labelHidden={true}
hasError={!!validationError['confirm_password']}
onBlur={handleBlur}
/>

{!!validationError['confirm_password'] && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
import { isInputOrSelectElement, isInputElement } from '../../../helpers/utils';

export const ConfirmResetPassword = (): JSX.Element => {
const { _state, submitForm, updateForm } = useAuthenticator();
const { _state, submitForm, updateForm, updateBlur } = useAuthenticator();
const { validationError } = getActorContext(_state) as ResetPasswordContext;

const headerText = translate('Reset your password');
Expand All @@ -41,6 +41,11 @@ export const ConfirmResetPassword = (): JSX.Element => {
submitForm();
};

const handleBlur = (event: React.FocusEvent<HTMLInputElement>) => {
const { name } = event.target;
updateBlur({ name });
};

return (
<form
data-amplify-form=""
Expand All @@ -63,6 +68,7 @@ export const ConfirmResetPassword = (): JSX.Element => {
name="password"
label={passwordText}
labelHidden={true}
onBlur={handleBlur}
/>
<PasswordField
data-amplify-confirmpassword
Expand All @@ -72,6 +78,7 @@ export const ConfirmResetPassword = (): JSX.Element => {
label={confirmPasswordLabel}
labelHidden={true}
hasError={!!validationError['confirm_password']}
onBlur={handleBlur}
/>

{!!validationError['confirm_password'] && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { PasswordField, PhoneNumberField, Text, TextField } from '../../..';
import { UserNameAlias as UserNameAliasComponent } from '../shared';

export function FormFields() {
const { _state } = useAuthenticator();
const { _state, updateForm, updateBlur } = useAuthenticator();
const { country_code, validationError } = getActorContext(
_state
) as SignUpContext;
Expand All @@ -23,6 +23,11 @@ export function FormFields() {
// Only 1 is supported, so `['email', 'phone_number']` will only show `email`
const loginMechanism = fieldNames.shift() as LoginMechanism;

const handleBlur = (event: React.FocusEvent<HTMLInputElement>) => {
const { name } = event.target;
updateBlur({ name });
};

return (
<>
<UserNameAliasComponent
Expand All @@ -39,6 +44,7 @@ export function FormFields() {
label={translate('Password')}
labelHidden={true}
placeholder={translate('Password')}
onBlur={handleBlur}
/>

<PasswordField
Expand All @@ -50,6 +56,7 @@ export function FormFields() {
label={translate('Confirm Password')}
labelHidden={true}
name="confirm_password"
onBlur={handleBlur}
/>

{validationError['confirm_password'] && (
Expand Down
1 change: 1 addition & 0 deletions packages/ui/src/helpers/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ export const getSendEventAliases = (send: Sender<AuthEvent>) => {
signOut: sendToMachine('SIGN_OUT'),
submitForm: sendToMachine('SUBMIT'),
updateForm: sendToMachine('CHANGE'),
updateBlur: sendToMachine('BLUR'),

// Actions that don't immediately invoke a service but instead transition to a screen
// are prefixed with `to*`
Expand Down
11 changes: 11 additions & 0 deletions packages/ui/src/machines/authenticator/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export const clearAttributeToVerify = assign({
export const clearChallengeName = assign({ challengeName: (_) => undefined });
export const clearError = assign({ remoteError: (_) => '' });
export const clearFormValues = assign({ formValues: (_) => ({}) });
export const clearFormControls = assign({ formControls: (_) => ({}) });
export const clearUnverifiedAttributes = assign({
unverifiedAttributes: (_) => undefined,
});
Expand Down Expand Up @@ -110,3 +111,13 @@ export const handleInput = assign({
};
},
});

export const handleBlur = assign({
formControls: (context, event: AuthEvent) => {
const { name } = event.data;
return {
...context['formControls'],
[`touched_${name}`]: true,
};
},
});
26 changes: 21 additions & 5 deletions packages/ui/src/machines/authenticator/actors/resetPassword.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import { runValidators } from '../../../validators';
import {
clearError,
clearFormValues,
clearFormControls,
clearUsername,
clearValidationError,
handleInput,
handleBlur,
setFieldErrors,
setRemoteError,
setUsername,
Expand All @@ -32,13 +34,14 @@ export const resetPasswordActor = createMachine<
},
resetPassword: {
initial: 'edit',
exit: ['clearFormValues', 'clearError'],
exit: ['clearFormValues', 'clearError', 'clearFormControls'],
states: {
edit: {
entry: sendUpdate(),
on: {
SUBMIT: 'submit',
CHANGE: { actions: 'handleInput' },
BLUR: { actions: 'handleBlur' },
},
},
submit: {
Expand All @@ -59,7 +62,12 @@ export const resetPasswordActor = createMachine<
},
confirmResetPassword: {
type: 'parallel',
exit: ['clearFormValues', 'clearError', 'clearUsername'],
exit: [
'clearFormValues',
'clearError',
'clearUsername',
'clearFormControls',
],
states: {
validation: {
initial: 'pending',
Expand All @@ -85,6 +93,10 @@ export const resetPasswordActor = createMachine<
actions: 'handleInput',
target: '.pending',
},
BLUR: {
actions: 'handleBlur',
target: '.pending',
},
},
},
submission: {
Expand All @@ -96,6 +108,7 @@ export const resetPasswordActor = createMachine<
SUBMIT: 'validate',
RESEND: 'resendCode',
CHANGE: { actions: 'handleInput' },
BLUR: { actions: 'handleBlur' },
},
},
validate: {
Expand Down Expand Up @@ -150,9 +163,11 @@ export const resetPasswordActor = createMachine<
actions: {
clearError,
clearFormValues,
clearFormControls,
clearUsername,
clearValidationError,
handleInput,
handleBlur,
setFieldErrors,
setRemoteError,
setUsername,
Expand All @@ -175,9 +190,10 @@ export const resetPasswordActor = createMachine<
return Auth.forgotPasswordSubmit(username, code, password);
},
async validateFields(context, event) {
return runValidators(context.formValues, [
defaultServices.validateConfirmPassword,
]);
return runValidators(
{ ...context.formValues, ...context.formControls },
[defaultServices.validateConfirmPassword]
);
},
},
}
Expand Down
Loading