fixes #104

[kamilkp]

No description provided.

[beradrian]

I don't think removing that check is good. Maybe checking oldValue against undefined or something similar. Also, could you please write a test for this case?

[kamilkp]

What harm could it do? (Not having that check)

Looks like only an optimization to me. In the "add" and "remove" function there are also checks to prevent adding a duplicate and removing something that is not there.

Kamil Pękala

Dnia 21 lis 2015 o godz. 20:30 Adrian Ber [email protected] napisał(a):

I don't think removing that check is good. Maybe checking oldValue against undefined or something similar. Also, could you please write a test for this case?

—
Reply to this email directly or view it on GitHub.

[beradrian]

Indeed, an optimization. But if it can be fixed without removing that optimization, even better.

[kamilkp]

But that optimization actually only saves us the check of whether the item already is in the array or already isn't. It's not heavy computation that would have any impact. Also this situation afaik should happen only when the component is initialized. Why else would a watcher function be fired with old and new values being equal later on?

Also - can you think of another solution that would fix my bug?

[kevinrenskers]

I'm having the exact same problem and would love to see this PR merged. Don't really see the problem with removing this optimisation either, and no other (or better) way of fixing this show-stopping bug.

[beradrian]

What do you think about this http://plnkr.co/edit/bQmry6aaCZzR8TQmASLW?p=preview ?
This fix keeps the optimization and also doesn't call beforeChange and change with the initial values.

Please comment on these two solutions so I can include the fix and release a new version asap. Many thanks for your involvement.

[kamilkp]

Looks like it solves my problem so, fine by me! Thanks.

[kevinrenskers]

Awesome, thanks! Can you ping us when a new version is released?

[beradrian]

I added this comment to issue #104 too.

What happens if there is an inconsistency between ngModel and checklistModel? Let's say that ngModel is true, but checklistModel doesn't contain the value? Or the other way around?

Why don't you use checklistModel for initialization?

As soon as we clarify this I can release a new version.

[beradrian]

@kevinrenskers, you can use https://sibbell.com to watch for GitHub releases.

[kamilkp]

I think the checklist model should just always reflect the state of the checkboxes. Why don't i use it for initialization? I guess i could but it seems like a workaround.

Kamil Pękala

Dnia 4 gru 2015 o godz. 10:54 Adrian Ber [email protected] napisał(a):

I added this comment to issue #104 too.

What happens if there is an inconsistency between ngModel and checklistModel? Let's say that ngModel is true, but checklistModel doesn't contain the value? Or the other way around?

Why don't you use checklistModel for initialization?

As soon as we clarify this I can release a new version.

—
Reply to this email directly or view it on GitHub.

[beradrian]

Actually, in this case using ngModel for initialization is more like a workaround. All the checkboxes are bound to checklistModel and this replaces here the ngModel. I implemented ngModel only to benefit from other Angular features.
In this case I have the issue of inconsistency between these two models. Which one should take priority?

[kamilkp]

Ah I see what you mean. I use checklist-model in this case just do have validation, i.e. I don't save to db the actual checklist but the boolean flags itself. I use the checklist to validate its length because i require that at least one checkbox is selected. I will try to initialize the checklist with the values of the boolean flags. Unless you see a better solution to my scenario?

[kamilkp]

Preinitializing checklist fixed the problem