Skip to content

Core: Bind the blur event just once in equalTo rule #1709

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 13, 2016
Merged

Core: Bind the blur event just once in equalTo rule #1709

merged 1 commit into from
Feb 13, 2016

Conversation

Arkni
Copy link
Member

@Arkni Arkni commented Feb 11, 2016

bind the event just once, avoiding the unbind-rebind overhead.
Also, unbind it when destroying the plugin.

Ref #1704
Ref #1707

@staabm
Copy link
Member

staabm commented Feb 12, 2016

@Arkni do you consider adding a unit test? I am also ok to land this as is.

@RoBorg could you confirm that this patch fixes the issue you observed?

@RoBorg
Copy link

RoBorg commented Feb 12, 2016

@staabm yup that will fix it

@Arkni
Copy link
Member Author

Arkni commented Feb 12, 2016

@staabm I will try to extend destroy's unit test.

@Arkni Arkni force-pushed the equalTo-blur-event branch from 3d30df2 to 92c51c6 Compare February 12, 2016 15:24
@Arkni
Copy link
Member Author

Arkni commented Feb 12, 2016

I added some tests :)

@staabm
Copy link
Member

staabm commented Feb 12, 2016

Nice. Did you make sure the tests fail without the patch?

@Arkni
Core: Bind the blur event just once in equalTo rule
ef22360 
bind the event just once, avoiding the unbind-rebind overhead.
Also, unbind it when destroying the plugin.

Ref jquery-validation#1704
Ref jquery-validation#1707
@Arkni Arkni force-pushed the equalTo-blur-event branch from 92c51c6 to ef22360 Compare February 12, 2016 16:59
@Arkni
Copy link
Member Author

Arkni commented Feb 12, 2016

Yep, I did :)

@staabm staabm merged commit ef22360 into jquery-validation:master Feb 13, 2016
@staabm
Copy link
Member

staabm commented Feb 13, 2016

merged thx. not sure why git wasnt able to fast forward it... required a merged commit..

@Arkni Arkni deleted the equalTo-blur-event branch February 13, 2016 11:58
@Arkni
Copy link
Member Author

Arkni commented Feb 13, 2016

I'm not sure why this happen.

The Git's docs give us a good reason:

Because the commit on the branch you’re on isn’t a direct ancestor of the branch you’re merging in, Git has to do some work. In this case, Git does a simple three-way merge, using the two snapshots pointed to by the branch tips and the common ancestor of the two.

@Arkni
Copy link
Member Author

Arkni commented Feb 13, 2016

If you want, you can reset the master and delete the last 2 commit (including the merge commit) and I will create another PR based on new master.

@staabm
Copy link
Member

staabm commented Feb 13, 2016

No worries, its fine

@Arkni Arkni mentioned this pull request Sep 11, 2017
Merged
@snyk-bot snyk-bot mentioned this pull request Aug 22, 2020
Open
@snyk-bot snyk-bot mentioned this pull request Apr 23, 2021
Open
@snyk-bot snyk-bot mentioned this pull request Apr 7, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Core Type: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Arkni @staabm @RoBorg