cursor plugin: fix element removal in destroy #2462
Merged
Conversation
This file contains 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
When the plugin destroy() method was called, I got an error because removeChild was called with something that wasn't a Node, for this.showTime and this.cursor. It seems that these members are Proxy objects, and the removeChild doesn't swallow that. Changing from removeChild to remove() on the Proxy objects seems to fix this.
thijstriemstra
changed the title
thijstriemstra
suggested changes
Feb 27, 2022
There was a problem hiding this comment.
thanks @JanMisker Can you also add a changelog entry (for new 6.0.3) with a reference to #2460?
|
Check, added an entry in the changelog. |
|
now there's a merge conflict, probably in changelog |
|
I can't really resolve that change I think, but adjusted CHANGES.md to be just the addition of 'my line'. |
|
Thanks @JanMisker, much appreciated! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When the plugin
destroy() method get called, I got an error becauseremoveChild()was called with something that wasn't aNode, forthis.showTimeandthis.cursor.Short description of changes:
It seems that
showTimeandcursorareProxyobjects, and theremoveChild()doesn't swallow that. Changing from e.g.removeChild(this.cursor)tothis.cursor.remove()on the Proxy objects seems to fix this.Breaking in the external API:
n/a
Breaking changes in the internal API:
n/a
Todos/Notes:
n/a
Related Issues and other PRs:
fixes #2460