Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

feat(chips): trigger ng-change on chip addition/removal #11237

Merged
merged 1 commit into from
Apr 17, 2018

Conversation

Splaktar
Copy link
Member

@Splaktar Splaktar commented Apr 16, 2018

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

AngularJS' ng-change directive is not supported with chips or contact chips.

Issue Number:
Closes #11161
Closes #3857

What is the new behavior?

AngularJS' ng-change directive is now supported with chips and contact chips.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

  • Add test of ng-change for md-chips
  • Add docs regarding ng-change for md-chips and md-contact-chips
  • Add demo for ng-change on md-chips
  • Add demo for ng-change on md-contact-chips

This PR brings back the functionality from @Free-Easy in PR #11166 which was reverted in #11219 due to #11218. However, #11218 appears to be caused by application-specific code and not a regression caused by this feature.

As mentioned in #11218 (comment), the team wants to test and try to reproduce the issue again so that they can investigate their application code. To do this, they need this change back in master.

* Add test of `ng-change` for `md-chips`
* Add docs regarding `ng-change` for `md-chips` and `md-contact-chips`
* Add demo for ng-change on `md-chips`
* Add demo for ng-change on `md-contact-chips`

Closes #11161
Closes #3857
@Splaktar Splaktar added type: feature pr: merge ready This PR is ready for a caretaker to review g3: reported The issue was reported by an internal or external product team. P1: urgent Urgent issues that should be addressed in the next minor or patch release. pr: lgtm This PR has been approved by the reviewer labels Apr 16, 2018
@Splaktar Splaktar added this to the 1.1.9 milestone Apr 16, 2018
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@googlebot googlebot added the cla: no PR author needs to sign Google's CLA: https://opensource.google.com/docs/cla/ label Apr 16, 2018
@Splaktar Splaktar removed the cla: no PR author needs to sign Google's CLA: https://opensource.google.com/docs/cla/ label Apr 16, 2018
@Splaktar
Copy link
Member Author

@Free-Easy can you please verify that you confirm that these commits can be added back to the repo?

@chenlijun99
Copy link
Contributor

I confirm.

@Thaina
Copy link
Contributor

Thaina commented Jun 13, 2018

Using 1.1.9 which seem like it trigger ng-change properly but when I set md-enable-chip-edit with md-max-chips and try to edit the chip. It just erase the chip I was edited

@Splaktar
Copy link
Member Author

@Thaina that is fixed in PR #11323.

@Thaina
Copy link
Contributor

Thaina commented Jun 17, 2018

@Splaktar When would the fix released?

@Splaktar
Copy link
Member Author

@Thaina it was scheduled for last week, but we ran into some issues with the merging and sync. We're hopefully working through the last one now and plan to release 1.1.10 this coming week.

chmelevskij pushed a commit to chmelevskij/material that referenced this pull request Jun 19, 2018
* Add test of `ng-change` for `md-chips`
* Add docs regarding `ng-change` for `md-chips` and `md-contact-chips`
* Add demo for ng-change on `md-chips`
* Add demo for ng-change on `md-contact-chips`

Closes angular#11161
Closes angular#3857
Splaktar added a commit that referenced this pull request Jul 31, 2018
* Add test of `ng-change` for `md-chips`
* Add docs regarding `ng-change` for `md-chips` and `md-contact-chips`
* Add demo for ng-change on `md-chips`
* Add demo for ng-change on `md-contact-chips`

Closes #11161
Closes #3857
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
g3: reported The issue was reported by an internal or external product team. P1: urgent Urgent issues that should be addressed in the next minor or patch release. pr: lgtm This PR has been approved by the reviewer pr: merge ready This PR is ready for a caretaker to review type: feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(chips): add support for ng-change feat(contact-chips): add support for ng-change
5 participants