Skip to content

Improve sfNewArray Directive's titleMapValues Binding #705

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

Merged
merged 2 commits into from
Jun 13, 2016

Conversation

kyse
Copy link
Contributor

@kyse kyse commented May 31, 2016

Description

Changes titleMapValues binding in sfNewArray directive. Allows multiple checkboxes controls to target the same ngModel array.

Additional Thoughts

While this helps the main concern of the issue referenced, I'm still a little iffy about this whole setup. It's basically bypassing ngModel when setting values causing trouble for anyone wanting to use a ngModel getterSetter.

Fixes Related issues

Checklist

  • I have read and understand the CONTRIBUTIONS.md file
  • I have searched for and linked related issues
  • I have created test cases to ensure quick resolution of the PR is easier
  • I am NOT targeting main branch
  • I did NOT include the dist folder in my PR

@json-schema-form/angular-schema-form-lead

sfNewArray: Change titleMapValue binding to prevent ngModel target array being cleared. Allows multiple checkboxes (array builder) controls to target the same ngModel target array.
sfNewArray: Adding test for new titleMapValues binding.

kyse added 2 commits May 30, 2016 21:48
…ray being cleared. Allows multiple checkboxes (array builder) controls to target the same ngModel target array.
@Anthropic
Copy link
Member

@kyse thanks for the PR I will try to review it tonight :)

@Anthropic Anthropic merged commit c62ff4b into json-schema-form:development Jun 13, 2016
Anthropic added a commit that referenced this pull request Jun 13, 2016
@Anthropic
Copy link
Member

@kyse I migrated the changed to the webpack branch too. Please feel free to check that out and confirm it works using the dist files, there are some tests failing on that branch still so don't expect everything else to work 100% though ;)

@kyse
Copy link
Contributor Author

kyse commented Jun 15, 2016

Yea, I believe the failing tests are related to the decorators?

@Anthropic
Copy link
Member

@kyse yes there are five failing tests currently four being decorator specific, the other I still have to check. But if there are no major problems when you view the example site then I would be happy to know that :)

@kyse
Copy link
Contributor Author

kyse commented Jun 16, 2016

Examples seem to be working fine.

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.

2 participants