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

Search for an item to compare in context of form #637

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sievlev
Copy link

@sievlev sievlev commented Feb 12, 2013

No description provided.

@jzaefferer
Copy link
Collaborator

All the equalTo examples use id-selectors, so it doesn't matter where the element is located. Do you have a specific issue this is addressing where the restriction to the form is needed? That would help to come up with unit tests and documentation.

@sievlev
Copy link
Author

sievlev commented Feb 13, 2013

It's a very error prone to use id-selectors in a complex administrative web applications with a lot of forms. Risk to create a collision is too high. I'm prefer to use in a such interfaces a more simple selectors by name in context of form.

I will add a unit tests and a documentation in next commits.

@sievlev
Copy link
Author

sievlev commented Feb 13, 2013

Another reason to apply my patch is to have a same behavior with 'required' method.

@sievlev
Copy link
Author

sievlev commented Feb 14, 2013

ping ;)

@jzaefferer
Copy link
Collaborator

Thanks for the test. The change is reasonable, too. I'm wondering how many people currently use the equalTo method with a target outside of the current form. In that case, this change would break their setup. Both 1.10 and 1.11 each have severe bugs that broke existing sites, so I don't want to add to the pile with something that I can see coming.

Unless you have a better idea, I'll keep this open for some more time, until I can make a cut to a 2.0 release with more backwards incompatible changes.

@staabm
Copy link
Member

staabm commented Feb 8, 2016

Is there still interest in getting this PR landed?

@staabm staabm force-pushed the master branch 2 times, most recently from d621aab to 3861ce2 Compare September 26, 2016 14:28
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