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

Feature/update array #304

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jscharett
Copy link
Contributor

PR Type

What changes does this PR include (check all that apply)?
[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Build process changes
[ ] Documentation changes
[ ] Other... please describe:

Related issue / current behavior

Two issue here:

  1. The enum validator was broken due to the build replacing _.isEqual with simply isEqual when it is already defined as a local fn. This caused a circular problem
  2. The JsonSchemaFormService.updateValue did not handle arrays well, specifically add/removing values. The checkboxes widget seemingly ran into this and created a separate updateArrayCheckboxList method

New behavior

enum validator conflict resolved
JsonSchemaFormService.updateArrayCheckboxList renamed to updateArray and set as protected. JsonSchemaFormService.updateValue now calls this internally for arrays.

Does this PR introduce a breaking change?

[x] Yes; JsonSchemaFormService.updateArrayCheckboxList no longer available

During the build, _.isEqual is getting replaced with isEqual
from the import.  This is conflicting with the local isEqual
causing a circular issue.
@mcnevan
Copy link

mcnevan commented Aug 16, 2018

@jscharett it's great, but could you help me please?
in item I have a nested array, which can changed.
and when i try to updateValue I get an error

Error:
There are no form controls registered with this array yet. If you're using ngModel,
you may want to check next tick (e.g. use setTimeout).

I've tried to exchange setValue to patchValue, but my nested array was removed

@jscharett
Copy link
Contributor Author

@mcnevan Does this PR break your scenario with the nested array, or just not fix it?

@mcnevan
Copy link

mcnevan commented Aug 16, 2018

@jscharett oh, I think just not fix it

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

Successfully merging this pull request may close these issues.

None yet

2 participants