Closed HollowMan6 closed 1 day ago
HollowMan6
commented
3 months ago The new changes in Blockly 11 may cause the compatibility issue with the backpack plugin again, so we should re-evaluate again after #39 is done https://github.com/google/blockly-samples/issues/2311
changminbark
commented
3 months ago 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
commented
1 month ago Quick update: The newly updated multiselect plugin works with the scroll-options plugin as well as multiple workspaces.
changminbark
commented
1 month ago Checklist:
Original
Workspace-related
Blocks-related
changminbark
commented
3 weeks ago https://developers.google.com/blockly/guides/configure/web/keyboard-nav
Tested with the backpack plugin and multiselect plugin
Note: When editing a field (e.g. true/false block), I cannot use "w" or "s" to navigate through the field options. However, I can use the arrow keys to navigate through the field options. I don't know if this is intended behavior for the keyboard navigation plugin. I tested this with and without the multiselect plugin.
Note: I am not sure if there is a bug with the keyboard navigation's copy/paste function. The following gif was tested without the multiselect plugin. I cannot seem to reproduce it, so I am also not sure about this bug.
However, with the multiselect plugin, it seems to be holding two different copyData storages (so two different copy-pastes are running simultaneously with the shortcut).
What should be the intended behavior here? Should we incorporate the copy functionality of the keyboard navigation plugin into the multiselect plugin? I don't think this would be a good idea as not everyone will use both plugins at the same time. Do you have any other suggestions?
Note: When installing the multiselect plugin first, there is no error regarding the delete shortcut. However, when installing the keyboard navigation plugin first, we encounter this error:
I am not sure why we don't see this issue when we install the multiselect plugin first.
Multiselect Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
Note: The multiselect mode is activated when holding shift for moving around the keyboard navigation cursor, but this does not cause any unwanted behavior as the multiselect mode affects cursor/mouse behavior.
Backpack Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
HollowMan6
commented
3 weeks ago Note: When editing a field (e.g. true/false block), I cannot use "w" or "s" to navigate through the field options. However, I can use the arrow keys to navigate through the field options. I don't know if this is intended behavior for the keyboard navigation plugin. I tested this with and without the multiselect plugin.
The Keyboard Navigation Plugin is highly experimental, so I would expect this to happen. You can report it to the Blockly team.
Note: I am not sure if there is a bug with the keyboard navigation's copy/paste function. The following gif was tested without the multiselect plugin. I cannot seem to reproduce it, so I am also not sure about this bug.
However, with the multiselect plugin, it seems to be holding two different copyData storages (so two different copy-pastes are running simultaneously with the shortcut).
What should be the intended behavior here? Should we incorporate the copy functionality of the keyboard navigation plugin into the multiselect plugin? I don't think this would be a good idea as not everyone will use both plugins at the same time. Do you have any other suggestions?
That's one thing to think about. I guess the Blockly team doesn't have a good solution for this either (e.g. cross tab copy-paste and the keyboard navigation also have two set of copyData storages). Of course, we shouldn't incorporate the copy functionality of the keyboard navigation plugin into the multiselect plugin this time, but it would be good to work out some design so that people can connect the multi-select copyData storages with possibly other plugins /for other uses (extensibility).
Note: When installing the multiselect plugin first, there is no error regarding the delete shortcut. However, when installing the keyboard navigation plugin first, we encounter this error:
I am not sure why we don't see this issue when we install the multiselect plugin first.
This is exactly https://github.com/mit-cml/workspace-multiselect/issues/41 Let me know if you have any insights on this one.
changminbark
commented
3 weeks ago The Keyboard Navigation Plugin is highly experimental, so I would expect this to happen. You can report it to the Blockly team.
This may be related to this issue: https://github.com/google/blockly-samples/issues/2380
That's one thing to think about. I guess the Blockly team doesn't have a good solution for this either (e.g. cross tab copy-paste and the keyboard navigation also have two set of copyData storages). Of course, we shouldn't incorporate the copy functionality of the keyboard navigation plugin into the multiselect plugin this time, but it would be good to work out some design so that people can connect the multi-select copyData storages with possibly other plugins /for other uses (extensibility).
I think that a possible solution to this would be to implement what the multiselect plugin is doing in Blockly core. Instead of having the base copy/cut/paste function directly using the copyData of that one block/element, it should define a set (can be called copyData) that contains a set of each element's copy data (whether it be just one element or multiple). This can be used across different plugins that have their own copy/cut/paste shortcut functions defined.
This is exactly #41 Let me know if you have any insights on this one.
I found out the reason why this occurs. Currently, the keyboard navigation plugin allows for collisions when they register the key mappings for the shortcut registry while the multiselect plugin does not. This should be a simple fix by also allowing the shortcuts in the multiselect plugin to allow for collisions. However, I am not sure if this would be a good idea as other plugins may have copy/cut/paste shortcuts that behave differently. Having multiple copy/cut/paste functions would lead to unexpected behaviors. It would also be impossible to make an all-in-one copy/cut/paste functions for all the plugins. So I am not sure how to approach this issue.
If there is nothing else that I can investigate in, I will mark this cross-checking as done and move onto the next plugin.
HollowMan6
commented
3 weeks ago I think that a possible solution to this would be to implement what the multiselect plugin is doing in Blockly core.
You can discuss that with the Blockly team to see if this is feasible.
I found out the reason why this occurs. Currently, the keyboard navigation plugin allows for collisions when they register the key mappings for the shortcut registry while the multiselect plugin does not.
Yeah, I mean that and that's why I mentioned this here: https://groups.google.com/g/blockly/c/MuL2Ln8SwDU/m/9WihibDsAgAJ
This should be a simple fix by also allowing the shortcuts in the multiselect plugin to allow for collisions. However, I am not sure if this would be a good idea as other plugins may have copy/cut/paste shortcuts that behave differently. Having multiple copy/cut/paste functions would lead to unexpected behaviors. It would also be impossible to make an all-in-one copy/cut/paste functions for all the plugins. So I am not sure how to approach this issue.
You can try and see if it works when you allow for collisions. One of the solutions I proposed (guess, not verified) is to temporarily unregister the multi-select plugin key mappings when keyboard navigation is activated, and register that back when deactivated. You can also verify if this works/is possible or not. if you have any better ideas/comments, feel free to continue commenting under that thread.
changminbark
commented
3 weeks ago You can discuss that with the Blockly team to see if this is feasible.
I will send an email to them soon.
Yeah, I mean that and that's why I mentioned this here: https://groups.google.com/g/blockly/c/MuL2Ln8SwDU/m/9WihibDsAgAJ
I don't have access to comment on the thread.
You can try and see if it works when you allow for collisions. One of the solutions I proposed (guess, not verified) is to temporarily unregister the multi-select plugin key mappings when keyboard navigation is activated, and register that back when deactivated. You can also verify if this works/is possible or not. if you have any better ideas/comments, feel free to continue commenting under that thread.
I was able to do the temporary unregister and it seems to be working. Since it works, should I implement it into the multi-select plugin and push the change into the blockly-v11 branch?
HollowMan6
commented
3 weeks ago Yeah, I mean that and that's why I mentioned this here: https://groups.google.com/g/blockly/c/MuL2Ln8SwDU/m/9WihibDsAgAJ
I don't have access to comment on the thread.
Really? It's a public Google group.
You can try and see if it works when you allow for collisions. One of the solutions I proposed (guess, not verified) is to temporarily unregister the multi-select plugin key mappings when keyboard navigation is activated, and register that back when deactivated. You can also verify if this works/is possible or not. if you have any better ideas/comments, feel free to continue commenting under that thread.
I was able to do the temporary unregister and it seems to be working. Since it works, should I implement it into the multi-select plugin and push the change into the blockly-v11 branch?
You mean implement the "allow for collisions"? Sure, you can test that and once you think it's fine, it's always preferred to have that allowed.
changminbark
commented
3 weeks ago Yeah, I mean that and that's why I mentioned this here: https://groups.google.com/g/blockly/c/MuL2Ln8SwDU/m/9WihibDsAgAJ
I don't have access to comment on the thread.
Really? It's a public Google group.
You mean implement the "allow for collisions"? Sure, you can test that and once you think it's fine, it's always preferred to have that allowed.
The only problem I have encountered is that when we unregister the multiselect shortcuts when we are in the keyboard navigation mode, we cannot use the cursor to select/copy/cut/paste. We can only copy/cut/paste the blocks selected in the keyboard navigation mode. I have tried registering the original Blockly core shortcuts, but they do not allow for collisions and will run into an error with the keyboard navigation plugin. If you think this behavior is still okay, I will push it into the blockly-v11 branch of the multiselect plugin.
changminbark
commented
3 weeks ago This is a gif of the plugins working together:
HollowMan6
commented
3 weeks ago Have you actually joined the group?
You mean implement the "allow for collisions"? Sure, you can test that and once you think it's fine, it's always preferred to have that allowed.
The only problem I have encountered is that when we unregister the multiselect shortcuts when we are in the keyboard navigation mode, we cannot use the cursor to select/copy/cut/paste. We can only copy/cut/paste the blocks selected in the keyboard navigation mode. I have tried registering the original Blockly core shortcuts, but they do not allow for collisions and will run into an error with the keyboard navigation plugin. If you think this behavior is still okay, I will push it into the blockly-v11 branch of the multiselect plugin.
I don’t quite understand, are you combining the things I mentioned with allowing collisions, or simply the methods I mentioned? If later, what are you going to commit in our code base? We can discuss this more during our meeting. BTW, since we are touching something new, there is no regression involved, do you think this is Okay or not? It’s always good to develop a “design taste” for everyone, not just for me. For me, my principle is that, if it introduces another higher priority bug that doesn’t have a possible workaround, it’s better not to introduce it.
HollowMan6
commented
3 weeks ago changminbark
commented
3 weeks ago Have you actually joined the group?
Oh, I guess that was the issue.
I don’t quite understand, are you combining the things I mentioned with allowing collisions, or simply the methods I mentioned? If later, what are you going to commit in our code base? We can discuss this more during our meeting. BTW, since we are touching something new, there is no regression involved, do you think this is Okay or not? It’s always good to develop a “design taste” for everyone, not just for me. For me, my principle is that, if it introduces another higher priority bug that doesn’t have a possible workaround, it’s better not to introduce it.
So what I did was I allowed collisions from the multiselect plugin side and then made it so that the multiselect plugin's shortcuts are unregistered when we turn on the keyboard navigation mode and re-register it when we turn off the keyboard navigation mode. I can just commit the portion of the code that allows for collisions from the multiselect plugin side, or I can commit the whole code that also unregisters/re-registers the shortcuts (this is based on the conditional shown below).
Can you by any chance multi select the blocks purely via keyboard, without the use of the mouse?
No, this is not possible. That is why I am a little unsure about whether we should commit just the collision allowance or also the shortcut unregistration/re-registration.
In my opinion, it does not make sense for the keyboard navigation to rely on the multiselect plugin to select multiple blocks. That should be feature on its own in the keyboard navigation plugin. As for the behavior shown in the gif, I think it should be fine as long as the user knows that you are not able to copy/paste using the cursor at all (as we unregister all cursor related copy/paste functions including the original Blockly core one) while in the keyboard navigation mode.
HollowMan6
commented
3 weeks ago Have you actually joined the group?
Oh, I guess that was the issue.
I don’t quite understand, are you combining the things I mentioned with allowing collisions, or simply the methods I mentioned? If later, what are you going to commit in our code base? We can discuss this more during our meeting. BTW, since we are touching something new, there is no regression involved, do you think this is Okay or not? It’s always good to develop a “design taste” for everyone, not just for me. For me, my principle is that, if it introduces another higher priority bug that doesn’t have a possible workaround, it’s better not to introduce it.
So what I did was I allowed collisions from the multiselect plugin side and then made it so that the multiselect plugin's shortcuts are unregistered when we turn on the keyboard navigation mode and re-register it when we turn off the keyboard navigation mode. I can just commit the portion of the code that allows for collisions from the multiselect plugin side, or I can commit the whole code that also unregisters/re-registers the shortcuts (this is based on the conditional shown below).
Sure, you can commit that. BTW, are you going to expose "unbindMultiselectCopyPaste_" to the developer, or is this going to happen automatically? If the former, there's a naming issue here in the method (public ones should not have an underscore as the suffix).
In my opinion, it does not make sense for the keyboard navigation to rely on the multiselect plugin to select multiple blocks. That should be feature on its own in the keyboard navigation plugin. As for the behavior shown in the gif, I think it should be fine as long as the user knows that you are not able to copy/paste using the cursor at all (as we unregister all cursor related copy/paste functions including the original Blockly core one) while in the keyboard navigation mode.
In my opinion, it does not make sense for the keyboard navigation to rely on the multiselect plugin to select multiple blocks.
It looks like the Blockly team wants to integrate the keyboard navigation into core in the future, so in the end, we should still be able to do that https://github.com/google/blockly-samples/issues/2380#issuecomment-2142176197
I think it should be fine as long as the user knows that you are not able to copy/paste using the cursor at all (as we unregister all cursor related copy/paste functions including the original Blockly core one)
How will the user know? It sounds like only the developer knows that.
changminbark
commented
3 weeks ago Sure, you can commit that. BTW, are you going to expose "unbindMultiselectCopyPaste_" to the developer, or is this going to happen automatically? If the former, there's a naming issue here in the method (public ones should not have an underscore as the suffix).
This will happen automatically, as it is a 'keydown' event listener. It only fires if it detects that the workspace is in keyboard accessibility mode.
It looks like the Blockly team wants to integrate the keyboard navigation into core in the future, so in the end, we should still be able to do that google/blockly-samples#2380 (comment)
Then the Blockly team would have to make the original core shortcuts to allow for collisions. Because right now, we cannot use both (core and keyboard nav) shortcuts as the core does not allow for collisions (if we are to re-register the core shortcuts). This would be the solution to allowing users to use the core shortcuts while in keyboard navigation mode (with the multiselect plugin). So, currently, after I push the latest commit, we will be able to use both plugins, but we will not be able to copy/paste regularly using a cursor when the keyboard navigation mode is on unless we make a change to the core shortcuts OR change the keyboard navigation plugin to unregister/re-register their shortcuts.
How will the user know? It sounds like only the developer knows that.
I guess there is no way the user knows unless there is some kind of written documentation/tutorial for them. I just thought it did not make sense for a user to turn on the keyboard navigation mode and then proceed to use the cursor to copy/paste. Isn't the purpose of the keyboard navigation mode to completely remove the need for a mouse/cursor?
HollowMan6
commented
3 weeks ago I just thought it did not make sense for a user to turn on the keyboard navigation mode and then proceed to use the cursor to copy/paste. Isn't the purpose of the keyboard navigation mode to completely remove the need for a mouse/cursor? So, currently, after I push the latest commit, we will be able to use both plugins, but we will not be able to copy/paste regularly using a cursor when the keyboard navigation mode is on unless we make a change to the core shortcuts OR change the keyboard navigation plugin to unregister/re-register their shortcuts.
Sounds good if we document this on our side.
Then the Blockly team would have to make the original core shortcuts to allow for collisions.
Maybe you also want to make the Blockly team aware of this (by opening an issue there or something)
changminbark
commented
3 weeks ago Okay, I have pushed the commit in the PR and created an issue: https://github.com/google/blockly-samples/issues/2424
I will now start cross-checking with the next plugin (Shadow-block-converter).
changminbark
commented
2 weeks ago https://github.com/google/blockly-samples/tree/master/plugins/shadow-block-converter
Tested with the backpack plugin, keyboard navigation plugin, and multiselect plugin
Note: I think that I've come across a bug where editing a shadow block will select the parent block (at least visually). However, when trying to select something else (like a block or the workspace), the former shadow block's parent block does not get unselected (visually). This was tested without any of the other plugins (except for backpack). I will report this on the Blockly plugins github page (https://github.com/google/blockly-samples/issues/2426).
Note: When a shadow block gets converted into a regular block, it is not in the multiselect draggable's subdraggables, but this is not an issue for modifying/moving the multiselection as the parent block is still in the selection.
Multiselect Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
Note: The multiselect mode is activated when holding shift for moving around the keyboard navigation cursor, but this does not cause any unwanted behavior as the multiselect mode affects cursor/mouse behavior.
Backpack Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
Keyboard Navigation Plugin: Everything works as intended (use cases). Editing a shadow block converts it to a regular block and the keyboard navigation's cursor/indicator is moved back to the workspace level.
changminbark
commented
2 weeks ago If there are no concerns or questions, I will move on to the next plugin (Workspace-content-highlight) soon.
changminbark
commented
2 weeks ago https://github.com/google/blockly-samples/tree/master/plugins/content-highlight
Tested with the backpack plugin, keyboard navigation plugin, shadow block converter plugin and multiselect plugin
Multiselect Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
Backpack Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
Keyboard Navigation Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
Shadow Block Converter Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
changminbark
commented
2 weeks ago https://github.com/google/blockly-samples/tree/master/plugins/disable-top-blocks
Tested with the backpack plugin and multiselect plugin.
Note: We have to install the multiselect plugin first if we want the disable top blocks plugin to work properly as the multiselect plugin installs/overrides the disable top blocks plugin's context menu. Should we write this down somewhere or is it enough to indicate it in this comment?
Multiselect Plugin:
Everything works as intended (use cases). We must just make sure that the disable top blocks plugin is installed after the multiselect plugin:
Backpack Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
HollowMan6
commented
2 weeks ago Note: We have to install the multiselect plugin first if we want the disable top blocks plugin to work properly as the multiselect plugin installs/overrides the disable top blocks plugin's context menu. Should we write this down somewhere or is it enough to indicate it in this comment?
Multiselect Plugin: Everything works as intended (use cases). We must just make sure that the disable top blocks plugin is installed after the multiselect plugin:
This sounds very similar to the keyboard navigation plugin, does this cause any conflict/missing feature? If not, then documenting this to let people know in README is a good idea.
changminbark
commented
2 weeks ago This sounds very similar to the keyboard navigation plugin, does this cause any conflict/missing feature? If not, then documenting this to let people know in README is a good idea.
This does not cause any bugs/conflicts/unintended use cases. I will document this in the README and push it to the blockly-v11 branch.
changminbark
commented
1 week ago https://github.com/google/blockly-samples/tree/master/plugins/strict-connection-checker
Tested with the backpack plugin and multiselect plugin.
Without strict connection check:
With strict connection check:
Multiselect Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
Backpack Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
changminbark
commented
1 week ago https://github.com/google/blockly-samples/tree/master/plugins/block-dynamic-connection
Tested with the backpack plugin and multiselect plugin.
Note: If you watch the gif, when I undo the connection of the dynamic connection, some of the blocks are moved to a random location first. This only occurs with the multiselection. I am not sure what is causing the location to be moved randomly. However, it does not seem to be causing any major issues (unintended behavior). I will continue looking into what is causing it.
Note: Another thing I noticed is that the dynamic create text block does not shrink back to its original number when I undo the action of dynamically connecting the block. This is also present without the multiselect plugin. I opened an issue at https://github.com/google/blockly-samples/issues/2434
Test environment:
/**
* @license
* Copyright 2022 MIT
* SPDX-License-Identifier: Apache-2.0
*/
/**
* @fileoverview Plugin test.
*/
import * as Blockly from 'blockly';
import {toolboxCategories, createPlayground} from '@blockly/dev-tools';
import {Multiselect} from '../src/index';
import {Backpack} from '@blockly/workspace-backpack';
import {NavigationController} from '@blockly/keyboard-navigation';
import {shadowBlockConversionChangeListener} from '@blockly/shadow-block-converter';
import {ContentHighlight} from "@blockly/workspace-content-highlight";
import {DisableTopBlocks} from "@blockly/disable-top-blocks";
import {pluginInfo as StrictConnectionsPluginInfo} from '@blockly/plugin-strict-connection-checker';
import * as BlockDynamicConnection from "@blockly/block-dynamic-connection";
/**
* Create a workspace.
* @param {HTMLElement} blocklyDiv The blockly container div.
* @param {!Blockly.BlocklyOptions} options The Blockly options.
* @returns {!Blockly.WorkspaceSvg} The created workspace.
*/
function createWorkspace(blocklyDiv, options) {
const workspace = Blockly.inject(blocklyDiv, options);
// Initialize backpack plugin.
const backpack = new Backpack(workspace);
backpack.init();
// // KEYBOARD NAVIGATION PLUGIN (DONE) ============================================================
// // Initialize plugin.
// const navigationController = new NavigationController();
// navigationController.init();
// navigationController.addWorkspace(workspace);
// // Turns on keyboard navigation.
// navigationController.enable(workspace);
// // SHADOW BLOCK CONVERTER PLUGIN (DONE) =========================================================
// workspace.addChangeListener(shadowBlockConversionChangeListener);
// // CONTENT HIGHLIGHT PLUGIN (DONE) ==============================================================
// // Initialize plugin.
// const contentHighlight = new ContentHighlight(workspace);
// contentHighlight.init();
// // DISABLE TOP BLOCKS PLUGIN (DONE) ==============================================================
// // The plugin must be initialized before it has any effect.
// const disableTopBlocksPlugin = new DisableTopBlocks();
// disableTopBlocksPlugin.init();
// // STRICT CONNECTION CHECKER PLUGIN (DONE) =======================================================
// BLOCK DYNAMIC CONNECTION PLUGIN
// Add the change listener so connections will be finalized on deletion.
workspace.addChangeListener(BlockDynamicConnection.finalizeConnections);
const multiselectPlugin = new Multiselect(workspace);
multiselectPlugin.init(options);
return workspace;
}
document.addEventListener('DOMContentLoaded', function() {
BlockDynamicConnection.overrideOldBlockDefinitions();
const toolbox = {
kind: 'flyoutToolbox',
contents: [
{
kind: 'block',
type: 'text_join',
},
{
kind: 'block',
type: 'lists_create_with',
},
{
kind: 'block',
type: 'controls_if',
},
{
kind: 'block',
type: 'logic_boolean',
fields: {
BOOL: 'TRUE',
},
},
{
kind: 'block',
type: 'text',
fields: {
TEXT: 'abc',
},
},
{
kind: 'block',
type: 'math_number',
fields: {
NUM: '123',
},
},
{
kind: 'block',
type: 'text_print',
inputs: {
TEXT: {
shadow: {
type: 'text',
fields: {
TEXT: 'abc',
},
},
block: undefined,
},
},
},
],
};
const defaultOptions = {
toolbox: toolbox,
plugins: {
// ...StrictConnectionsPluginInfo,
connectionPreviewer:
BlockDynamicConnection.decoratePreviewer(
// Replace with a custom connection previewer, or remove to decorate
// the default one.
Blockly.InsertionMarkerPreviewer,
),
},
useDoubleClick: true,
bumpNeighbours: false,
multiFieldUpdate: true,
multiselectIcon: {
hideIcon: false,
weight: 3,
enabledIcon: 'media/select.svg',
disabledIcon: 'media/unselect.svg',
},
multiSelectKeys: ['Shift'],
multiselectCopyPaste: {
crossTab: true,
menu: true,
},
grid: {
spacing: 25,
length: 3,
colour: '#ccc',
snap: true,
},
move: {
wheel: true,
},
zoom: {
wheel: true,
},
};
createPlayground(document.getElementById('root'), createWorkspace,
defaultOptions);
});Multiselect Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
Backpack Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
changminbark
commented
1 week ago After looking into the weird undo behavior, I found that each block undergoes a drag and then two move actions. This seems to be the same for the block that does not move to a random location prior to connecting (the block with the normal behavior also has 1 drag and then 2 move actions being called). I am not too sure what is causing this. If you have any suggestions, that would be helpful. However, this might also be related to the weird undo compatibility with the multiselect plugin.
changminbark
commented
1 week ago https://github.com/google/blockly-samples/tree/master/plugins/block-shareable-procedures
Tested with the multiselect plugin.
Test environment index.js:
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
/**
* @fileoverview Block test.
*/
import * as Blockly from 'blockly';
import {toolboxCategories} from '@blockly/dev-tools';
import {
blocks,
unregisterProcedureBlocks,
registerProcedureSerializer,
} from '@blockly/block-shareable-procedures';
import {ProcedureBase} from '@blockly/block-shareable-procedures';
import {Multiselect} from "../src";
/* eslint no-unused-vars: "off" */
unregisterProcedureBlocks();
Blockly.common.defineBlocks(blocks);
registerProcedureSerializer();
let workspace1;
let workspace2;
document.addEventListener('DOMContentLoaded', function () {
injectTwoWorkspaces();
});
/**
* Injects two workspaces that share procedure models. Undo and redo are off.
*/
function injectTwoWorkspaces() {
const options = {
toolbox: toolboxCategories,
useDoubleClick: true,
bumpNeighbours: false,
multiFieldUpdate: true,
multiselectIcon: {
hideIcon: false,
weight: 3,
enabledIcon: 'media/select.svg',
disabledIcon: 'media/unselect.svg',
},
multiSelectKeys: ['Shift'],
multiselectCopyPaste: {
crossTab: true,
menu: true,
},
grid: {
spacing: 25,
length: 3,
colour: '#ccc',
snap: true,
},
move: {
wheel: true,
},
zoom: {
wheel: true,
},
};
workspace1 = Blockly.inject('blockly1', options);
workspace2 = Blockly.inject('blockly2', options);
// If we allow undoing operations, it can create event loops when sharing
// events between workspaces.
workspace1.MAX_UNDO = 0;
workspace2.MAX_UNDO = 0;
workspace1.addChangeListener(createEventSharer(workspace2));
workspace2.addChangeListener(createEventSharer(workspace1));
workspace1.addChangeListener(
createSerializationListener(workspace1, 'save1'),
);
workspace2.addChangeListener(
createSerializationListener(workspace2, 'save2'),
);
const multiselectPlugin1 = new Multiselect(workspace1);
multiselectPlugin1.init(options);
const multiselectPlugin2 = new Multiselect(workspace2);
multiselectPlugin2.init(options);
document
.getElementById('load')
.addEventListener('click', () => reloadWorkspaces());
}
/** Injects one workspace with undo and redo enabled. */
function injectOneWorkspace() {
const elem = document.getElementById('blockly2');
elem.parentElement.removeChild(elem);
const options = {
toolbox: toolboxCategories,
};
workspace1 = Blockly.inject('blockly1', options);
workspace1.addChangeListener(
createSerializationListener(workspace1, 'save1'),
);
document
.getElementById('load')
.addEventListener('click', () => loadWorkspace());
}
/**
* Returns an event listener that shares procedure and var events from the
* connected workspace to the other workspace.
* @param {!Blockly.Workspace} otherWorkspace The workspace to share events to.
* @returns {function(Blockly.Events.Abstract)} The listener.
*/
function createEventSharer(otherWorkspace) {
return (e) => {
if (
!(e instanceof ProcedureBase) &&
!(e instanceof Blockly.Events.VarBase)
) {
return;
}
let event;
try {
event = Blockly.Events.fromJson(e.toJson(), otherWorkspace);
} catch (e) {
// Could not deserialize event. This is expected to happen. E.g.
// when round-tripping parameter deletes, the delete in the
// secondary workspace cannot be deserialized into the original
// workspace.
return;
}
event.run(true);
};
}
/**
* Returns an event listener that serializes the given workspace to JSON and
* outputs it to the text area with the given ID.
* @param {!Blockly.Workspace} workspace The workspace to serialize.
* @param {string} textAreaId The ID of the text area to output to.
* @returns {function(Blockly.Events.Abstract)} The listener.
*/
function createSerializationListener(workspace, textAreaId) {
const textArea = document.getElementById(textAreaId);
return (e) => {
if (workspace.isDragging()) return;
textArea.value = JSON.stringify(
Blockly.serialization.workspaces.save(workspace),
undefined,
2,
);
};
}
/**
* Reloads both workspaces to showcase serialization working.
*/
async function reloadWorkspaces() {
const save1 = Blockly.serialization.workspaces.save(workspace1);
const save2 = Blockly.serialization.workspaces.save(workspace2);
workspace1.clear();
workspace2.clear();
// Show people the cleared workspace so they know the reload did something.
await new Promise((resolve) => setTimeout(resolve, 500));
Blockly.serialization.workspaces.load(save1, workspace1);
Blockly.serialization.workspaces.load(save2, workspace2);
}
/** Loads the workspace in single workspace mode. */
function loadWorkspace() {
const save1 = JSON.parse(document.getElementById('save1').value);
workspace1.clear();
Blockly.serialization.workspaces.load(save1, workspace1);
}index.html:
<!doctype html>
<html>
<head>
<meta charset="utf-8" />
<title>Blockly Shareable Procedures</title>
<style>
html,
body {
margin: 0;
}
h2,
h3 {
padding: 1em;
margin: 0;
background-color: black;
color: white;
font-family: monospace;
}
#root {
display: flex;
}
#saveArea {
display: flex;
flex-direction: column;
flex-grow: 1;
height: 100vh;
}
.save {
flex-grow: 1;
background-color: black;
color: white;
resize: none;
}
.blockly {
flex-grow: 4;
height: 100vh;
}
</style>
</head>
<body>
<div id="root">
<div class="blockly" id="blockly1"></div>
<div class="blockly" id="blockly2"></div>
<div id="saveArea">
<h2>Saved State</h2>
<input id="load" type="button" value="Reload" />
<h3>Workspace 1</h3>
<textarea class="save" id="save1"></textarea>
<h3>Workspace 2</h3>
<textarea class="save" id="save2"></textarea>
</div>
</div>
<script src="../build/test_bundle.js"></script>
</body>
</html>Multiselect Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
changminbark
commented
6 days ago https://github.com/google/blockly-samples/tree/master/plugins/block-test
Tested with the multiselect plugin.
Note: The drag to dupe block is never added to the multiselection as it breaks the functionality (addressed previously in PR).
Multiselect Plugin: Everything works as intended (use cases). There are no clashes as there are no overlapping use cases.
HollowMan6
commented
1 day ago Closing as the goal has been reached, thank you @changminbark
Check for duplicates
Problem
Test the integration with other Blockly plugins, apply fixes whenever applicable (something to start with is https://groups.google.com/g/blockly/c/MuL2Ln8SwDU)
Request
To start with:
Workspace-related
Blocks-related
We might want to check with other plugins as well later if time allows
Alternatives considered
No response
Additional context
No response