Skip to content
This repository has been archived by the owner on Sep 5, 2024. It is now read-only.

chips: ng-change with read-only array-model causes problems #11301

Closed
fsmeier opened this issue May 28, 2018 · 6 comments · Fixed by #11310
Closed

chips: ng-change with read-only array-model causes problems #11301

fsmeier opened this issue May 28, 2018 · 6 comments · Fixed by #11310
Assignees
@Splaktar
Labels
has: Pull Request A PR has been created to address this issue P3: important Important issues that really should be fixed when possible. resolution: fixed severity: regression This issue is related to a regression type: bug
Milestone
1.1.10

Comments

@fsmeier
Copy link
Contributor

fsmeier commented May 28, 2018

Bug, enhancement request, or proposal:

ng-change introduced new assignment to model-value instead of just adding/removing the elements of the array. Therefore model-values which just have a getter and no setter are not working anymore.

In some cases it is just wanted to have a property with no setter to not touch the reference to the array but let the values be changed.

CodePen and steps to reproduce the issue:

CodePen Demo 1.1.8 where it still works.
CodePen Demo 1.1.9 where it broke.

Detailed Reproduction Steps:

See codepen + look at The codeline which broke the working code.

What is the expected behavior?

Code does still work after updating to 1.1.9.

What is the current behavior?

Completely dies, latets after removing a chip.

What is the use-case or motivation for changing an existing behavior?

Make it work again without changing the application logic because of a material update

Which versions of AngularJS, Material, OS, and browsers are affected?

  • AngularJS Material: 1.1.9

Is there anything else we should know? Stack Traces, Screenshots, etc.

Introduced with #11237
@Splaktar Splaktar self-assigned this May 30, 2018
@Splaktar
Copy link
Member

Completely dies, latets after removing a chip.

I'm not seeing this. I'm seeing the chip added, but the text being left in the input.
I.e.
screen shot 2018-05-29 at 8 47 41 pm
This is obviously not desirable though.

I'm a little hazy on all of the fine details of using objects with get and set in ng-model. Can you please point me to some documentation where it says that providing the get and not the set in such a configuration is acceptable/advised?

@Splaktar Splaktar added needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community needs: feedback The issue creator or community need to respond to questions in this issue labels May 30, 2018
@Splaktar
Copy link
Member

@Free-Easy heads up that this appears to be triggered by your changes from #11166.

@Splaktar Splaktar mentioned this issue May 30, 2018
Merged
3 tasks
@fsmeier
Copy link
Contributor Author

fsmeier commented May 30, 2018

@Splaktar when you remove the chip and try to add another one: nothing happens for me anymore.

A quick google research did not find some article if it is good or bad.
We do this in some cases to not loose the reference to the array itself -> having some kind of caching.
Nevertheless it is a breaking change of how chips deal with the model: with this change it is the first time they change the model-object itself instead of just manipulating the items.

@Splaktar
Copy link
Member

Splaktar commented Jun 1, 2018

This seems to be related to #11304 which I am working on atm. My guess is that fixing that issue will resolve this as well.

@Splaktar Splaktar mentioned this issue Jun 1, 2018
Closed
@Splaktar Splaktar added type: bug P3: important Important issues that really should be fixed when possible. and removed needs: feedback The issue creator or community need to respond to questions in this issue needs: investigation The cause of this issue is not well understood and needs to be investigated by the team or community labels Jun 1, 2018
@Splaktar Splaktar added this to the 1.1.10 milestone Jun 1, 2018
@Splaktar Splaktar added the severity: regression This issue is related to a regression label Jun 1, 2018
@Splaktar Splaktar mentioned this issue Jun 1, 2018
Merged
3 tasks
@Splaktar Splaktar added the has: Pull Request A PR has been created to address this issue label Jun 1, 2018
@Splaktar
Copy link
Member

Splaktar commented Jun 1, 2018

PR #11310 posted that should resolve this.

Splaktar added a commit that referenced this issue Jun 1, 2018
@Splaktar
fix(chips): regression where chips model gets out of sync with view
0cc804d 
manually call `ngChange` as it won't be triggered by `ngModel` for chips
add `ng-change` test to contact-chips
refinements to chips and contact-chips docs
remove references to `filter-selected` on contact-chips
it was disabled 2 years ago
remove use of `angular.lowercase` which is removed in AngularJS 1.7

Fixes #11304. Fixes #11301.
Splaktar added a commit that referenced this issue Jun 1, 2018
@Splaktar
fix(chips): regression where chips model gets out of sync with view
66c0ccf 
manually call `ngChange` as it won't be triggered by `ngModel` for chips
add `ng-change` test to contact-chips
refinements to chips and contact-chips docs
remove references to `filter-selected` on contact-chips
it was disabled 2 years ago
remove use of `angular.lowercase` which is removed in AngularJS 1.7

Fixes #11304. Fixes #11301.
@fsmeier
Copy link
Contributor Author

fsmeier commented Jun 1, 2018

Yes, this resolves it. Thanks.

Splaktar added a commit that referenced this issue Jun 6, 2018
@Splaktar
fix(chips): regression where chips model gets out of sync with view
7d6db21 
manually call `ngChange` as it won't be triggered by `ngModel` for chips
add `ng-change` test to contact-chips
refinements to chips and contact-chips docs
remove references to `filter-selected` on contact-chips
it was disabled 2 years ago
remove use of `angular.lowercase` which is removed in AngularJS 1.7

Fixes #11304. Fixes #11301.
Splaktar added a commit that referenced this issue Jun 6, 2018
@Splaktar
fix(chips): regression where chips model gets out of sync with view
fd225a1 
manually call `ngChange` as it won't be triggered by `ngModel` for chips
add `ng-change` test to contact-chips
refinements to chips and contact-chips docs
remove references to `filter-selected` on contact-chips
it was disabled 2 years ago
remove use of `angular.lowercase` which is removed in AngularJS 1.7

Fixes #11304. Fixes #11301.
jelbourn pushed a commit that referenced this issue Jun 13, 2018
@Splaktar @jelbourn
fix(chips): regression where chips model gets out of sync with view (#…
74d2445 
…11310)

manually call `ngChange` as it won't be triggered by `ngModel` for chips
add `ng-change` test to contact-chips
refinements to chips and contact-chips docs
remove references to `filter-selected` on contact-chips
it was disabled 2 years ago
remove use of `angular.lowercase` which is removed in AngularJS 1.7

Fixes #11304. Fixes #11301.
Splaktar added a commit that referenced this issue Jul 31, 2018
@Splaktar
fix(chips): regression where chips model gets out of sync with view (#…
656942e 
…11310)

manually call `ngChange` as it won't be triggered by `ngModel` for chips
add `ng-change` test to contact-chips
refinements to chips and contact-chips docs
remove references to `filter-selected` on contact-chips
it was disabled 2 years ago
remove use of `angular.lowercase` which is removed in AngularJS 1.7

Fixes #11304. Fixes #11301.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@Splaktar Splaktar

Labels
has: Pull Request A PR has been created to address this issue P3: important Important issues that really should be fixed when possible. resolution: fixed severity: regression This issue is related to a regression type: bug
Projects
None yet
Milestone
1.1.10
Development

Successfully merging a pull request may close this issue.

fix(chips): regression where chips model gets out of sync with view angular/material
2 participants
@fsmeier @Splaktar