fix: fix block factory in manual mode #6533
Merged
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.
The basics
npm run format
andnpm run lint
The details
Resolves
fixes #6175 (and then the other problems with block factory in manual mode)
Proposed Changes
Behavior Before Change
Block factory is broken in manual mode.
factory_base
and the block disappears from the main workspaceBehavior After Change
factory_base
you get a console error and warning text on the block.Reason for Changes
Manual mode broke in #5571
The reason is we're removing all the blocks from Blockly.Blocks during the execution of
BlockFactory.updatePreview
. In normal mode that's fine, but in manual mode the following happens:BlockFactory.updateBlocksFlag
istrue
due to the event listener on the text areaupdatePreview
deletes all the blocks fromBlockly.Blocks
so that we can detect the new one that's added.updatePreview
callsBlockFactory.updateGenerator
FactoryUtils.getGeneratorStub
BlockFactory.updateBlocksFlag
is true, the main workspace of block factory is rebuilt. basically, this is the step that updates the block factory workspace to be in sync with the text area now that we're editing the text area directly. Why is that buried in this generator function, I don't know. but while rebuilding, we need to have the definition offactory_base
and other blocks available. that definition is not available because we've just deleted everything fromBlockly.Blocks
, so that causes the error about missing definition forfactory_base
.Even if you check out a commit from before #5571 such as
5f72a61b2dba14ec86c8a128060cfb38c29cde00
things didn't really work that well before if you used the same name as an existing block. (Note if you actually try this, you'll have to fix the color thing and then probably runnpm run build:compressed
andnpm run checkin:built
). Because we follow all the same steps above, but instead offactory_base
not existing, you've overwritten the definition offactory_base
so things get weird when we try to serialize that block into the main workspace. This is despite the comments inupdatePreview
that try to claim things will be fine if a user picks an existing name.That's why I just decided to not let them use existing names. Is it the best experience? no. but it's good enough and it's better than it used to be.
This probably isn't the ideal solution but the existing code is a bit of a mess. The problem comes from there not being a clear source of truth for the block definition. Is it the main workspace, or the text area? it depends on if you are in manual mode or not, but it isn't handled cleanly in both cases. Ideally we'd know which one is the source of truth and be able to update the other one cleanly, but I didn't really feel like re-architecting all of block factory, so I slapped this bandaid on instead.
Test Coverage
Documentation
Additional Information
Me: sweet #6175 should take me 30 seconds to fix
Me 4 hours later: T_T