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

revert(chips): ng-required and ng-change #11219

Merged
merged 2 commits into from
Apr 9, 2018

Conversation

Splaktar
Copy link
Member

@Splaktar Splaktar commented Apr 9, 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
[ ] 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
[x] Other... Please describe: Reverts previous PRs that introduced regressions

What is the current behavior?

md-chips (standalone)
We have multi value supported with separator keys of enter, space and tab.
It works good when we have input like (url1 url2) and hit tab to move out of the field
It doesn't transform into chips when we have input like (url1 url2) and click anywhere outside of the field (this used to work before this sync)
After adding chips, if we remove the chips and retry with any input, it doesn't work. Somehow it makes the model as "undefined"

md-chips with md-autocomplete
We support multi value with separator comma, enter, space and tab.
It works good as long as we don't delete any chip. As soon as we delete the chip, nothing works.
We made sure model looks good even after deleting but somehow md-item-template doesn't get rendered (edited)

Issue Number:
#11217 and #11218

What is the new behavior?

md-add-on-blur works again and deleting chips does not break the ability to add or render new chips.

Does this PR introduce a breaking change?

[x] Yes
[ ] No

It removes two features that were introduced in 1.1.8. New PRs will need to be submitted to add them again with tests that cover these issues.

Other information

TBD

@Splaktar Splaktar added type: bug severity: regression This issue is related to a regression 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. labels Apr 9, 2018
@Splaktar Splaktar added this to the 1.1.9 milestone Apr 9, 2018
@Splaktar Splaktar self-assigned this Apr 9, 2018
@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Apr 9, 2018
@Splaktar Splaktar added the pr: merge ready This PR is ready for a caretaker to review label Apr 9, 2018
@fsmeier
Copy link
Contributor

fsmeier commented Apr 9, 2018

Would it not make more sense to wait 1-2 days before reverting? I it is still an improvement which needs a small bugfix and an additional test-case in my opinion.

@Splaktar
Copy link
Member Author

Splaktar commented Apr 9, 2018

@IMM0rtalis the reason for a quick revert is due to this code affecting Google production apps.

I am with you in hoping that this will only need a small/quick change to resolve and get back in. Unfortunately, I am still unable to reproduce these issues and I am working to get more reproduction information from the internal teams. After reproduction, we'll certainly work on adding test coverage.

@tinayuangao tinayuangao merged commit aaf75cc into master Apr 9, 2018
@Splaktar Splaktar deleted the revertChipsNgRequiredNgChange branch April 10, 2018 01:30
chmelevskij pushed a commit to chmelevskij/material that referenced this pull request Jun 19, 2018
* Revert "feat(chips): trigger ng-change on chip addition/removal (angular#11166)"

This reverts commit 19da42d.

* Revert "feat(md-chips): added validation for ng-required (angular#11125)"

This reverts commit ba0e9fe.
Splaktar added a commit that referenced this pull request Jul 31, 2018
* Revert "feat(chips): trigger ng-change on chip addition/removal (#11166)"

This reverts commit 19da42d.

* Revert "feat(md-chips): added validation for ng-required (#11125)"

This reverts commit ba0e9fe.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ 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: merge ready This PR is ready for a caretaker to review severity: regression This issue is related to a regression type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants