fix(material/chips): Async chips with a delay are not highlighted #27399
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
yurakhomitsky
force-pushed
the
fix-selecting-async-chip
branch
from
e0772e2
to
f69992f
Compare
yurakhomitsky
force-pushed
the
fix-selecting-async-chip
branch
from
f69992f
to
9ace971
Compare
mmalerba
added
action: merge
| this._chips.changes.pipe(startWith(null), takeUntil(this._destroyed)).subscribe(() => { | ||
| if (this.value !== undefined) { |
There was a problem hiding this comment.
I think this should be if (this.value === undefined) {. Previously we were checking _pendingInitialValue !== undefined, which implied the value was undefined, as the pending value had not been set. Hopefully making this change will resolve the test failures I'm seeing in google
There was a problem hiding this comment.
This change if (this.value === undefined) fails even more unit tests
There was a problem hiding this comment.
Hmm, in that case I think there's something subtly different between checking if the value is undefined vs the _pendingInitialValue flag. What if we try adding back the flag, but keep the rest of the changes?
There was a problem hiding this comment.
@mmalerba How will I know whether my adjustments solved the issue? Where am I able to view it? You mentioned that some Google tests were failing; what is in there now?
There was a problem hiding this comment.
I will have to rerun the internal tests and report back
There was a problem hiding this comment.
@mmalerba Any updates? I have synced the branch
There was a problem hiding this comment.
It's looking better - there's still 3 failing tests, 1 ExpressionChangedAfterChecked and 2 screenshot diffs that look possibly wrong. It's not clear if these are issues with the changes or issues with the tests themselves. Unfortunately I don't think there's much we can do until I have some time to dig deeper into it
There was a problem hiding this comment.
Thanks for letting me know! In case you need me to update the branch just ping
yurakhomitsky
force-pushed
the
fix-selecting-async-chip
branch
2 times, most recently
from
bd5b54c
to
3ecfb2b
Compare
yurakhomitsky
force-pushed
the
fix-selecting-async-chip
branch
from
3ecfb2b
to
7af22cb
Compare
Fixes a bug in the Angular Material `chips` component where async chips with a delay were not highlighted correctly. Fixes angular#27370
yurakhomitsky
force-pushed
the
fix-selecting-async-chip
branch
from
7af22cb
to
f50bae8
Compare
Fixes a bug in the Angular Material
chipscomponent where async chips with a delay were not highlighted correctly.Fixes #27370