Skip to content

fix a couple of block rendering bugs#10602

Merged
riknoll merged 4 commits intomasterfrom
dev/riknoll/rendering-bugs
May 27, 2025
Merged

fix a couple of block rendering bugs#10602
riknoll merged 4 commits intomasterfrom
dev/riknoll/rendering-bugs

Conversation

@riknoll
Copy link
Member

@riknoll riknoll commented May 27, 2025

fixes microsoft/pxt-microbit#6147
fixes microsoft/pxt-microbit#6242

fixes two unrelated rendering bugs for blocks

the first bug was because we weren't listening to the path changes correctly inside of PathObject. I corrected that by overriding setPath and removed our old hack for fixing highlights manually in blocks.tsx

the second bug was caused by blockly deprecating the setEnabled function on blocks. blocks can now be disabled for multiple reasons, and we weren't clearing all of those reasons when we updated the disabled blocks on the workspace. in particular, the disabling caused by Blockly.Events.disableOrphans() (which we call in blocks.tsx) was sometimes sticking around. i fixed this and removed all calls to the deprecated function

@riknoll riknoll requested a review from a team May 27, 2025 17:55
block.setDisabledReason(false, Blockly.constants.MANUALLY_DISABLED);

// this is the reason for blocks that are disabled via Blockly.Events.disableOrphans
block.setDisabledReason(false, "ORPHANED_BLOCK");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it that we can have all of these calls without conditions?

Copy link
Member Author

@riknoll riknoll May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a no-op if the block isn't disabled


}

const DIFF_DISABLED_REASON = "disabled_for_diff";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to have these consts somewhere consistent? Like if we were to add more reasons why the blocks are disabled, would it be better to have it in one place rather than in the different files or are these the only scenarios blocks can be disabled?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this workspace is only being used to generate the diff SVG for the github integration. it gets thrown away after the diff is rendered. only reason we have this reason here is because we need some string to pass to setDisabledReason (but it could be any random string)

@riknoll riknoll enabled auto-merge (squash) May 27, 2025 20:15
@riknoll riknoll merged commit 14616c7 into master May 27, 2025
20 checks passed
@riknoll riknoll deleted the dev/riknoll/rendering-bugs branch May 27, 2025 20:31
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.

Connected block is displayed as disconnected Debugger highlight is off for error blocks

2 participants