Skip to content

Conversation

@microbit-grace
Copy link
Contributor

Fixes #401
Possible mitigation 1 in the issue

@rachel-fenichel Please feel free to not take this change and implement it differently. Not sure if there is a better way of implementing this.

@microbit-grace microbit-grace requested a review from a team as a code owner April 8, 2025 16:10
@microbit-grace microbit-grace requested review from maribethb and removed request for a team April 8, 2025 16:10
getNextNode(
node: ASTNode | null,
isValid: (p1: ASTNode | null) => boolean,
loop = true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please also add a loop param to getPrevNode

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI Grace won't be around until tomorrow so please feel free to take this and run with it.

We may need should pass loop to the recursive calls too.

@rachel-fenichel rachel-fenichel self-assigned this Apr 8, 2025
@microbit-matt-hillsdon
Copy link
Contributor

This has given us looping during constrained move up (e.g. of draw circle in draw in the default demo). But if I move down then it breaks:

b.getSourceBlock is not a function
TypeError: b.getSourceBlock is not a function
    at ConnectionChecker$$module$build$src$core$connection_checker.doSafetyChecks (webpack-internal:///./node_modules/blockly/blockly_compressed.js:1233:272)
    at ConnectionChecker$$module$build$src$core$connection_checker.canConnectWithReason (webpack-internal:///./node_modules/blockly/blockly_compressed.js:1230:658)
    at ConnectionChecker$$module$build$src$core$connection_checker.canConnect (webpack-internal:///./node_modules/blockly/blockly_compressed.js:1230:526)
    at eval (webpack-internal:///./src/keyboard_drag_strategy.ts:88:39)
    at Array.forEach (<anonymous>)
    at KeyboardDragStrategy.getConstrainedConnectionCandidate (webpack-internal:///./src/keyboard_drag_strategy.ts:86:24)
    at KeyboardDragStrategy.getConnectionCandidate (webpack-internal:///./src/keyboard_drag_strategy.ts:111:25)
    at KeyboardDragStrategy.updateConnectionPreview (webpack-internal:///./node_modules/blockly/blockly_compressed.js:914:495)
    at KeyboardDragStrategy.drag (webpack-internal:///./node_modules/blockly/blockly_compressed.js:914:307)
    at KeyboardDragStrategy.drag (webpack-internal:///./src/keyboard_drag_strategy.ts:36:15)

@microbit-matt-hillsdon
Copy link
Contributor

This code has now moved into core so I think someone else will need to take #401 and implement looping in core's version. Suggest closing.

@rachel-fenichel
Copy link
Collaborator

Agreed. I'm implementing it in core.

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.

shortcuts to jump between stacks of blocks

3 participants