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

Convert BsForm to Glimmer component #1775

Merged
merged 4 commits into from Apr 28, 2022
Merged

Convert BsForm to Glimmer component #1775

merged 4 commits into from Apr 28, 2022

Conversation

simonihmig
Copy link
Contributor

Also fixes tests to test real validation plugin interface, closes #1338

@simonihmig
Copy link
Contributor Author

@jelhan we might need to update ember-bootstrap-cp-validations and ember-bootstrap-changeset-validations after landing this I guess? For them it might be a breaking change, while users should not be affected as they are not allowed to subclass components. But as we consider them "privileged" addons, we won't bump a major here, I think we agreed on that, right?

@simonihmig
Copy link
Contributor Author

OK, due to the use of a component class (vs. component definition as with the {{component}} helper) tests are failing for older Ember. But I think I know how to fix. Also need to look into Embroider, but not today anymore...

@simonihmig
Copy link
Contributor Author

I tested the changes locally with ember-bootstrap-cp-validations, and everything seems to work fine. ember-bootstrap-changeset-validations however fails. It is still using the legacy object model: https://github.com/kaliber5/ember-bootstrap-changeset-validations/blob/master/addon/components/bs-form.js#L6 🙁

So until that is fixed, I'll keep this open...

@jelhan
Copy link
Contributor

jelhan commented Mar 25, 2022

I tested the changes locally with ember-bootstrap-cp-validations, and everything seems to work fine. ember-bootstrap-changeset-validations however fails. It is still using the legacy object model: https://github.com/kaliber5/ember-bootstrap-changeset-validations/blob/master/addon/components/bs-form.js#L6 slightly_frowning_face

So until that is fixed, I'll keep this open...

Good catch. I created an issue in that repository to track: ember-bootstrap/ember-bootstrap-changeset-validations#39 Sadly I don't have time to pick it up right now.

@simonihmig
Copy link
Contributor Author

I'll merge this now, as the fix on the ember-bootstrap-changeset-validations side has been merged as well. Will then cut a release for both addons soon...

@simonihmig simonihmig merged commit c18a6d8 into master Apr 28, 2022
@simonihmig simonihmig deleted the fix-form-tests branch April 28, 2022 10:21
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.

Tests for validation support in <BsForm> do not test validation plugin interface
2 participants