Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Don't bind validation to form controls #29264

Merged
merged 2 commits into from
Aug 27, 2019

Conversation

MartijnCuppens
Copy link
Member

@MartijnCuppens MartijnCuppens commented Aug 17, 2019

Alternative solution for #29228. This PR doesn't bind the .is-valid and .is-invalid classes to the form control.

Issue: https://codepen.io/browner12/pen/zgjKbJ?editors=1000
Solution: https://codepen.io/MartijnCuppens/pen/ExYyXBy?editors=1100

CC @browner12

Closes #29228

@browner12
Copy link
Contributor

Okay, so instead of binding to .form-control .is-valid, we are now just binding to is-valid, correct?

I'm okay with that solution, but it seems like this is probably a breaking change since we're becoming less specific, so it can only go to 5?

@MartijnCuppens
Copy link
Member Author

We're switching from .form-control.is-valid ~ .valid-feedback to .is-valid ~ .valid-feedback. Since ~ only targets siblings, it's pretty safe to merge without introducing breaking changes IMO.

@browner12
Copy link
Contributor

I would agree the risk is extremely low. Wasn't sure what our threshold is in this project.

Copy link
Contributor

@ysds ysds left a comment

Choose a reason for hiding this comment

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

@mdo Do you have any concerns?

Copy link
Member

@mdo mdo left a comment

Choose a reason for hiding this comment

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

No concerns here!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants