-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(module:tooltip): fix tooltip accessing destroyed view #4387
Conversation
Deploy preview for ng-zorro-master ready! Built with commit 455d90c |
Codecov Report
@@ Coverage Diff @@
## master #4387 +/- ##
==========================================
+ Coverage 92.11% 92.23% +0.12%
==========================================
Files 521 521
Lines 11078 11082 +4
Branches 2010 2010
==========================================
+ Hits 10205 10222 +17
+ Misses 446 436 -10
+ Partials 427 424 -3
Continue to review full report at Codecov.
|
Tested with the reproduction in #4386. This fix works. |
616f67f
to
455d90c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@Svetomechc Could you make a reproduction? It looks that calling |
Okay, @wendzhue, created a reproduction. This is a pretty serious error which made me revert to 8.4.1. UPD: Turns out it happens in previous versions too. I didn't notice it before because the error only happens in development mode, not in production. |
@Svetomechc Great! I will test it with my latest fix. Thank you very much. |
@wendzhue no problem! I should be the one thanking you for maintaining this awesome library (: |
It should be fixed in #4418. |
@wendzhue thx! Looking forward to 8.5.1 :3 |
close #3875
close #4317
close #4386
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information