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

Added email validation to ForgotPasswordForm #1997

Merged
merged 9 commits into from
Dec 10, 2019
Merged

Added email validation to ForgotPasswordForm #1997

merged 9 commits into from
Dec 10, 2019

Conversation

gaurav-473
Copy link
Contributor

@gaurav-473 gaurav-473 commented Nov 21, 2019

Description

Added Email validation in forgot password form.

Related Issue

Closes PWA-191.

Verification Steps

Steps to reproduce the behavior:
Go to pwa-studio home page
Click on navigation
Go for Sign in and then click on forgot password link
Forgot password form email field has only required validation

Expected behavior

Forgot password form email field must have email as well as required field validation

@gaurav-473 gaurav-473 changed the title Added Email validation in forgot password form Fixed issue #1998 Added Email validation in forgot password form Nov 21, 2019
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Nov 21, 2019

Messages
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Associated JIRA tickets: PWA-191.

Generated by 🚫 dangerJS against b7188c7

@revanth0212
Copy link
Contributor

Some tests have failed. Please check.

Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

Please fix all the danger warnings.

@supernova-at supernova-at added the version: Patch This changeset includes backwards compatible bug fixes. label Nov 26, 2019
@gaurav-473
Copy link
Contributor Author

Please fix all the danger warnings.

ok I will fix warnings..

sirugh
sirugh previously approved these changes Dec 6, 2019
Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

I fixed the issue, which was a bad import, but this does now apply the email validation to the form.

@sirugh sirugh changed the title Fixed issue #1998 Added Email validation in forgot password form Added email validation to ForgotPasswordForm Dec 6, 2019
@dpatil-magento
Copy link
Contributor

@gauravagarwal1001 Observation - Enter some string and hit submit >> Error will display >> Now make it valid email and hit submit again.
Actual - Error disappeared but email has not been submitted yet. User has to hit Submit again.
Expected - Error should disappear and form should also be submitted at same time.

image

image

image

@revanth0212
Copy link
Contributor

@gauravagarwal1001 Observation - Enter some string and hit submit >> Error will display >> Now make it valid email and hit submit again.
Actual - Error disappeared but email has not been submitted yet. User has to hit Submit again.
Expected - Error should disappear and form should also be submitted at same time.

image image image

Fixed.

@dpatil-magento
Copy link
Contributor

QA Pass. @sirugh Need your thumbs up again.

Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

Good job, team!

@@ -25,8 +26,7 @@ const ForgotPasswordForm = props => {
<TextInput
autoComplete="email"
field="email"
validate={isRequired}
validateOnBlur
Copy link
Contributor

Choose a reason for hiding this comment

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

Before @dpatil-magento merges maybe @revanth0212 can explain why we had to remove this. I feel like the validation should happen on blur and before submit, but it seems like it wasn't happening correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue with validateOnBlur is that it only validates on blur. So once you enter a new valid email address and click on the submit button, the validation is not done yet and the click won't be registered because clicking button === losing focus for the email field but the validation result will only propagate in the next render. That is the reason why @dpatil-magento had to click twice.

@dpatil-magento dpatil-magento merged commit 96a2e32 into magento:develop Dec 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants