[Google Blockly] v10 bump! #55770

mikeharv · 2024-01-16T19:57:42Z

This bump our version of Google Blockly to v10.3.1, which is the most recent version!

Major release notes: https://github.com/google/blockly/releases/tag/blockly-v10.0.0

Breaking changes addressed

Some thing were renamed:

Simplest to fix were some renamed variables, such as arrow_ to arrow.
See commits bf5d0fd, f7a08d4, 7c44082, 73555d1 for details.

New API for Generators

Code generators were refactored here: google/blockly#7150
Where previously, we might have said generator.sprite_parameter_get, for a generator function, now it would be generator.forBlock.sprite_parameter_get. In a couple of places we can just patch this. Additionally, we can also assign all the properties of our JS generator to forBlock. Even though this isn't technically a breaking change, it was causing a flood of deprecation warnings, so we needed to address it. See first commit: 51af955

Updated plugins

All Blockly plugins were updated to v10 compatible versions. These were tested manually. An exception exists for @blockly/renderer-inline-row-separators which was deprecated. See below.

Inline Row Separators -> End Row Inputs

A plugin that we have been relying on (for minitoolbox blocks) was deprecated with v10.2.
https://www.npmjs.com/package/@blockly/renderer-inline-row-separators
This changes our logic slightly. The plugin allowed us to arrange multiple inline value input connectors on separate rows by inserting dummy inputs between them (ie. we could put the dummy input with a flyout field below the other inputs, as long as the row also ended with a dummy input). Now the flyout field's input needs to follow an end row input (https://developers.google.com/blockly/guides/create-custom-blocks/define-blocks#block_inputs).
This means we need to convert the last input of a block from dummy to end row type. The only difference is that end row inputs always acts like a newline. This gets complicated as these custom blocks are made by our infamous block_utils flow. As far as I can tell we only use a dummy at the end of a block so it should be safe to just always use end row inputs now - for all blocks. Again, the only difference is that they will always mark the end of a row.
CDO Blockly has no concept of end row inputs so I added a monkey patch to its wrapper. I opted for this change over the opposite because once Sprite Lab is migrated, the only CDO Blockly still using block_utils will be Minecraft.

Included Improvements

Edit buttons can show in modal function editor

https://codedotorg.atlassian.net/browse/CT-124
The shareable procedures plugin has been updated so that we're now able to edit a function in the modal editor even when it's already open. This work was drafted #55155 back in November. I included it here because it's the single biggest feature that the v10 bump was blocking. Note that the buttons to create Functions/Behaviors are hidden from the modal editor's toolbox, but they would now work correctly if added.

Flyout fields cleaned up

The v10 update also includes a new render management system. Other than that it should improve rendering efficiency, we shouldn't have to interact with it much. However, Beka and I discovered that it had a couple of bugs that were causing issues when our mini-toolbox blocks would dynamically create their flyout fields. This was be patched in Blockly version 10.3.1, which is why we are using that version in this branch.
While pairing with Beka, we noticed that some of the code, originally sourced from outside our repo, seemed to be redundant or unnecessary for our implementation. Specifically, we had some leftover logic about showing and hide flyouts. This logic isn't needed because we were never hiding the flyouts; rather we just remove the field and flyout entirely when the user clicks the toggle. A few lines that were easy to remove have been deleted here.

Links

https://codedotorg.atlassian.net/browse/CT-1
https://codedotorg.atlassian.net/browse/CT-124
https://codedotorg.atlassian.net/browse/CT-130

Testing story

Some tests were failing initially. Our block utils test had to be updated to use appendEndRowInput. We also had some UI tests failures due to changes in how block ids were created.
Blockly had made a change here: google/blockly#7432
They've ended up reverting this for 10.3.1 for us which is another reason we need to use this specific version.

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

Tests provide adequate coverage
Privacy and Security impacts have been assessed
Code is well-commented
New features are translatable or updates will not break translations
Relevant documentation has been added or updated
User impact is well-understood and desirable
Pull Request is labeled appropriately
Follow-up work items (including potential tech debt) are tracked and linked

molly-moen

Looks good, some minor comments/questions

apps/src/blockly/addons/cdoFieldImage.js

apps/src/blockly/addons/cdoFieldFlyout.js

molly-moen · 2024-01-17T23:01:09Z

apps/src/blockly/addons/functionEditor.js

@@ -101,10 +101,10 @@ export default class FunctionEditor {
    // we have to pass the main ws so that the correct procedures are populated
    // false to not show the new function button inside the modal editor
    this.editorWorkspace.registerToolboxCategoryCallback('PROCEDURE', () =>
-      functionsFlyoutCategory(Blockly.mainBlockSpace, true)


why was this changed?

+1 It looks like passing the main workspace here vs. the editor workspace was intentional: we have to pass the main ws so that the correct procedures are populated. But we did move away from the mainBlockSpace language?

We need to register this category using the editor workspace so that when we getNewFunctionButtonWithCallback, it register a button callback for the editor workspace. Otherwise, the button doesn't do anything if it's in the flyout for the modal editor. Note that we also register these category callbacks on the main workspace in the wrapper. We no longer have to look at the main workspace to find the procedures, so I'll removed that comment the Emily is quoting above.

So we don't care if the "new function"/"new behavior" button is rendered inside the editor toolbox? I see it in your screenshots and I don't think that's how it worked before (and I imagine, if we could avoid that, we'd want to?).

We need to register this category using the editor workspace so that when we getNewFunctionButtonWithCallback, it register a button callback for the editor workspace. Otherwise, the button doesn't do anything if it's in the flyout for the modal editor.

But you're saying we want this button? I thought we a) didn't want it and b) didn't want it to do anything. (Sorry for two comments, my 4:30 PM brain is dragging.)

Do you recall a reason why we wouldn't to be able to create a function/behavior from within the editor? I don't think there's a technical reason we can't, so maybe it's worth checking with Amy?

On CDO Blockly, you are right that you can't create a new function if the function editor is open (same for behaviors). Strangely, you can create a function if just the behavior editor is open (and vice versa). If you try this, things get gross, because CDO makes two modals which it isn't really meant to. :(

I'm not saying I know for sure, but I wonder if the decision to hide these buttons in the original modal editor was a technical one or a design decision.

We no longer have to look at the main workspace to find the procedures

Why is this?

@mikeharv I think it was a design decision, but I'm not sure it's one I disagree with?

@molly-moen More detailed answer in Slack, but we always populate the flyout by looking for procedure blocks on both the main and hidden workspaces. I think the comment has long been out of date.

@ebeastlake I've made a partial revert so that we again only add the "Create" buttons to the flyout if we're on the main workspace. I've left in the change that would make these buttons work correctly if we did want to use them in the modal editor's toolbox at any point.

apps/src/blockly/customBlocks/googleBlockly/mutators/functionMutatorHelpers.js

apps/src/blockly/addons/cdoRendererThrasos.js

ebeastlake · 2024-01-17T23:28:05Z

apps/src/blockly/customBlocks/googleBlockly/behaviorBlocks.js

  const blockList = [];

-  const behaviorDefinitionBlock = {


Can you clarify why this was removed? I see it's not currently used, but I don't remember when we stopped needing it.

It was used to define an empty c-shaped behavior definition block that was placed in the toolbox when the modal editor was disabled. We do this for functions, but not behaviors because behaviors are always hidden from the main workspace. I originally added this back in the summer as a way of testing our earliest behavior blocks.

apps/src/blockly/customBlocks/googleBlockly/proceduresBlocks.js

ebeastlake · 2024-01-17T23:39:05Z

apps/src/blockly/customBlocks/googleBlockly/proceduresBlocks.js

    blockList.push(newFunctionButton);
  } else {
+    // Note: Blockly.Msg was undefined when this code was extracted into global scope
+    const functionDefinitionBlock = {


nit that doesn't need to be addressed: Since this is a block "constant"/"template" that I would have extracted into a constants file if I could have, it could also have continued to live above where it's used, but this also makes sense and no need to revert!

ebeastlake · 2024-01-17T23:41:40Z

apps/src/blockly/customBlocks/googleBlockly/spritelabBlocks.js

@@ -55,7 +53,6 @@ export const blocks = {
      if (!block.getInput(INPUTS.FLYOUT)) {
        const flyoutField = createFlyoutField(block);
        flyoutField.showEditor();
-        flyoutField.render_();


Is this part of the flyout fields clean-up?

Yes! It was actually just redundant. Normally render_ should only be called by getSize.

dashboard/test/ui/features/code_tools/google_blockly/modal_function_editor.feature

molly-moen

yay v10! 🎉

* bump blockly and plugins to v10.3 * connection check was renamed * fix generator for sprite getter * imageElement was renamed * imageElement was renamed * more renames * allow procedure editing from within modal editor * deprecate inline row separate; add end row inputs * support end row input calls with cdo blockly * rename appendDummyInput * use multiline text input fields for descriptions * bump to 10.3.1 * account for taller description field * update comments * partial revert: once again hide create buttons in modal * Update functionEditor.js * update block placement in eyes test * revert multiline description fields

mikeharv added 15 commits December 14, 2023 16:14

bump blockly and plugins to v10.3

51af955

Merge branch 'staging' into mike/blockly-v10

524d388

connection check was renamed

bf5d0fd

fix generator for sprite getter

2f0fef4

imageElement was renamed

f7a08d4

imageElement was renamed

7c44082

more renames

73555d1

allow procedure editing from within modal editor

4133e4c

deprecate inline row separate; add end row inputs

097fe9c

Merge branch 'staging' into mike/blockly-v10

09cfb3f

support end row input calls with cdo blockly

7cbccba

Merge branch 'staging' into mike/blockly-v10

8c07cd2

Merge branch 'staging' into mike/blockly-v10

925be20

rename appendDummyInput

78b68e0

use multiline text input fields for descriptions

46a5076

mikeharv requested a review from a team January 17, 2024 21:02

bump to 10.3.1

e3672fe

molly-moen reviewed Jan 17, 2024

View reviewed changes

ebeastlake reviewed Jan 17, 2024

View reviewed changes

apps/src/blockly/addons/cdoRendererThrasos.js Show resolved Hide resolved

ebeastlake reviewed Jan 17, 2024

View reviewed changes

apps/src/blockly/customBlocks/googleBlockly/proceduresBlocks.js Show resolved Hide resolved

ebeastlake reviewed Jan 17, 2024

View reviewed changes

mikeharv added 4 commits January 17, 2024 18:55

account for taller description field

a270e0a

update comments

14ee12a

partial revert: once again hide create buttons in modal

c7be77d

Update functionEditor.js

8a8794e

mikeharv requested review from ebeastlake and molly-moen January 18, 2024 17:49

molly-moen reviewed Jan 18, 2024

View reviewed changes

dashboard/test/ui/features/code_tools/google_blockly/modal_function_editor.feature Outdated Show resolved Hide resolved

mikeharv added 2 commits January 18, 2024 13:24

update block placement in eyes test

403d210

revert multiline description fields

7f33b40

molly-moen approved these changes Jan 18, 2024

View reviewed changes

mikeharv merged commit e5fdf34 into staging Jan 18, 2024
2 checks passed

mikeharv deleted the mike/blockly-v10 branch January 18, 2024 21:36

bencodeorg restored the mike/blockly-v10 branch February 27, 2024 22:04

bencodeorg deleted the mike/blockly-v10 branch February 27, 2024 22:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Google Blockly] v10 bump! #55770

[Google Blockly] v10 bump! #55770

mikeharv commented Jan 16, 2024 •

edited

Loading

molly-moen left a comment

molly-moen Jan 17, 2024

ebeastlake Jan 17, 2024

mikeharv Jan 18, 2024 •

edited

Loading

ebeastlake Jan 18, 2024

ebeastlake Jan 18, 2024 •

edited by mikeharv

Loading

mikeharv Jan 18, 2024

molly-moen Jan 18, 2024 •

edited

Loading

ebeastlake Jan 18, 2024

mikeharv Jan 18, 2024

ebeastlake Jan 17, 2024

mikeharv Jan 18, 2024

ebeastlake Jan 17, 2024

ebeastlake Jan 17, 2024

mikeharv Jan 18, 2024

molly-moen left a comment

[Google Blockly] v10 bump! #55770

[Google Blockly] v10 bump! #55770

Conversation

mikeharv commented Jan 16, 2024 • edited Loading

Breaking changes addressed

Some thing were renamed:

New API for Generators

Updated plugins

Inline Row Separators -> End Row Inputs

Included Improvements

Edit buttons can show in modal function editor

Flyout fields cleaned up

Links

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Checklist:

molly-moen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikeharv Jan 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebeastlake Jan 18, 2024 • edited by mikeharv Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

molly-moen Jan 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

molly-moen left a comment

Choose a reason for hiding this comment

mikeharv commented Jan 16, 2024 •

edited

Loading

mikeharv Jan 18, 2024 •

edited

Loading

ebeastlake Jan 18, 2024 •

edited by mikeharv

Loading

molly-moen Jan 18, 2024 •

edited

Loading