Skip to content

destroyStrategy isn't followed when a field is removed from DOM. #508

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

Closed
jbsaff opened this issue Aug 17, 2015 · 5 comments
Closed

destroyStrategy isn't followed when a field is removed from DOM. #508

jbsaff opened this issue Aug 17, 2015 · 5 comments

Comments

@jbsaff
Copy link
Contributor

jbsaff commented Aug 17, 2015

http://schemaform.io/examples/bootstrap-example.html#/976bfc96771600614451

Toggle the checkbox, and the condition-limited field remains in the model.

Without the new builder:
If the condition is applied to a container (such as the deprecated "conditional" type, or a field set), then the fields are cleaned up following the appropriate destroyStrategy. But fields that have a condition applied directly as a form option have the $destroy listener set on the bootstrap-decorator wrapper tag, which is never actually removed from the DOM by conditions.

With the new builder:
I'm seeing some fields work as expected (triggering to show a text field when a certain value in a drop down list is selected), but checkbox list fields don't appear to be following the condition behavior. I'll need to dig into this more for 0.8.5+ development.

@davidlgj
Copy link
Contributor

Ohhh... interesting! Thanks for testing the new builder. I'll check it out. In the new builder the sf-field directive should take care of that. I'll try to check it out soon.

@jbsaff
Copy link
Contributor Author

jbsaff commented Aug 17, 2015

Okay, thanks for the follow-up. It sounds like the sf-field directive is what we talked about in #371. Is that for the new builder only? My quick perusal through the code seemed to indicate that was a possibility.

@davidlgj
Copy link
Contributor

@jbsaff No its not quite what we talked about in #371. But you are right it's for the new builder. The sf-field directive replaces some of the things that the bootstrap-decorator (or any other decorator directive) does. Most importantly create a scope and set the appropriate form definition on it under the name form so that the template can use it. It's is added to first child of the template for each field.

jbsaff referenced this issue Aug 17, 2015
From the new bootstrap implementation.
jbsaff referenced this issue in json-schema-form/angular-schema-form-bootstrap Aug 17, 2015
@jbsaff
Copy link
Contributor Author

jbsaff commented Aug 17, 2015

Okay, sounds like a reasonable spot to put the functionality, and looks really nice with the new builder.

Found the primary problem with checkboxes in the new builder:
https://github.com/Textalk/angular-schema-form-bootstrap/blob/develop/src/bootstrap-decorator.js#L43

Condition isn't in the dependency list, so it's never checked for the checkboxes type. It looks like it was removed in commit e24d164534e4d6d872e14b0e6f5d9a1b37541dfc. I've added line comments in a couple places in each project related to this issue.

@davidlgj
Copy link
Contributor

Ok, so I did a small release of ASF and one of the new bootstrap decorator with the condition in the list of builders. Thanks for the help @jbsaff!

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

No branches or pull requests

2 participants