10000 fix: disposing workspace comments. by johnnesky · Pull Request #7264 · google/blockly · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: disposing workspace comments. #7264

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

Merged

Conversation

johnnesky
Copy link
Member
@johnnesky johnnesky commented Jul 8, 2023

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

Fixes #7177

Proposed Changes

Makes it the sole responsibility of the WorkspaceComment superclass to mark the comment as disposed, instead of letting the subclass mark it as disposed before the superclass has a chance to handle disposing it.

Behavior Before Change

The superclass's dispose method did not actually run, and the comment would not be completely disposed.

Behavior After Change

Deleting a comment now fully disposes it.

Test Coverage

Previously there was test coverage for disposing WorkspaceComment, but not for disposing WorkspaceCommentSvg. I added a method testing the latter.

@johnnesky johnnesky requested a review from a team as a code owner July 8, 2023 00:51
@johnnesky johnnesky requested a review from maribethb July 8, 2023 00:51
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels Jul 8, 2023
@rachel-fenichel rachel-fenichel merged commit 75ea8d9 into google:develop Jul 10, 2023
@johnnesky johnnesky deleted the fix_workspace_comment_disposing branch July 28, 2023 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workspace comments aren't deletable and can cause infinite loops.
3 participants
0