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: Let old block factory overwrite user defined blocks. #8605

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

johnnesky
Copy link
Member

@johnnesky johnnesky commented Sep 30, 2024

The basics

The details

Resolves

Fixes #6820

Proposed Changes

Makes a record of all of the reserved block type names that are required by the old block factory workspace on startup, and uses that to determine whether a user-defined block type has a conflicting name, instead of assuming the current definitions in Blockly.Blocks is an authoritative proxy for the reserved types.

Reason for Changes

The desired behavior is to prevent replacing critical blocks that are used by the block factory, like core Blockly blocks or even block-factory-specific blocks like factory_base. The existing strategy is to refuse to overwrite any block type that is currently recognized by the definitions in Blockly.Blocks, and to support this strategy, whenever the factory defines a user-defined block in this mapping, it always removes the block immediately after adding it so that user-defined block types become available again while critical block types remain reserved. However, there are ways that user-defined block types can "leak" into Blockly.Blocks permanently, after which point the block factory will mistake it for a critical block that shouldn't be replaced. For example, if there are already user-defined blocks saved in the block library when the block factory is reloaded or you switch between the factory/exporter tabs, all user-defined blocks will be added to Blockly.Blocks without being removed, so you're not allowed to make edits to saved blocks after reloading or switching tabs.

Test Coverage

There are no tests for the old block factory. It seems to work in my manual testing.

Documentation

N/A

Additional Information

There are at least two publicly hosted copies of the old block factory that ought to be updated:
https://blockly-demo.appspot.com/static/demos/blockfactory/index.html
https://google.github.io/blockly/demos/blockfactory/index.html

@johnnesky johnnesky requested a review from a team as a code owner September 30, 2024 16:05
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels Sep 30, 2024
@johnnesky johnnesky merged commit 9b3603a into google:develop Oct 2, 2024
12 checks passed
@johnnesky johnnesky deleted the nesky_block_factory branch October 2, 2024 00:01
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels Oct 4, 2024
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.

blockly developer tools block display is broken
2 participants