Skip to content
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: Mitigate shareable procedure flyout bug. #2493

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

johnnesky
Copy link
Member

The basics

The details

Resolves

Fixes #2484

Proposed Changes

Prevents deletion of a procedure call block inside a flyout in response to deleting the associated definition.

Reason for Changes

There's a bug where call blocks in flyouts get disposed twice, and the second time results in an error getting thrown that prevents opening the flyout.

Flyouts do not get immediately rerendered or updated in response to a block being deleted. However, when opening a flyout, they do dispose any blocks that they already own before re-rendering themselves. If one of these blocks was already disposed, it'll get disposed a second time (which fails). This PR prevents the block from getting disposed the first time, so that the flyout can successfully clear itself.

It might seem like a problem that a flyout that is open can continue to contain a call block for a procedure that doesn't exist anymore. However, if this call block is dragged to the workspace, the procedure definition will reappear.

This is the same workaround implemented by code.org in: code-dot-org/code-dot-org#63662

Alternatively, we could investigate properly re-rendering the flyout in response to changes in its workspace, but that might involve changes to core blockly?

Test Coverage

I tested manually.

Documentation

N/A

Additional Information

N/A

@johnnesky johnnesky requested a review from a team as a code owner February 22, 2025 04:06
@johnnesky johnnesky requested review from RoboErikG and removed request for a team February 22, 2025 04:06
@RoboErikG
Copy link
Contributor

RoboErikG commented Feb 24, 2025

@johnnesky The change looks good but please make PRs against develop instead of master.

Edit: Whoops, wrong repo for develop. Nevermind!

@RoboErikG RoboErikG merged commit 909de8f into google:master Feb 24, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[block-shareable-procedures] Toolbox breaks after deleting functions
2 participants