Blockly v11

[changminbark]

The basics

• [x] I validated my changes

The details

Resolves

Fixes https://github.com/mit-cml/workspace-multiselect/issues/39 and part of https://github.com/mit-cml/workspace-multiselect/issues/50 (backpack and scroll options plugins fixed and verified)

Proposed Changes

Updates the multiselect plugin to be compatible with Blockly version 11.

Reason for Changes

To make it compatible with the latest Blockly version and avoid the need to monkey patch.

Test Coverage

There are several test files in the test subdirectory.

Documentation

I have created javascript docs as well as comments for any new code.

Additional Information

[HollowMan6]

I'll continue reviewing your actual code once the above ones have been addressed.

[changminbark]

I think the merge conflict is just a matter of indentation.

[HollowMan6]

I think the merge conflict is just a matter of indentation.

Anyway you need to fix this as it stops us from merging.

[HollowMan6]

I just merged the upstream main into your PR to facilitate my reviewing, remember to pull the merge commit to your local git repo @changminbark

[HollowMan6]

Looks like we will eventually end up with page crashed when I play around with multi-selection for a while at my side, @changminbark did you have a similar issue at your side:

[changminbark]

@HollowMan6 I didn't get any pages that crash. How long did you use it for and do you know what was the last action you did before it crashed?

[HollowMan6]

@HollowMan6 I didn't get any pages that crash. How long did you use it for and do you know what was the last action you did before it crashed?

It seems to be pretty random, so I can't tell exactly, but not long, maybe 30 seconds to 1 minute with quick actions for testing. Also, this seems to be only reproducible in Chrome, not Firefox. While in Firefox, I can see this error here as frequently as the time for Chrome crashes:

Also, I have caught another error here:

[HollowMan6]

Looks like after dragging, sometimes I may also get a block connected like this disassembled:

[HollowMan6]

Let's first get the above issues addressed, once these are done, I'll take a close look at the logic of the code.

[changminbark]

@HollowMan6 I didn't get any pages that crash. How long did you use it for and do you know what was the last action you did before it crashed?

It seems to be pretty random, so I can't tell exactly, but not long, maybe 30 seconds to 1 minute with quick actions for testing. Also, this seems to be only reproducible in Chrome, not Firefox. While in Firefox, I can see this error here as frequently as the time for Chrome crashes:

Also, I have caught another error here:

What do you think is causing the issue for chrome? I am not sure because there is no proper error message that gives me a hint as to what is causing it.

[HollowMan6]

What do you think is causing the issue for chrome? I am not sure because there is no proper error message that gives me a hint as to what is causing it.

I'm not quite sure either but I guess it can be related to the logic of the code, maybe somewhere we have a bug that causes the memory usage to explode, and the thread for rendering the page was killed? If I find something, I'll let you know, you can focus on the other issues first.

[changminbark]

I have made the requested fixes.

As for the firefox error, I am not encountering it. Maybe with some of the changes I made, the issue is solved.

As for the chrome web page crashing, I am also not encountering it; I'm guessing it may had something to do with the getAllBlocks instead of the getTopBlocks (which I changed).

As for the third error that you showed related to reading the properties of 'x', I am not encountering it. I am not sure how to replicate this error, but for now I am not coming across this issue.

Looks like after dragging, sometimes I may also get a block connected like this disassembled:

As for this issue in the picture, how are you encountering this bug? I am also not coming across this. Do you know what steps replicate this behavior?

[HollowMan6]

As for the firefox error, I am not encountering it. Maybe with some of the changes I made, the issue is solved.

As for the third error that you showed related to reading the properties of 'x', I am not encountering it. I am not sure how to replicate this error, but for now I am not coming across this issue.

Clearly, these bugs are related to null/types checks since they indicate that something is undefined. For these bugs usually having a strong condition check is enough and you don't need to take time to replicate, but of course, if you like/want to find out the root cause, you can try to reproduce as well (didn't take the notes for reproducing these, but usually you can try with random blocks and do some random multi-selected actions).

As for the chrome web page crashing, I am also not encountering it; I'm guessing it may had something to do with the getAllBlocks instead of the getTopBlocks (which I changed).

I don't think so for this case, I was only having less than 10 blocks on my page. If you can't reproduce, maybe it's just because something is wrong with the Chrome on my computer, but let's see.

Looks like after dragging, sometimes I may also get a block connected like this disassembled:

As for this issue in the picture, how are you encountering this bug? I am also not coming across this. Do you know what steps replicate this behavior?

I don't have an exact description for reproducing this either, it's kind of hard to reproduce, but I guess it can be related to a possible edge case where you failed to find out the top-most block and drag all of them together.

[HollowMan6]

Note that it will also be necessary for you to be able to spot those user behavior inconsistencies if you want to do well in task 2, and I'm sure there should still be other bugs, so that's why I want to pause for a while and hand the remaining bug-hunting task yo you. I will convert this into the draft for now, once you have implemented all the fixes and believe you can no longer find any bugs, you can mark this PR as ready again and I'll continue my hunting.

[HollowMan6]

Note that in https://github.com/mit-cml/workspace-multiselect/commit/396f88ba1a97880624a1bc08e4965181d8a2fb86 We are having a new use case for justUnselectedBlock_ so that shouldn't be removed here. You can first merge the main branch changes into your PR and add that back. You will also need to accommodate that commit for blockly-v11

[changminbark]

As for the firefox error, I am not encountering it. Maybe with some of the changes I made, the issue is solved.

As for the third error that you showed related to reading the properties of 'x', I am not encountering it. I am not sure how to replicate this error, but for now I am not coming across this issue.

Clearly, these bugs are related to null/types checks since they indicate that something is undefined. For these bugs usually having a strong condition check is enough and you don't need to take time to replicate, but of course, if you like/want to find out the root cause, you can try to reproduce as well (didn't take the notes for reproducing these, but usually you can try with random blocks and do some random multi-selected actions).

As for the chrome web page crashing, I am also not encountering it; I'm guessing it may had something to do with the getAllBlocks instead of the getTopBlocks (which I changed).

I don't think so for this case, I was only having less than 10 blocks on my page. If you can't reproduce, maybe it's just because something is wrong with the Chrome on my computer, but let's see.

Looks like after dragging, sometimes I may also get a block connected like this disassembled:

As for this issue in the picture, how are you encountering this bug? I am also not coming across this. Do you know what steps replicate this behavior?

I don't have an exact description for reproducing this either, it's kind of hard to reproduce, but I guess it can be related to a possible edge case where you failed to find out the top-most block and drag all of them together.

Were you still encountering these issues regularly? I think I will add the type checking for the first two errors. I am not sure what to do with the web page crashing error and the last error with the edge case (if it is not reproducible)

[HollowMan6]

Were you still encountering these issues regularly? I think I will add the type checking for the first two errors. I am not sure what to do with the web page crashing error and the last error with the edge case (if it is not reproducible)

If you can’t you can move on to the other change requests first, I’ll check again once you mark this PR as ready again (spot bugs as much as possible and tried your best to implement fixes).

[changminbark]

Note that in 396f88b We are having a new use case for justUnselectedBlock_ so that shouldn't be removed here. You can first merge the main branch changes into your PR and add that back. You will also need to accommodate that commit for blockly-v11

How do I merge the main branch changes into my PR? I tried running git merge origin main and it seems to be not working.

[HollowMan6]

How do I merge the main branch changes into my PR? I tried running git merge origin main and it seems to be not working.

What do you mean? Do you have conflicts with main or something else?

[changminbark]

How do I merge the main branch changes into my PR? I tried running git merge origin main and it seems to be not working.

What do you mean? Do you have conflicts with main or something else?

It just says that it is up to date

[HollowMan6]

It just says that it is up to date

What does your origin point to? Your fork or upstream? If it's the former, I recommend you first go to your GitHub fork page, sync fork for the main branch before you try again.

[HollowMan6]

BTW, I remember you resolved those conflicts successfully in the past, why does it not work now? (suppose you are always using the same command for doing that)

[changminbark]

I am not sure. I am thinking of typing these commands, but I am not sure:

[HollowMan6]

Yes, I fully and strongly agree with what your screenshots said (use rebase), and I would also recommend you to squash all the commits into 1 in this PR, we already have 28 commits here in this PR and it's way too many. You can do something like (git rebase -i HEAD~28, and edit in the popped-up editor, which is "interactive rebase" https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History#:~:text=file%E2%80%9D%20commit%20completely.-,Squashing%20Commits,-It%E2%80%99s%20also%20possible), but them you will need to use force push (git push -f)

[changminbark]

I am not sure how to use the rebase command. I don't want to accidentally lose all of my progress so I will just wait until we get into a call before I do anything related to that.

[HollowMan6]

I am not sure how to use the rebase command. I don't want to accidentally lose all of my progress so I will just wait until we get into a call before I do anything related to that.

One trick is that you can always keep a record of your current commit id before you mess things up, and you can always use hard reset (e.g. git reset --hard 494939e) to get things back to work.

[changminbark]

I cannot resolve the merge conflict from my side. If I add that line, it leads to a lot of errors/bugs. What is the purpose of that line in main? Other than that, it seems that I cannot find any more bugs. I will wait for your reply before marking this as ready for review.

[HollowMan6]

I cannot resolve the merge conflict from my side. If I add that line, it leads to a lot of errors/bugs. What is the purpose of that line in main?

I'm not sure which line you are referring to, this one?

Then it's fine to remove this line. In https://github.com/mit-cml/workspace-multiselect/commit/396f88ba1a97880624a1bc08e4965181d8a2fb86 I just need to use justUnselectedBlock_ for revertLastUnselectedBlock() to fix #63, and removing this one shouldn't break it.

You should also make sure that the things I managed to fix recently (commits) don't get broken by your upgrades (e.g. #63, #58, #57)

Other than that, it seems that I cannot find any more bugs. I will wait for your reply before marking this as ready for review.

Did you manage to find any other bugs aside from my change requests? If not, I doubt I'm so lucky that when I pause, I have discovered all the bugs already. But anyway, I do hope that you have already taken this seriously, it would be a good chance to get yourself familiarized with the user behaviors of the multi-select plugin https://github.com/mit-cml/workspace-multiselect?tab=readme-ov-file#user-behavior, check the consistency with the old v10 version as well as with the single select one in Blockly, so that you can get well prepared to work on task 2 (as your tasks will be to find out those inconsistencies, and I won't be able to help you with spotting those things then)

It can also be a good idea to check the old closed issues and see if you break anything for these as well https://github.com/mit-cml/workspace-multiselect/issues?q=is%3Aissue+is%3Aclosed

Once you believe everything should be 100% fine on your side, you can mark this PR as ready.

[changminbark]

What should I do for the merge conflict then? Will you handle that on the main branch? I don't think I can resolve the conflict by deleting the line on the main branch.

I will continue to look for bugs and compare the behavior with the old version 10. The only bug that I can't seem to find is the web page crashing. My Chrome is not crashing, but yours did. I am not sure about that bug and how to resolve it. I don't have access to another computer so I cannot test this out on another computer. Do you mind trying to encounter this bug after I mark the PR as ready?

I have tested all of the user behavior and they seem to be working properly. I plan to continue testing/debugging for another day or two, so that if there are any bugs you could find, I still have some time to fix them.

[HollowMan6]

What should I do for the merge conflict then? Will you handle that on the main branch? I don't think I can resolve the conflict by deleting the line on the main branch.

Looks like you still need to practice your git skills. Merge conflict shouldn't be handled at the upstream and it should be handled at your branch at any time. I just helped you do that (remove that line) in https://github.com/mit-cml/workspace-multiselect/pull/62/commits/85db24ad16a45f476f762f0c48c5c662bab02fd5 Remember to pull from your fork branch (origin/blockly-v11)

I will continue to look for bugs and compare the behavior with the old version 10. The only bug that I can't seem to find is the web page crashing. My Chrome is not crashing, but yours did. I am not sure about that bug and how to resolve it. I don't have access to another computer so I cannot test this out on another computer. Do you mind trying to encounter this bug after I mark the PR as ready?

That one is fine in particular if you don't fix this.

I have tested all of the user behavior and they seem to be working properly. I plan to continue testing/debugging for another day or two, so that if there are any bugs you could find, I still have some time to fix them.

Good to hear!

[changminbark]

Can I also ask more about this issue:

https://github.com/mit-cml/workspace-multiselect/issues/58

How does the cascading look like? Currently, if I have 4 blocks selected (of the same type) and change the field, I get 3 subsequent calls to the setFieldValue, is that considered a cascade of calls or was Mark referring to something else?

[HollowMan6]

Can I also ask more about this issue:

58

How does the cascading look like? Currently, if I have 4 blocks selected (of the same type) and change the field, I get 3 subsequent calls to the setFieldValue, is that considered a cascade of calls or was Mark referring to something else?

No, then that works fine still (no cascading here)!

[changminbark]

I have tested this extensively and cross checked it with the old blockly-v10 version as well as the use cases. If there is anything I missed, please let me know! I will write them down to make sure I also check for those behaviors in part 2. Thank you!

[HollowMan6]

The main goal now is to get workspace comments supported. It can be a relatively large change so I'll continue reviewing after it's done (as it might cause more issues here). I'll mark this PR as draft again and please let me know once it's ready.

[changminbark]

Some errors I had while I was dragging (forgot to record so I can't tell you exactly how I have this, maybe this can be fixed together with the above cut issue):

Edit: Looks like it can be related to shadow blocks since after that one of the shadow block is gone!

I am guessing that this might have to do with something calling the connect on the shadow block. For now, I have created a type checking when I add to the connectionDBList. I will continue to test dragging and seeing if I encounter this issue. It would also be helpful if you can provide more details on how to replicate this (if you have the time).

As for the bug related to cutting, I was able to resolve it.

I forgot about being able to select the comments. I will try to implement the feature and run all the bug testing for it. Hopefully, I will have it implemented and ready by Saturday. BTW, do you want me to visually indicate if the comment is selected? If so, should I do it using the link/information you provided for comments?

Thank you!

[changminbark]

Wait, I just tried selecting a single comment, but it does not seem to visually update that it is being selected. Is this something wrong with the Blockly side? Shouldn't it be visually highlighted?

[changminbark]

And are comments the only other thing that can be selected by the multiselect? or are there any other things that can be selected by it?

[HollowMan6]

I am guessing that this might have to do with something calling the connect on the shadow block. For now, I have created a type checking when I add to the connectionDBList. I will continue to test dragging and seeing if I encounter this issue. It would also be helpful if you can provide more details on how to replicate this (if you have the time).

No worries, I will let you know if I encounter this again.

BTW, do you want me to visually indicate if the comment is selected? If so, should I do it using the link/information you provided for comments?

Wait, I just tried selecting a single comment, but it does not seem to visually update that it is being selected. Is this something wrong with the Blockly side? Shouldn't it be visually highlighted?

The visual update for the Blockly workspace comment works fine if you go to: https://blockly-demo.appspot.com/static/demos/code/index.html We should of course visually indicate if the comment is selected. The IDraggable should be able to handle those comment.

And are comments the only other thing that can be selected by the multiselect? or are there any other things that can be selected by it?

I'm not aware of any other things currently, but in theory, everything that is an IDraggable can be selected by multi-select, so maybe another plugin implements another type of IDraggable, so to ensure compatibility, it would be good if you make your design as flexible as possible.

[changminbark]

I am guessing that this might have to do with something calling the connect on the shadow block. For now, I have created a type checking when I add to the connectionDBList. I will continue to test dragging and seeing if I encounter this issue. It would also be helpful if you can provide more details on how to replicate this (if you have the time).

No worries, I will let you know if I encounter this again.

BTW, do you want me to visually indicate if the comment is selected? If so, should I do it using the link/information you provided for comments?

Wait, I just tried selecting a single comment, but it does not seem to visually update that it is being selected. Is this something wrong with the Blockly side? Shouldn't it be visually highlighted?

The visual update for the Blockly workspace comment works fine if you go to: https://blockly-demo.appspot.com/static/demos/code/index.html We should of course visually indicate if the comment is selected. The IDraggable should be able to handle those comment.

And are comments the only other thing that can be selected by the multiselect? or are there any other things that can be selected by it?

I'm not aware of any other things currently, but in theory, everything that is an IDraggable can be selected by multi-select, so maybe another plugin implements another type of IDraggable, so to ensure compatibility, it would be good if you make your design as flexible as possible.

Another weird thing I noticed is that the comments themselves do not have an ID in the HTML code of the webpage even though it is a Blockly Draggable element. How should I select specific comments? Below is a picture of the HTML of a given workspace comment. This is unlike blocks, which all have an ID, which I can use in manipulation like dragging, copying, etc.

Currently, this is how I am adding the comments to the dragSelect selectables. Since it is not part of the g tag; it is part of the rect tag. Will it be a problem if all other draggable elements that may be implemented in the future do not use these tags?

[HollowMan6]

No, I was meant for WORKSPACE comments instead of BLOCK comments. ~Block comments are not selectable.~ (No need to select them) Do check out the link I mentioned before: https://developers.google.com/blockly/guides/configure/web/workspace_comment

Another weird thing I noticed is that the comments themselves do not have an ID in the HTML code of the webpage even though it is a Blockly Draggable element. How should I select specific comments? Below is a picture of the HTML of a given workspace comment. This is unlike blocks, which all have an ID, which I can use in manipulation like dragging, copying, etc.

Edit: I just checked and found that the workspace comments also doesn't have an ID, so maybe it's the case that ID no longer suits our need and we should store the IDraggle instances directly.

Currently, this is how I am adding the comments to the dragSelect selectables. Since it is not part of the g tag; it is part of the rect tag.

For Blockly workspace comments, we still use the SVG groups, but now they belongs to a class called blocklyEditable, so you might want to reflect that as well.

Will it be a problem if all other draggable elements that may be implemented in the future do not use these tags?

That's why I want to pause a while and continue reviewing after you finish this implementation. Let's see.

[changminbark]

No, I was meant for WORKSPACE comments instead of BLOCK comments. ~Block comments are not selectable.~ (No need to select them) Do check out the link I mentioned before: https://developers.google.com/blockly/guides/configure/web/workspace_comment

Another weird thing I noticed is that the comments themselves do not have an ID in the HTML code of the webpage even though it is a Blockly Draggable element. How should I select specific comments? Below is a picture of the HTML of a given workspace comment. This is unlike blocks, which all have an ID, which I can use in manipulation like dragging, copying, etc.

Edit: I just checked and found that the workspace comments also doesn't have an ID, so maybe it's the case that ID no longer suits our need and we should store the IDraggle instances directly.

Currently, this is how I am adding the comments to the dragSelect selectables. Since it is not part of the g tag; it is part of the rect tag.

For Blockly workspace comments, we still use the SVG groups, but now they belongs to a class called blocklyEditable, so you might want to reflect that as well.

Will it be a problem if all other draggable elements that may be implemented in the future do not use these tags?

That's why I want to pause a while and continue reviewing after you finish this implementation. Let's see.

From what I've tested so far, I don't think it would be possible to implement the workspace comment multiselect without using the blocklyEditable class selector. I tried just using the blocklyDraggable class selector and it would not select the comments. This may be an issue when trying to support the multiselect of multiple different draggables. To make things standardized, they should all belong to the same class, so that multiselection of them can be easily captured. Another thing I found out is that to use the CSS query selector, I need to put a second selector using the child combinator, or else it would select the whole canvas.

For now, this is what I have, to select the workspace comment. If I just have g.blocklyEditable, the element selected will be the blocklyBlockCanvas.

[HollowMan6]

Anyway, feel free to implement as you wish to have an initial draft to get this working (I haven't investigated this myself), you just need to keep in mind that extensibility is also important for future maintainability.

[changminbark]

Something that I am also running into as a problem is that I cannot directly add the HTML object and access the class's methods. I am not sure how to access the methods like isMovable. With the blocks, you obtained the BlockSVG object using the workspace's getBlockById method, but I am not aware of an equivalent or similar method for workspace comments. Should I reach out to Beka for some advice?

[HollowMan6]

Something that I am also running into as a problem is that I cannot directly add the HTML object and access the class's methods. I am not sure how to access the methods like isMovable. With the blocks, you obtained the BlockSVG object using the workspace's getBlockById method, but I am not aware of an equivalent or similar method for workspace comments. Should I reach out to Beka for some advice?

Ah yeah, then I guess it's a good idea to ask Beka for help.

[mark-friedman]

With the blocks, you obtained the BlockSVG object using the workspace's getBlockById method, but I am not aware of an equivalent or similar method for workspace comments. Should I reach out to Beka for some advice?

There is a Workspace.getCommentById method in Blockly, but it appears to be marked @internal. So perhaps you could ask Beka if Blockly could expose the Workspace.getCommentById method and add the id as an attribute to the HTML element for the workspace comment, though she might have other, better, thoughts on how to handle this.

[changminbark]

What should I do until this feature gets implemented? There is nothing much I can do on my side to continue developing this for workspace comments. My only worry is that I may not be able to get this fully implemented and tested before July 8. I also have other responsibilities, so I do not think I can personally implement and fix the features in the issue that Beka opened.

[HollowMan6]

What should I do until this feature gets implemented? There is nothing much I can do on my side to continue developing this for workspace comments.

We should, of course, first start to do some refactoring work to get workspace comments and possibly other IDraggable supported, so it would need a careful redesign of the data structure for blockSelection, to upgrade it into something maybe called iDraggerSelection. And you definitely want to fix those errors when we drag a rectangle on top of workspace comments (Not working doesn't mean that we should have those errors). You will also need to add a filter for those actions that are not supported by workspace comments. (e.g. in the keyboard shortcut and context menu) Once this feature gets implemented, ideally we should get it working without having any major changes.

My only worry is that I may not be able to get this fully implemented and tested before July 8.

It's still fine. Initially, when I started to plan this GSoC idea, I thought everything should work out of the box, but now it looks like we still need more work as issues pop up here and there.

I also have other responsibilities, so I do not think I can personally implement and fix the features in the issue that Beka opened.

For https://github.com/google/blockly/issues/8242, I don't think it's hard to implement as you can refer to the blockSVG code and basically do the same thing for the workspace comment, so I would still encourage you to try that, but if you are not interested in it anyway, it's still fine, but I do hope that we can get workspace comment supported sooner or later as I believe this is the major reason why Blockly team decide to introduce this change.

[changminbark]

I think for the blockSelection, we can still keep the same data structure as long as the Blockly team implements id's for all draggable objects. I think that this should be required unless there is some other way of accessing the draggable objects without using their id's and a corresponding workspace.getById() method. Maybe we just need to rename it? What do you think?

As for the errors, the only reason I am getting errors is because of me trying to update them visually to be selected, which requires the evaluation of if statements related to the isMovable() method. Since I cannot access these methods without having direct access to the objects themselves, I am encountering the error. I actually do not think there is a problem with adding the objects into the multiDraggable object (as it is just appending a Map).

If I have the time over this weekend, I may try to implement/look into https://github.com/google/blockly/issues/8242. Do you know which part of the code is related to what is being shown in the HTML of the page? This would be helpful.

For now, I will add support/filtering for the workspace comments. More specifically, I will work on the multiselect_controls, multiselect_contextmenu, and multiselect_shortcut files, as I think these files would need the most updates. Do you know which shortcuts/contextmenu features that the workspace comments should not interact with?

[HollowMan6]

I think for the blockSelection, we can still keep the same data structure as long as the Blockly team implements id's for all draggable objects. I think that this should be required unless there is some other way of accessing the draggable objects without using their id's and a corresponding workspace.getById() method. Maybe we just need to rename it? What do you think?

According to Mark, "There is a Workspace.getCommentById method in Blockly, but it appears to be marked @internal." So that's why I thought it would be needed. I think what you are proposing here would be a better idea to get this properly supported on our side, so you can discuss it with the Blockly team / Beka under that issue https://github.com/google/blockly/issues/8242 to see if it's feasible. I will follow you up if there's anything I can help.

If I have the time over this weekend, I may try to implement/look into google/blockly#8242. Do you know which part of the code is related to what is being shown in the HTML of the page? This would be helpful.

If you are not sure, you can ask for Beka / other Blockly team members for help as this will be a contribution to Blockly directly, you will submit a separate PR there and they will review your code, so it's better to discuss what to modify to fit their need. Also, I haven't checked the code myself, and the only suggestion I have if I would start to implement this, is to refer to the BlockSvg code.

As for the errors, the only reason I am getting errors is because of me trying to update them visually to be selected, which requires the evaluation of if statements related to the isMovable() method. Since I cannot access these methods without having direct access to the objects themselves, I am encountering the error. I actually do not think there is a problem with adding the objects into the multiDraggable object (as it is just appending a Map).

Okay, anyway, let's say, if we can't get workspace comments fully supported before this year's GSoC ends, at that time, it would be a good idea to not have these errors as we might want to make a new release.

For now, I will add support/filtering for the workspace comments. More specifically, I will work on the multiselect_controls, multiselect_contextmenu, and multiselect_shortcut files, as I think these files would need the most updates. Do you know which shortcuts/contextmenu features that the workspace comments should not interact with?

Unfortunately, I'm not familiar with workspace comments as well, please read the documentation, try and find out these by yourself https://developers.google.com/blockly/guides/configure/web/workspace_comment

[mark-friedman]

If I have the time over this weekend, I may try to implement/look into google/blockly#8242. Do you know which part of the code is related to what is being shown in the HTML of the page? This would be helpful.

I believe that this is where you could add a data-id attribute to the rendered comment. Also, Here is where Workspace.getCommentById is marked internal.

I would suggest cloning the Blockly repo, making the appropriate changes, building Blockly, and using it with your plugin. If it goes well and it solves your problems, then you can make a PR to resolve google/blockly#8242