fix: Enable blocks if user can't manually enable them. #8354
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
| @@ -25,7 +25,7 @@ import {defineRowBlock} from '../test_helpers/block_definitions.js'; | |||
| suite('Procedures', function () { | |||
| setup(function () { | |||
| sharedTestSetup.call(this, {fireEventsNow: false}); | |||
| this.workspace = Blockly.inject('blocklyDiv', {}); | |||
| this.workspace = Blockly.inject('blocklyDiv', {disable: true}); | |||
There was a problem hiding this comment.
Which test did this change target? Since it's in setup for a suite, I think it will impact multiple tests.
There was a problem hiding this comment.
I've updated the test to only modify options in the one affected test method.
johnnesky
force-pushed
the
nesky_manually_disabled
branch
from
c221121 to
ca23567
Compare
johnnesky
force-pushed
the
nesky_manually_disabled
branch
from
24396b5 to
bd90a48
Compare
rachel-fenichel
approved these changes
Jul 19, 2024
Arun-cn
pushed a commit
to Arun-cn/blockly
that referenced
this pull request
Jul 24, 2024
* fix: Enable blocks if user can't manually enable them. * Only change the affected test method.
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.
The basics
The details
Resolves
Fixes #8310
Proposed Changes
When any disabled reason is removed, and the workspace is configured to not allow manually disabling/enabling, and a block is currently disabled for the manual reason, remove the manual reason in addition to whatever reason initiated the change.
Reason for Changes
A legacy serialized workspace does not specify the reason why blocks are disabled. When loaded with blocks that had been disabled due to being orphaned but without a recorded reason, Blockly defaults to using the manually disabled reason. However, some workspaces are configured to disallow disabling/enabling blocks, in which case the user can't remove the manually disabled reason.
For the specific case where a workspace does not allow manually disabling or enabling blocks, and a block currently has the manually disabled reason, and some other disabled reason is being removed (e.g. the block is no longer orphaned), then removing the manually disabled reason will allow the block to be fully enabled again.
Test Coverage
I haven't created any unit tests for this legacy workaround yet, but I did have to explicitly set the option
{disable: true}in a test workspace to get one test method to pass since it was designed for the common case where users are allowed to manually disable blocks.I manually tested that the workaround behaves as intended by using the browser console to set
Blockly.getMainWorkspace().options.disable = falseafter manually disabling a block.Documentation
N/A
Additional Information
N/A