Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: caller args disconnecting shareable procedures #1918

Merged
merged 3 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion plugins/block-shareable-procedures/src/blocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1003,13 +1003,29 @@ const procedureCallerUpdateShapeMixin = {
* data model.
*/
updateParameters_: function() {
this.syncArgsMap_();
this.deleteAllArgInputs_();
this.addParametersLabel__();
this.createArgInputs_();
this.reattachBlocks_();
this.prevParams_ = [...this.getProcedureModel().getParameters()];
},

/**
* Makes sure that if we are updating the parameters before any move events
* have happened, the args map records the current state of the block. Does
* not remove entries from the array, since blocks can be disconnected
* temporarily during mutation (which triggers this method).
*/
syncArgsMap_: function() {
// We look at the prevParams array because the current state of the block
// matches the old params, not the new params state.
for (const [i, p] of this.prevParams_.entries()) {
const target = this.getInputTargetBlock(`ARG${i}`);
if (target) this.argsMap_.set(p.getId(), target);
}
},

/**
* Saves a map of parameter IDs to target blocks attached to the inputs
* of this caller block.
Expand Down Expand Up @@ -1112,7 +1128,7 @@ const procedureCallerOnChangeMixin = {
if (this.disposed || this.workspace.isFlyout) return;
// Blockly.Events not generated by user. Skip handling.
if (!event.recordUndo) return;
if (event.type === Blockly.Events.BLOCK_MOVE) this.updateArgsMap_();
if (event.type === Blockly.Events.BLOCK_MOVE) this.updateArgsMap_(true);
if (event.type !== Blockly.Events.BLOCK_CREATE) return;
if (event.blockId !== this.id && event.ids.indexOf(this.id) === -1) return;
// We already found our model, which means we don't need to create a block.
Expand Down
34 changes: 33 additions & 1 deletion plugins/block-shareable-procedures/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {toolboxCategories} from '@blockly/dev-tools';
import {blocks, unregisterProcedureBlocks, registerProcedureSerializer} from '../src/index';
import {ProcedureBase} from '../src/events_procedure_base';

/* eslint no-unused-vars: "off" */

unregisterProcedureBlocks();
Blockly.common.defineBlocks(blocks);
Expand All @@ -22,6 +23,13 @@ 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,
};
Expand All @@ -41,7 +49,24 @@ document.addEventListener('DOMContentLoaded', function() {

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
Expand Down Expand Up @@ -101,3 +126,10 @@ async function reloadWorkspaces() {
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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ suite('Procedures', function() {
.callsFake((callback) => {
callback();
});
window.cancelAnimationFrame = this.sandbox.stub();
});

teardown(function() {
Expand Down
Loading