Skip to content

Don't bind validation to form controls#29264

Merged
MartijnCuppens merged 2 commits into
masterfrom
master-mc-flexible-form-validation
Aug 27, 2019
Merged

Don't bind validation to form controls#29264
MartijnCuppens merged 2 commits into
masterfrom
master-mc-flexible-form-validation

Conversation

@MartijnCuppens

@MartijnCuppens MartijnCuppens commented Aug 17, 2019

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

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

@ysds ysds left a comment

Copy link
Copy Markdown
Contributor

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?

@mdo mdo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No concerns here!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants