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

Core: Trigger validation when next field is already filled out. #1072

Closed
wants to merge 1 commit into from
Closed

Conversation

stijnherreman
Copy link

Fixes #524 (and #244) without causing #521.

@stijnherreman
Copy link
Author

As asked in the comments on #952, a PR.

@jzaefferer
Copy link
Collaborator

Thanks! Will test this next week, along with other new PRs. Currently trying to reserve a few hours each Tuesday.

@jzaefferer
Copy link
Collaborator

Just tested this. #524 said:

On the index demo, edit the markup to fill in the second field with a valid email address. Load that demo, type "abc" into the first field, tab to the next field, tab back, delete the value.

Without your patch, the field doesn't get validated, even after deleting the value. With your patch applied, validation is immediately triggered when typing "a" into the field (same as #521).

Did you run grunt after patching src/core.js? If not, you'd see the code run with your last version in dist, ignoring the actual changes.

@stijnherreman
Copy link
Author

Yes, I run grunt to execute the tests. Without patching src/core.js, the test I added for #524 fails:

events - issue #524
Message: Validation fires and reports invalid input after erasing input.
Actual: 0
Expected: 1
[...]

Warning: 1/1089 assertions failed (6091ms) Use --force to continue.

With the patch, all tests succeed:

1089 assertions passed (6161ms)

Travis also confirms my test results.

The test I added for #521 is supposed to be functionally identical to the test case you provided on that issue, but apparently I made a mistake. You're right that running that fiddle test case with my latest version, validation is triggered immediately after typing a letter in the field.

I don't immediately see my error, I'll take another look this evening.

@jzaefferer
Copy link
Collaborator

Okay, thanks!

@jzaefferer
Copy link
Collaborator

@stijnherreman did you ever get around to testing this again?

@stijnherreman
Copy link
Author

@jzaefferer I haven't yet, some other things on the project I'm on had/have a higher priority unfortunately. I'll see if I can use some of my spare time soon for this.

@jzaefferer
Copy link
Collaborator

@stijnherreman would still be great to address this

@stijnherreman
Copy link
Author

@jzaefferer I'm taking a look this weekend.

@jzaefferer
Copy link
Collaborator

Great, thank you.

@jzaefferer
Copy link
Collaborator

Would still be great to get the issue addressed.

@jzaefferer
Copy link
Collaborator

Closing due to inactivity.

@jzaefferer jzaefferer closed this Jun 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation fails to trigger when next field is already filled out
2 participants