Navigation Menu

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

Trigger revalidation on change event #506

Closed
wants to merge 1 commit into from

Conversation

thenewguy
Copy link

I noticed, when using jQuery Mobile's custom select menus, that validation errors due to the "required" validator were not being removed when I made a selection. This pull request adds a change event listener to fix this bug.

@jzaefferer
Copy link
Collaborator

Thanks for the contribution. In your commit, you change events bound to radio and checkbox inputs, but your PR description mentions custom select menus. Is it just a bad description?

Also, a unit test would help. When you add that, be sure to also run grunt to check for lint warnings.

@thenewguy
Copy link
Author

The description could probably use some work, but I did encounter this using JQuery Mobile's custom select menu widgets.

If you try to use jquery-validation with JQM's custom select menu popup widget, once a validation error is shown, it will not be removed.

I tried to create a test case, but had trouble creating a test case for this. It is easy to see visually, but I couldn't put a test case together.

Also, doesn't the selector "[type='radio'], [type='checkbox'], select, option" actually add the listener to radio buttons, checkboxes, select menus and option elements?

It has been a little while since I did this, so I do not remember internally how JQM renders the widget in html. However, this change fixes the issue.

---- edit ----

I think the commit diff shown is misleading: thenewguy@36214da

I think you were talking about this line: https://github.com/thenewguy/jquery-validation/blob/36214da28a975668fa9936c5b00a20b82bef55be/jquery.validate.js#L338

The commit diff makes that line look like it only applies to radio buttons, but it doesn't when you look at it in context.

@jzaefferer
Copy link
Collaborator

You're right, I missed that. This brings up an interesting issue, one that users of jQuery UI will also have to deal with.

@scottgonzalez what do you think is the right approach here? jQuery UI's selectmenu won't trigger a change event, apparently unlike the jQuery Mobile one. Will the response for UI users be to implement custom bindings to provide the integration between custom widgets and, in this case, form validation?

@scottgonzalez
Copy link

Yes, users will need to setup custom bindings. Or, preferably, someone will build an extension that makes jQuery UI + validation automatically work.

@jzaefferer
Copy link
Collaborator

@thenewguy could you try to trigger the click event the validation plugin expects from a change-event (or whatever event jQuery Mobile exposes) to setup the binding?

@jzaefferer
Copy link
Collaborator

See my last comment above.

@jzaefferer jzaefferer closed this Feb 27, 2013
@thenewguy
Copy link
Author

@jzaefferer I don't understand why you wouldn't listen to the change event? It seems more appropriate than the click event doesn't it? You know the value needs to be re-validated if it "changes".

@jzaefferer
Copy link
Collaborator

The native change is even is terribly unreliable. Native selects are inconsistent with triggering it, so the click event ends up being more useful.

The integration with jQuery Mobile can't be that big of a problem. Though I'd be happy to revisit this issue if you can show that it actually is a big deal.

@scottgonzalez
Copy link

When does the change event not get fired during user interaction?

@jzaefferer
Copy link
Collaborator

With selects, sometimes the change event is triggered on click, sometimes on blur. With text inputs, using the browser's autocomplete doesn't trigger a change event at all, similar to using jQuery UI's autocomplete.

@scottgonzalez
Copy link

Wonderful. Is change at least reliable in oldIE? If so, we could try to get the other browsers to change.

@jzaefferer
Copy link
Collaborator

I don't know, yet. To keep track of that, as it isn't restrict to this plugin, and to keep track of a few other issues, I've created a public issue tracker. Issue for the change event: jzaefferer/standards-tracker#1
If you have feedback on one of others, too, let me know!

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.

None yet

3 participants