Ensure hover gets disposed when icon label is removed #125667
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There were 2 problems here: - The timeout wasn't cleared, so the hover could reshow - When the setupCustomHover dispose was called, the hover wouldn't be disposed Fixes #125565
aeschli
requested review from
aeschli and
alexr00
and removed request for
aeschli
Contributor
|
Thanks @Tyriar , the changes make sense. I believe the mouseMoveDomEmitter also need to be disposed when the timeout is disposed. Asking @alexr00 for a review as well. I basically moved the code from icon label to a separate class and the one change I did is moving away from the deprecated I'm happy to look into proper disposing, just let me know. |
Member
Author
|
@aeschli thanks, if you can clean this up and merge that would be great 👍 |
Member
|
👍 to what Martin said. All the emitters should also be disposed to be safe. |
Contributor
|
I polished the code a bit and made sure all listeners, the timeout and the hover widget get disposed: 8a17eb3abf7fa6ae906817d8492db2d64fa1fb56 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There were 2 problems here:
Fixes #125565