Upgrade `multiselect` plugin to use an IDragger (for Blockly 11)

[HollowMan6]

Currently the multiselect plugin uses a custom block dragger to handle dragging multiple blocks at once. This is bad because it doesn’t allow you to select workspace comments as part of a multiselection. In Blockly 11, the upstream will make some changes to how the dragging in Blockly works. This would require some breaking changes, which would require fixes to the multi-select plugin. But after it's implemented we shouldn't have to monkey patch anymore! Which should make the plugin easier to maintain in the future.

Implementation

Custom Draggable

The multiselect plugin can implement a custom IDraggable that wraps all of the selected elements. It can delegate to drag operations to the individual elements.

class MultiselectDraggable implements IDraggable {
  private subDraggables: IDraggable[];

  addSubDraggable() { /* Implementation */ }

  startDrag(e: PointerEvent) {
    for (const draggable of this.subDraggables) {
      draggable.startDrag(e);
    }
  }

  drag(newLoc: Coordinate, target: IDragTarget, e?: PointerEvent) {
    for (const draggable of this.subDraggables) {
      draggable.drag(Blockly.Coordinate.sum(newLoc, draggable.dragOffset), target, e);
    }
  }

  endDrag(e: PointerEvent) {
    for (const draggable of this.subDraggables) {
      draggable.endDrag(e);
    }
  }
}

This implementation also means that the multiselect plugin won’t have to do any special work to be compatible with the scroll options plugin, since the scroll options will override the IDragger while this creates a custom IDraggable.

Selection

Currently, the multiselect plugin handles selection by passing a random one of the selected blocks to Blockly.common.setSelected. Instead, it will need to pass the MultiselectDraggable to setSelected so the Gesture can properly pass it to the IDragger.

Related issue: https://github.com/google/blockly-samples/issues/2317

[HollowMan6]

The beta version of v11 (v11.0.0-beta.8) has included the IDragger feature. Some docs to help with migration:

• Draggables: https://developers.google.com/blockly/guides/configure/web/dragging/draggable This is the kind of object to implement to represent a multiselection.
• Draggers: https://developers.google.com/blockly/guides/configure/web/dragging/dragger These handle calculating the workspace location where draggables should move to. We shouldn't need to implement one, but it might be useful to read this for context.
• Copy paste: https://developers.google.com/blockly/guides/configure/web/copy-paste Since we'll be using a custom draggable, we'll need to handle copy-paste ourselves. We should be able to delegate to the pasters, so it shouldn't be too much lift.

[changminbark]

Hi everyone,

My name is Chang Min Bark and I will be the GSoC contributor in charge of upgrading the multiselect plugin. I am also in charge of cross-testing the mutliselect plugin with other Blockly plugins. I am looking forward to contributing to this community!

Thanks, Chang Min

[changminbark]

Hello,

I made a few changes and wanted to test them out but when I run "npm start" it does not show anything. I do not even see the error messages. I ran the script without my changes and it also showed a blank white page (so I don't think it is a result of my changes). Is there something I can do to see the error messages?

Thanks, Chang Min

[HollowMan6]

Copied from our last email, the bold and italic part in the first entry answers your question. Also, check the screenshots, you need to use IDraggable to replace that so we can fix and get the plugin working on Blockly v11.

• Now the only difference between the blockly-v11 and the main is this commit https://github.com/mit-cml/workspace-multiselect/commit/11b76b8e651538f55c17e2743e6641ea14ee9f51, which is just a version change done by me to get you started. I haven’t done anything related to actual code for IDraggable, so that branch is actually still broken and you will see a blank page with errors in the developer tools console (F12) once you do npm start. It will be your job to implement IDraggable so that we can fix the Blockly v11 support. You can start your implementation / fix guided by those errors as well as the documentation I mentioned in my first email/issue on GitHub https://github.com/mit-cml/workspace-multiselect/issues/39.
• I don’t expect “@blockly/dev-tools” and any other possible conflict dependencies to explicitly indicate support for Blockly v11 now, as v11 is still in beta. But it looks fine to forcibly override that at this stage. Feel free to upgrade the version at any time later once you find they have the support.
• You are allowed to do whatever you want at this stage in the codebase without the need to ask for permission (be bold and creative), as long as it doesn’t cause any regression. Once you submit the PR, I will review your code and let you know the best practice/alternative way if I find any problems, so relax and explore!
• As another first step during the community bonding period, I would encourage you to try out the main branch as well to learn how things are supposed to work (since the blockly-v11 branch is broken, you won’t learn anything there). It will benefit you a lot after you finish the IDraggable implementation to make sure no regression/bug is introduced, as well as for your second cross-testing task. A possible checklist at https://github.com/mit-cml/workspace-multiselect?tab=readme-ov-file#user-behavior or you might want to experience the features I mentioned in my talk last year https://github.com/HollowMan6/My-Google-Summer-of-Code/blob/main/Slide_Songlin_Multi-Select_Plugin_2023_Blockly_Summit.pptx

[changminbark]

For the IDragger component, we would just need to import one without reimplementing it as the default one should work with the MultiselectDraggable that implements the IDraggable interface. Am I correct?

[HollowMan6]

Ah yes, sorry for the confusion I caused in my previous emails. I should call them IDraggable. Just correct them in my last comment.

[changminbark]

Hello,

I just had a question regarding the implementation of the IDraggable interface. Since the previous code was written in JavaScript and implementing an interface is a feature of TypeScript, should I write the code for the IDraggable implementation of the multi-select draggable in TypeScript? If so, do I have to install and configure the Webpack bundler for TypeScript?

[HollowMan6]

Good question! Yes, and if you feel like doing so, you can help convert the whole plugin into Typescript. I don't think you need to configure the Webpack bundler for TypeScript as the bundler is configured by @blockly/dev-scripts and it should already have the ability to handle Typescript, you can check one of the Typescript based plugin for reference https://github.com/google/blockly-samples/tree/master/plugins/block-dynamic-connection

[changminbark]

The main reason I asked is because when I tried importing the new class I created that implemented the IDraggable interface, I was encountering an error related to the code being in Typescript.

[HollowMan6]

I haven't investigated how to mix Javascript with Typescript in the Blockly plugin, you can check block-dynamic-connection to see how to use Typescript in the plugin. We can discuss it today later during the meeting if you can't get it fixed.

My guess is that you may need a tsconfig.json file https://github.com/google/blockly-samples/blob/master/plugins/block-dynamic-connection/tsconfig.json

[changminbark]

After looking at the main branch (original one that you worked on), I am thinking of creating a typescript declared class called MultiDraggable that implements the IDraggable interface. Then, I was planning to create the MultiselectDraggable class that extends the MultiDraggable declared class that I created. The reason for this is that I cannot directly implement the IDraggable interface (typescript) in the MultiselectDraggable class (javascript). What do you think about this approach?

[HollowMan6]

Sounds good to me as long as it works. It's fine to use your approach if you find something that is not doable in Typescript, but I guess it's better to directly have MultiselectDraggable class implements the IDraggable interface in Typescript (so that we can skip MultiDraggable).

[changminbark]

I guess that if I want to skip the MultiDraggable, I would have to use some sort of tsc to compile the typescript into javascript. However, if I want to avoid that, I would have to go with my method. I will decide whether it would be better to use tsc or just use javascript as I start to develop this more.

I also have another question about the skeleton code that you provided in the original implementation description. In the "drag" method, you have a target parameter. However, the IDraggable interface does not have this, so the IDE is giving me an incompatible override message. What was your intention with this parameter? Was it to check whether the blocks being dragged can be connected to an existing block in the workspace?

[HollowMan6]

That code snippet was taken from the Blockly team's original design document, maybe later they found that unnecessary and removed that parameter in the beta version. Anyway, you can safely ignore that target parameter as now they don't have that.

[mark-friedman]

I cannot directly implement the IDraggable interface (typescript) in the MultiselectDraggable class (javascript).

AFAIK, you should be able to just create a JavaScript class which satisfies the TypeScript interface and avoid using any TypeScript. See, for example, this section of the Blockly docs, which Songlin previously mentioned. What issues were you seeing?

[mark-friedman]

The key, I think, is that you want your class to extend the Blockly interface.

[changminbark]

@mark-friedman @HollowMan6 I have tried extending the Blockly interface but it does not allow me to extend an interface. That's why I thought that I needed to implement the interface to a MultiDraggable class (TypeScript) and then extend from that class (JavaScript).

I also don't know why I cannot directly extend the IDraggable interface in the JavaScript file for MultiselectDraggable.

I guess one way to "extend" the IDraggable interface would be to manually create the methods/properties in the interface.

[HollowMan6]

No worries, I think for the first step, we should get this working. Later we can consider refactoring and I will also see if there’s any way to avoid Typescript. At this stage I’m also not sure if there’s a better way.

[HollowMan6]

Or, you might want to try upgrading Blockly version https://www.npmjs.com/package/blockly/v/11.0.0-beta.12 Not sure if this can help improve things or not.

[HollowMan6]

Okay, looks like we still have the same error in 11.0.0-beta.12 as @changminbark mentioned.

[ewpatton]

You cannot extend a TypeScript interface in JavaScript because it doesn't exist. It is akin to type erasure in Java. The definition is used for type checking in TypeScript and then it is no longer needed at the JavaScript layer since JavaScript doesn't have the concept of an interface. It should be sufficient to write the JavaScript code to conform to the IDragger interface until we migrate the project over to typescript. Sent from my iPhoneOn May 15, 2024, at 19:29, Chang Min Bark @.***> wrote: I have tried extending the Blockly interface but it does not allow me to extend an interface. That's why I thought that I needed to implement the interface to a MultiDraggable class (TypeScript) and then extend from that class (JavaScript). I also don't know why I cannot directly extend the IDraggable interface in the JavaScript file for MultiselectDraggable. image.png (view on web)

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

[changminbark]

Does that mean if I just create the class conforming to the IDraggable interface, there will be a default Dragger class that will handle dragging the created IDraggable class? I was looking through the Blockly-v11 files and found a Dragger class that implemented the IDragger interface, but it only had skeleton methods. Do I also have to create a class that implements the IDragger interface?

[mark-friedman]

I was looking through the Blockly-v11 files and found a Dragger class that implemented the IDragger interface, but it only had skeleton methods. Do I also have to create a class that implements the IDragger interface?

Hm. I'm looking at the rc/v11.0.0 branch of Blockly and the Dragger class appears to be fully implemented (see here).

[HollowMan6]

Does that mean if I just create the class conforming to the IDraggable interface, there will be a default Dragger class that will handle dragging the created IDraggable class?

Yes.

Do I also have to create a class that implements the IDragger interface?

No.

@changminbark Theoretically you don't need to touch anything related to the Dragger class, check this issue description and https://github.com/mit-cml/workspace-multiselect/issues/39#issuecomment-2058325621 (taken from Blockly team's design documentation):

This implementation also means that the multiselect plugin won’t have to do any special work to be compatible with the scroll options plugin, since the scroll options will override the IDragger while this creates a custom IDraggable.

Currently, the multiselect plugin handles selection by passing a random one of the selected blocks to Blockly.common.setSelected. Instead, it will need to pass the MultiselectDraggable to setSelected so the Gesture can properly pass it to the IDragger.

• Draggables: https://developers.google.com/blockly/guides/configure/web/dragging/draggable This is the kind of object to implement to represent a multiselection.
• Draggers: https://developers.google.com/blockly/guides/configure/web/dragging/dragger These handle calculating the workspace location where draggables should move to. We shouldn't need to implement one, but it might be useful to read this for context.
• Copy paste: https://developers.google.com/blockly/guides/configure/web/copy-paste Since we'll be using a custom draggable, we'll need to handle copy-paste ourselves. We should be able to delegate to the pasters, so it shouldn't be too much lift.

If you get into a situation such that we have to implement the IDragger interface to fix some bugs, then we should contact the Blockly team since this is not something they intend to happen and will cause conflict with the scroll-options plugin.

[changminbark]

Okay, I was just a little concerned when looking into the IDragger interface and the Dragger class that implemented the interface. I will just assume that a Dragger class should handle it and continue to develop the MultiselectDraggable class.

[changminbark]

Is there an IRenderedElement interface? I tried opening the page for the IRenderedElement interface on https://developers.google.com/blockly/guides/configure/web/dragging/draggable, but it leads me to a 404 page. I just wanted to let the Blockly team know this so they can fix the documentation page.

My question regarding implementing the IRenderedElement interface, which can be found here: https://github.com/google/blockly/blob/rc/v11.0.0/core/interfaces/i_rendered_element.ts, is what it means by SVG root.

Does this refer to the SVG of the workspace or the SVG of layer the MultiselectDraggable will be in while it is being dragged? Or is it completely different? I remember Songlin said something about this, but I do not remember clearly.

[mark-friedman]

... what it means by SVG root

From the description on the doc page that you quote from and the looking at the source of a few of the Blockly existing classes that implement the IDraggable interface (e.g. block_svg.ts and bubble.ts) it seems reasonably clear that getSvgRoot() needs to return the SVG of the thing that is getting dragged. In this case the SVG of the multi-selected blocks.

[HollowMan6]

@changminbark Yes, Mark is right. I have already followed up on the related issue with Beka. Let's see how it goes.

[changminbark]

Okay, thank you!

[mark-friedman]

Yes, Mark is right. I have already followed up on the related issue with Beka. Let's see how it goes.

Thanks for the email to Beka, @HollowMan6. It wasn't clear to me either how to create or obtain that SVG.

[HollowMan6]

@changminbark So for MultiselectDraggable, we don't need to implement the IRenderedElement interface, which means no need for having the getSvgRoot() method.

[changminbark]

Okay, thank you!

[HollowMan6]

Blockly v11 is officially released! https://github.com/google/blockly/releases/tag/blockly-v11.0.0 Now we should work on the release version instead of the beta one, and I have done an upgrade of the dev dependencies that actually support v11 and forced pushed the commit to the blockly-v11 branch: https://github.com/mit-cml/workspace-multiselect/blob/blockly-v11/package.json#L45-L48 @changminbark

[changminbark]

Should I just manually update the dependencies? Currently, my branch is 4 commits ahead of the remote branch and 1 commit behind it.

[changminbark]

I am also encountering an issue where I cannot seem to import the Coordinate class from Blockly after updating it. I am getting the following error message when I run the npm start script.

I have checked the package.json file in the Blockly directory under node_modules and the export statement seems to be correct.

Is there something wrong I am doing?

[changminbark]

I think I fixed it by using Blockly.utils.Coordinate. I guess that I cannot directly import Coordinate but rather have to use it indirectly.

[HollowMan6]

Should I just manually update the dependencies? Currently, my branch is 4 commits ahead of the remote branch and 1 commit behind it.

Sorry, I wasn't aware that you already have local commits. It can be a little complicated since I force-pushed. I guess you can drop these 2 commits related to package*.json changes to avoid conflict:

• https://github.com/changminbark/workspace-multiselect/commit/fb2c83534e5855943e8a6f9e4d54e0d827ec0971
• https://github.com/changminbark/workspace-multiselect/commit/11b76b8e651538f55c17e2743e6641ea14ee9f51

Then rebase on top of the upstream blockly-v11 branch.

I think I fixed it by using Blockly.utils.Coordinate. I guess that I cannot directly import Coordinate but rather have to use it indirectly.

Great to know you fixed this by yourself!

[ewpatton]

Blockly provides a migration tool that helps with situations where they reorganize classes. You may want to run that on the plugin code to see if there is anything else that needs to be corrected. Sent from my iPhoneOn May 22, 2024, at 01:50, Hollow Man @.***> wrote:

Should I just manually update the dependencies? Currently, my branch is 4 commits ahead of the remote branch and 1 commit behind it.

Sorry, I wasn't aware that you already have local commits. It can be a little complicated since I force-pushed. I guess you can drop these 2 commits to avoid conflict:

@. @.

Then rebase on top of the upstream blockly-v11 branch.

I think I fixed it by using Blockly.utils.Coordinate. I guess that I cannot directly import Coordinate but rather have to use it indirectly.

Great to know you fixed this by yourself!

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: @.***>

[changminbark]

Hello @ewpatton ,

Thank you for the reply. How do I access this migration tool for Blockly? What I did was just copy over the updated dev-dependencies that Songlin made in the package.json file into my package.json file.

[mark-friedman]

I think I fixed it by using Blockly.utils.Coordinate. I guess that I cannot directly import Coordinate but rather have to use it indirectly.

It's worth noting the following from the Blockly 11 release notes:

If you install Blockly through npm, we now provide an exports clause to explicitly declare what you can import from our package.

• Your build tooling may now throw an error if you deep import from our package such as import 'blockly/core/some/file/name.js', which has never been supported by Blockly but may have been made possible by your build tools.

[mark-friedman]

Hello @ewpatton ,

Thank you for the reply. How do I access this migration tool for Blockly? What I did was just copy over the updated dev-dependencies that Songlin made in the package.json file into my package.json file.

Its here, though I'm not sure that it has been updated (yet) for Blockly 11. It's worth trying, though.

[changminbark]

On another note, I think I found a bug in the Multiselect plugin (original). I multi-deselect a group of blocks and then click at empty space, which seems to re-select the previously-selected group of blocks. This is footage of me testing it out on the original version of the plugin:

https://github.com/mit-cml/workspace-multiselect/assets/88669927/e43982b9-8bb8-4fb0-93a3-2e754d7151b8

[changminbark]

I am also encountering an issue where I select a group of blocks and then disable multiselect mode and then encounter an error where it is trying to run updateBlocks on the MultiselectDraggable object, which does not have a pathObject property, resulting in an error. However, the weird thing is that the code that calls the updateBlocks on the Blockly.getSelected should only run under these conditions:

// When not in multiple selection mode and Blockly selects a block not // in currently selected set or unselects, clear the selected set.

However, I am not satisfying those conditions as I am merely disabling multiselect mode without unselecting the group of blocks.

https://github.com/mit-cml/workspace-multiselect/assets/88669927/e00adaab-4a68-44d1-9a3e-13352511b571

updateBlocks_ is called here:

[HollowMan6]

On another note, I think I found a bug in the Multiselect plugin (original). I multi-deselect a group of blocks and then click at empty space, which seems to re-select the previously-selected group of blocks. This is footage of me testing it out on the original version of the plugin:

That's interesting, I don't remember clearly if this was done intentionally or not, but I think this can be a feature/behavior to document XD (Revert back to the initial selection state of entering the multi-selection mode when you click the empty workspace).

I am also encountering an issue where I select a group of blocks and then disable multiselect mode and then encounter an error where it is trying to run updateBlocks_ on the MultiselectDraggable object, which does not have a pathObject property, resulting in an error.

You can safely remove all the pathObject updateSelected thing, that was initially intended to make the block look like it's "selected", now all the selection is handled by the Dragger, and we don't need that anymore.

However, the weird thing is that the code that calls the updateBlocks_ on the Blockly.getSelected should only run under these conditions: // When not in multiple selection mode and Blockly selects a block not // in currently selected set or unselects, clear the selected set. However, I am not satisfying those conditions as I am merely disabling multiselect mode without unselecting the group of blocks.

updateBlocks_ is also called by the DragSelect library element (un)selection event, so it might also be called when disabling multiselect mode without unselecting the group of blocks. I think this should be fine previously, let me know if this is blocking you from doing something.

https://github.com/mit-cml/workspace-multiselect/blob/af7eeb41ea2774c6e58c39e4a186c2f37592dd7c/src/multiselect_controls.js#L402-L417

[changminbark]

That's interesting, I don't remember clearly if this was done intentionally or not, but I think this can be a feature/behavior to document XD (Revert back to the initial selection state of entering the multi-selection mode when you click the empty workspace).

The weird thing about this is that it does not happen all the time. It only occurs in this scenario:

1. Turn on multiselect mode
2. Select a group of blocks
3. Turn off multiselect mode
4. Turn on multiselect mode
5. Unselect a group of blocks
6. Click on blank space to replicate weird behavior.

You can safely remove all the pathObject updateSelected thing, that was initially intended to make the block look like it's "selected", now all the selection is handled by the Dragger, and we don't need that anymore.

I tried commenting these portions of the code out and it leads to the blocks not being highlighted during the multi-selection (though they are highlighted after I let go of the mouseclick) and another error, which I think is because the multiselectDraggable class I made does not have a bringToFront() method:

[changminbark]

So I traced the error to the disableMultiselect() method. When I change the value of the Blockly.common.setSelected() in the pic to one of the random blocks in the selection, the error does not appear.

This is probably because updateBlocks_(Blockly.getSelected()) is being called in the updateMultiselect() method right after. The thing that confuses me is why is updateMultiselect() being called when I am merely turning off the multiselect mode? Is it because multiselect makes it so that updateMultiselect() is called for any workspace event?

[HollowMan6]

About that bug, I guess we just miss some condition checks, feel free to fix that in your code

I tried commenting these portions of the code out and it leads to the blocks not being highlighted during the multi-selection (though they are highlighted after I let go of the mouseclick)

You might want to update the selection on the go during the multi-selction as well in this case.

and another error, which I think is because the multiselectDraggable class I made does not have a bringToFront() method:

Then feel free to do something, either comment out the code and handle the issue at a later stage to find a solution, or do type checks and only call that when it’s an actual block.

Is it because multiselect makes it so that updateMultiselect() is called for any workspace event?

Ah yes, I do have some fix for fixing bugs in multi-workspace situations. Feel free to check old issues and PRs, such as https://github.com/mit-cml/workspace-multiselect/pull/24/files

[changminbark]

@HollowMan6 @mark-friedman Hello, so I finished implementing the multiple-workspace helper functions. However, I'm stuck with the drag functions for the multiselectDraggable. I think I have an idea about why the drag functions are not being called. Basically, there is no clickable interface/SVG for the multiselectDraggable, as I explained in my email to Beka. What suggestions do you have to fix this? Or should I just wait for Beka to respond?

Thank you!

[HollowMan6]

Basically, there is no clickable interface/SVG for the multiselectDraggable, as I explained in my email to Beka. What suggestions do you have to fix this? Or should I just wait for Beka to respond?

We have already talked about this during our Saturday meeting, I'm not sure about the possible cause and I doubt if this is something really related to the SVG, since Beka has already mentioned that we don't need to implement the IRenderedElement interface in the previous email, but anyway it would be good to wait for Beka to respond.

@changminbark So for MultiselectDraggable, we don't need to implement the IRenderedElement interface, which means no need for having the getSvgRoot() method.

[changminbark]

I am uploading here the email thread between Songlin, Beka, and me:

This is for anyone who wants to learn more about the problem and what was proposed for the solution.