fix: Make workspace-multiselect plugin workable with Blockly v12 by HollowMan6 · Pull Request #9261 · RaspberryPiFoundation/blockly

HollowMan6 · 2025-07-29T13:41:11Z

The basics

I validated my changes

The details

Resolves

Fixes mit-cml/workspace-multiselect#103

Proposed Changes

Workspace-multiselect plugin is in the process of updating to Blockly v12:

[WIP] Support Blockly v12 mit-cml/workspace-multiselect#106

However, some changes are necessary to get Blockly working properly with the plugin. The fundamental issue here is that Blockly v12 is buggy at this stage to properly support a custom and dummy IDraggable (MultiselectDraggable for the plugin's case), as Blockly constantly wants to change the focus to the actual block via hardcoded getFocusManager().focusNode calls that are scattered all around the codebase. Setting MultiselectDraggable directly via Blockly.common.setSelected doesn't work as well, and we will need to call Blockly.getFocusManager().updateFocusedNode directly.

I'm sure the changes in this PR now will break some native Blockly features here and there. Feel free to modify on top of this PR directly, and I'll help test out to see if it still works with the plugin.

Reason for Changes

Necessary changes to make workspace-multiselect plugin workable with Blockly v12

Test Coverage

N/A

Documentation

N/A

Additional Information

https://groups.google.com/g/blockly/c/7RSn9cXAulA

Signed-off-by: Hollow Man <hollowman@opensuse.org>

cpcallen · 2025-07-29T18:37:12Z

core/block_svg.ts

+    // Make sure the element is of Node type
+    if (!(element instanceof Node)) {
+      element = null;
+    }


AFAICT this check does nothing (and I'm surprised that TSC does not complain), because IFocusableNode's getFocusableElement method is typed as returning HTMLElement | SVGElement and both HTMLElement and SVGElement inherit from Node, so the only time the body of the if statement will be executed is if element is already null.

If some focusable node's .getFocusableElement() returns something which is not an instanceof Node then it has violated the interface definition—unless I have greatly misunderstood something about how the DOM works, at any rate.

If some focusable node's .getFocusableElement() returns something which is not an instanceof Node then it has violated the interface definition

For now the MultiselectDraggable is dummy and doesn't have an actual corresponding DOM object, that's why this check is added here, not sure if there's a better way for handling this one.

mit-cml/workspace-multiselect@1c9b6d5#diff-a6ef3f5605a5577d348743aa99d160ec6523f07f18c754f34e638513878515a5R47-R49

cpcallen · 2025-07-29T18:43:04Z

core/block_svg.ts

+    // Make sure the element is of Node type
+    if (!(element instanceof Node)) {
+      element = null;
+    }


Ditto here.

cpcallen · 2025-07-29T18:44:41Z

core/focus_manager.ts

+    // const prevFocusedElement = this.focusedNode?.getFocusableElement();
+    // const hasDesyncedState = prevFocusedElement !== document.activeElement;
+    const hasDesyncedState = false;


The multiselect plugin will definitely need to be able to operate without causing focus desynchronisation.

As explained:

Currently, the workspace-multiselect plugin acts as an adapter. It maintains its own multiple selection set (state), which keeps track of currently selected blocks. At the Blockly side, this is implemented as if there's a global MultiselectDraggable (which implements IDraggable) for each workspace, and current design for the plugin is that this instance of MultiselectDraggable should always be selected (in a dummy way as it won't create any new DOM object) when we have multiple blocks selected, so that the plugin can receive corresponding actions and pass all the actions to the blocks in the multiple selection set (state). It was working well in previous versions, but now in v12, the current code for getFocusManager().focusNode seems to break this completely, as it keeps unselecting MultiselectDraggable.

This change is an attempt to address the MultiselectDraggable unselecting issue, as we don't want the actual focus to get synchronised when in multiselect mode, although I'm aware that this will cause issues with other Blockly features.

cpcallen · 2025-07-29T18:49:15Z

core/block_svg.ts

+    if (!(element instanceof Node)) {
+      element = null;
+    }
+    const restoreFocus = this.getSvgRoot().contains(element);


It's possible that the existing check, reproduced here, is overly-broad: I note that restoreFocus will get set to true even if this merely contains focusedNode (rather than is focusedNode).

I also wonder whether the focus manipulation in appendChild, which this section of code is working around, could itself be either eliminated or made more adept, such that this workaround would not be needed at all.

For now, the MultiselectDraggable is dummy and doesn't have an actual corresponding DOM object. If this is properly addressed, I would assume this change will already be eliminated.

cpcallen · 2025-07-29T18:50:52Z

core/layer_manager.ts

+    //   // Since moving the element to the drag layer will cause it to lose focus,
+    //   // ensure it regains focus (to ensure proper highlights & sent events).
+    //   getFocusManager().focusNode(elem);


It's not obvious to me why moving an element to the drag layer will cause it to lose focus. Maybe that is the problem that should be fixed, so this section of code is not needed.

It's because that we should keep MultiselectDraggable selected when we have multiple blocks selected, and when we do the dragging, we drag multiple blocks one by one by calling the method here. This code will make the single exact block that is in the dragging process get selected, instead of keeping MultiselectDraggable selected.

cpcallen · 2025-07-29T18:51:01Z

core/layer_manager.ts

+    //   // Since moving the element off the drag layer will cause it to lose focus,
+    //   // ensure it regains focus (to ensure proper highlights & sent events).
+    //   getFocusManager().focusNode(elem);
+    // }


Ditto here.

cpcallen · 2025-07-29T18:55:26Z

core/utils/dom.ts

-  if (classNames.every((name) => element.classList.contains(name))) {
+  if (
+    classNames.every(
+      (name) => !!element.classList && element.classList.contains(name),


MDN tells me that Element's .classList will always be a DOMTokenList, even if the element has no class attribute, so I cannot see why the nullish check might be needed here.

Similar reason as for now, the MultiselectDraggable is dummy and doesn't have an actual corresponding DOM object.

cpcallen · 2025-07-29T18:55:57Z

core/utils/dom.ts

 */
 export function removeClasses(element: Element, classNames: string) {
-  element.classList.remove(...classNames.split(' '));
+  if (element.classList) {


Ditto here.

cpcallen · 2025-07-29T18:56:12Z

core/utils/dom.ts

-  if (classNames.every((name) => !element.classList.contains(name))) {
+  if (
+    classNames.every(
+      (name) => !element.classList || !element.classList.contains(name),


Ditto here.

DaryeDev · 2026-01-21T20:22:55Z

:(

fix: Make workspace-multiselect plugin workable with Blockly v12

efdb098

Signed-off-by: Hollow Man <hollowman@opensuse.org>

HollowMan6 requested a review from a team as a code owner July 29, 2025 13:41

HollowMan6 requested a review from rachel-fenichel July 29, 2025 13:41

github-actions bot assigned rachel-fenichel Jul 29, 2025

github-actions bot added the PR: fix Fixes a bug label Jul 29, 2025

HollowMan6 mentioned this pull request Jul 29, 2025

[WIP] Support Blockly v12 mit-cml/workspace-multiselect#106

Draft

cpcallen reviewed Jul 29, 2025

View reviewed changes

cpcallen requested a review from BenHenning July 29, 2025 18:57

github-actions bot assigned BenHenning Jul 29, 2025

HollowMan6 marked this pull request as draft July 29, 2025 22:27

florian-h05 mentioned this pull request Nov 5, 2025

Upgrade to Blockly v11 and respective plugin versions openhab/openhab-webui#3398

Merged

HollowMan6 closed this by deleting the head repository Jan 21, 2026

Conversation

HollowMan6 commented Jul 29, 2025

The basics

The details

Resolves

Proposed Changes

Reason for Changes

Test Coverage

Documentation

Additional Information

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HollowMan6 Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HollowMan6 Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DaryeDev commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

HollowMan6 Jul 29, 2025 •

edited

Loading

HollowMan6 Jul 29, 2025 •

edited

Loading