[Google Blockly] Add support for Behavior blocks by mikeharv · Pull Request #53154 · code-dot-org/code-dot-org

mikeharv · 2023-08-03T21:32:30Z

This builds on the work to implement Blockly's shareable procedures blocks plugin. This adds support for behavior blocks. Last week, we merged support for function blocks, which were a bit more straightforward:

[Google Blockly] Shareable procedures blocks #52932

Behavior blocks have some additional peculiarities.

Note: The current Blockly labs are Dance, Poetry, Flappy, and Bounce. Since behaviors are not used in any of these labs, most of the changes in this PR are not discoverable unless you use the ?blocklyVersion=google URL param to force a non-Blockly lab (e.g., Sprite Lab) to use Google Blockly.

Expand here for more information about how behavior blocks work as of this PR

The screenshots below compare the behavior in Sprite Lab on studio.code.org using CDO Blockly to Sprite Lab on localhost using Google Blockly. Note: The following screenshots assume that useModalFunctionEditor is true (the default for Sprite Lab projects) but that the modalFunctionEditor experiment is disabled. There are two control levels because Dance Party levels use functions, and some Dance Party levels are configured to have useModalFunctionEditor set to true. This means that, on the function side, we could expose the unfinished function editor on any level that accidentally has that flag set incorrectly. There were too many levels with this flag set to true to change this on the level-file side, so instead, Molly added an experiment flag so that useModalFunctionEditor has to be true and the experiment has to be enabled for the editor to be rendered. The following tables describe the interactions of those two flags and their effect on functionality.

For Functions

`useModalFunctionEditor` flag	`modalFunctionEditor` experiment	Result
T	Enabled	“Create New Function” button opens a working but unfinished modal editor; the edit button appears, and functions can be edited through the modal
T	Disabled	“Create New Function” button opens the function definition block in the main workspace; the edit button appears, and clicking it centers on the relevant function definition block
F	Enabled	Blank function definition block appears in the functions category in the toolbox; there is no edit button, but the function can be renamed and edited in the main workspace
F	Disabled	Blank function definition block appears in the functions category in the toolbox; there is no edit button, but the function can be renamed and edited in the main workspace

This PR does not cover enabling the “behavior editor” (i.e., the function editor for behaviors). Therefore, the following behavior is expected:

For Behaviors

#	`useModalFunctionEditor` flag	`modalFunctionEditor` experiment	Result
1	T	Enabled	“Create a Behavior” button creates a behavior definition block in the main workspace; edit button appears on the call block and clicking it opens the modal function editor. The modal function editor appears to work as expected for functions, but unnamed blocks might still occasionally appear. Behavior definitions appear and are edited on the main workspace because moving them to the hidden workspace has not been implemented yet.
2	T	Disabled	“Create a Behavior” button creates a behavior definition block in the main workspace; the edit button appears, and clicking it centers on the relevant behavior definition block. Behavior definitions appear and are edited on the main workspace because moving them to the hidden workspace has not been implemented yet.
3	F	Enabled	Behavior definitions appear on the main workspace because moving them to the hidden workspace has not been implemented yet.
4	F	Disabled	Behavior definitions appear on the main workspace because moving them to the hidden workspace has not been implemented yet.

The screenshots in the "Testing" section focus specifically on case 2 above.

Links

Jira ticket: https://codedotorg.atlassian.net/browse/SL-641

Testing story

Before

In the previous Blockly version, behavior definition blocks were invisible on the main workspace.

After

Mainline/Google Blockly no longer supports invisible blocks. For now, the definition blocks for default behaviors (wobbling, growing, etc.) will be displayed on the main workspace, where they can also be edited. A follow-up PR will add these to the hidden definition workspace. At that point, the behavior definition blocks will no longer be visible here.

Before

After

Some of the styling (ex. gray “Create a Behavior” button) still needs to be updated, but the current Behavior category toolbox has parity with the CDO toolbox. (Note: Even though the edit buttons do not currently appear on the call blocks while they are in the toolbox, they appear once dragged to the main workspace.)

Before

After

Clicking the “Create a Behavior” button creates a new, editable behavior definition block on the main workspace. The corresponding call block is added to the toolbox.

Testing was done in English, Spanish, and Arabic. Reach out to @ebeastlake for a spreadsheet that documents manual testing.

Follow-up work

See: https://codedotorg.atlassian.net/browse/CT-13

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

This reverts commit 4e206dd.

was breaking unit tests

molly-moen · 2023-08-18T19:05:09Z

  },
+
+  installBehaviorBlocks(behaviorEditor) {
+    const generator = Blockly.getGenerator();


Can we extract this logic out into another file? It's a bunch of code and this file will be more readable if it doesn't contains super-specific logic.

Which logic do you think it makes the most sense to move? The code for installBehaviorBlocks?

yeah we could just pull all the code out (and pass in a generator?) or pull out functions/objects that get assigned to various fields on generator. Basically I want to avoid making this another mega-file if we can avoid it, but I don't have a super strong opinion on the specific refactoring here.

Assuming you're okay with tracking this as a follow-up, I added it here: https://codedotorg.atlassian.net/browse/CT-13

* remove unused import * remove unnecessary whitespace diff * expand definition in function names and convert to arrow function * refactor to use partitionBlocksByType * add comment to behaviorDefMutator * remove unused function * fix comment that indicates returning xml * add missing argument * fix flyout category callbacks * remove console log * add id to call block and sort by name * remove console.logs * convert to arrow function * update comment

ebeastlake · 2023-08-29T20:40:06Z

@maribethb My edits are in if you'd like to give this a once-over! I understand it's a large PR with lots of Code.org-specific logic, so your relevant parts are limited.

molly-moen

A couple nits/questions for my information but overall this looks good!

molly-moen · 2023-08-30T15:55:57Z


+// In Lab2, the level properties are in Redux, not appOptions. To make this work in Lab2,
+// we would need to send that property from the backend and save it in lab2Redux.
+const useModalFunctionEditor = window.appOptions?.level?.useModalFunctionEditor;


we can use Blockly.useModalFunctionEditor once this gets merged (can be done as a follow up)

Tracking! https://codedotorg.atlassian.net/browse/CT-13

molly-moen · 2023-08-30T16:05:27Z

+
+// In Lab2, the level properties are in Redux, not appOptions. To make this work in Lab2,
+// we would need to send that property from the backend and save it in lab2Redux.
+const useModalFunctionEditor = window.appOptions?.level?.useModalFunctionEditor;


should we also incorporate the experiment flag here?

I'm not sure we need it here until the behavior editor exists. I think the implications are:

If the modal function editor is enabled for the level, but the experiment isn't on, the names of the behaviors aren't editable anyway (doesn't seem like a big deal?). Option: We could require both the modal function editor to be enabled and the experiment to be on to keep the names non-editable.

{ type: useModalFunctionEditor ? 'field_label' : 'field_input', name: 'NAME', text: '', spellcheck: false, },

If the modal function editor is enabled for a level, we don't render the SVG frame for the behavior definition blocks, even though those definition blocks get defined on the main workspace until the editor works. Option: We could render the SVG frame for the behavior definition blocks even when the modal function editor is enabled as long as the experiment is off.

if (!useModalFunctionEditor && !this.workspace.noFunctionBlockFrame) { this.functionalSvg_ = new BlockSvgFrame( this, msg.behaviorEditorHeader(), 'blocklyFunctionalFrame' ); this.setOnChange(function () { if (!this.isInFlyout) { this.functionalSvg_.render(this.svgGroup_, this.RTL); } }); }

Because there's no alternate callback to open the behavior editor yet, the code below doesn't need to be modified.

} else if (useModalFunctionEditor) { workspace.registerButtonCallback('createNewBehavior', createNewBehavior); blockList.push(newBehaviorButton);

Thoughts?

that all seems fine!

molly-moen · 2023-08-30T16:10:36Z

+        lowestBlockBottom = blockBottom;
+      }
+    });
+  return lowestBlockBottom + 16;


why do we add 16 here? Can you add a comment explaining this?

My understanding is this is an arbitrary offset for when we add a new definition block to the bottom of of the blockspace. I added a constant BLOCK_OFFSET at the top of the file since we use 16 both for x and y in createNewDefinitionBlock, so it's now:

const newDefinitionBlock = Blockly.serialization.blocks.append( {...blockState, x: BLOCK_OFFSET, y: getLowestBlockBottomY() + BLOCK_OFFSET}, Blockly.getMainWorkspace() );

It's worth noting that eventually, this'll get removed.

molly-moen · 2023-08-30T16:11:46Z

+        block.getFieldValue('NAME'),
+        Blockly.Names.NameType.PROCEDURE
+      );
+      let xfix1 = '';


I'm not sure what xfix1 is. Can you add a comment and/or update the variable name?

same for xfix2 and branch

Tried my best 😅

molly-moen · 2023-08-30T16:12:56Z

+        this.getFieldValue('NAME'),
+        'PROCEDURE'
+      );
+      return [`new Behavior(${name}, [])`, generator.ORDER_ATOMIC];


(can be a follow up) should new Behavior be translated?

Added to https://codedotorg.atlassian.net/browse/CT-13!

I don't know that we can just start translating this since it's the student's JavaScript code. If we translated new Behavior, the JS interpreter probably wouldn't know how to handle it.

molly-moen · 2023-08-30T16:14:23Z

  };

+  const originalSetOutput = blocklyWrapper.Block.prototype.setOutput;
+  blocklyWrapper.Block.prototype.setOutput = function (isOutput, check) {


what is the purpose of this?

Blockly has a setOutput method, in which null specifies any type could be returned: https://developers.google.com/blockly/reference/js/blockly.block_class.setoutput_1_method

We have a "None" type specified in our constants, which we seem to have used to mean the same thing (any/a variable type as output):

code-dot-org/apps/src/blocklyAddons/cdoConstants.js

Line 9 in 9d63cbc

NONE: 'None', // Typically as a connection/input check means "accepts any type"

So, this code replaces the original setOutput method with a custom version that will handle the case when "None" is passed appropriately. That's my guess, anyway 😅 Thoughts?

seems right! Maybe worth adding a comment?

molly-moen · 2023-08-30T16:18:29Z

+export const nameComparator = (a, b) => {
+  const nameA = a.name.toUpperCase(); // ignore upper and lowercase
+  const nameB = b.name.toUpperCase(); // ignore upper and lowercase
+  if (nameA < nameB) {


I didn't know you could do this with strings!

Yeah! For reference: https://javascript.info/comparison#string-comparison

…s. labels) unless experiment is turned on

…nto local scope

Mike Harvey added 30 commits July 20, 2023 17:07

add support for color overrides for button fields

4434697

add shareable procedure block definitions

621866f

register custom procedure blocks

f2ab4e3

enabled modal function editor on google blockly levels

0408339

register shareable procedure blocks in music lab, no svg frames

ba974c9

bump to 1.2.10

cb806fd

add serializer to wrapper

2bad511

sort definitions to front of block lists

05e28c8

copy block array before sorting

dfbd8fe

re-sort blocks before positioning

4e206dd

Revert "re-sort blocks before positioning"

d52b1cd

This reverts commit 4e206dd.

reorder blocks before positioning

425f4f8

add parameter to sort function

8c082f5

reorganize functions

e325223

add comments

3ee4090

remove unnecessary unregistering

9249881

revert remove unregister

3140ea6

was breaking unit tests

add unit tests

4afe58c

rename procedureDefinitionTypes

3176204

specify any type for value option in button fields

57513c6

typo

6e1da5e

no-op for blocks without any inputs

796a88c

replace sort logic with partitioning

4313d91

remove extra copy

d41999b

check point: functions and behaviors can be created from toolbox

2728fa9

Merge branch 'staging' into mike/behavior-blocks

965f4e5

checkpoint: behaviors work with 'this sprite' parameter

4675673

Merge branch 'staging' into mike/behavior-blocks

2c4ca30

removed old unit tests

06ef2fe

add color to sprite parameter block

1966944

ebeastlake reviewed Aug 16, 2023

View reviewed changes

Comment thread apps/src/blockly/customBlocks/googleBlockly/behaviorBlocks.js Outdated

molly-moen reviewed Aug 18, 2023

View reviewed changes

ebeastlake reviewed Aug 23, 2023

View reviewed changes

Comment thread apps/src/blockly/customBlocks/googleBlockly/mutators/behaviorDefMutator.js Outdated

ebeastlake mentioned this pull request Aug 23, 2023

Extract partitionBlocksByType into dedicated util function #53408

Merged

8 tasks

ebeastlake reviewed Aug 23, 2023

View reviewed changes

Comment thread apps/src/blockly/customBlocks/googleBlockly/proceduresBlocks.js Outdated

fix linting for educate_csd file

f0114af

ebeastlake reviewed Aug 25, 2023

View reviewed changes

Comment thread pegasus/sites.v3/code.org/public/js/page/educate_csd.js

ebeastlake added 2 commits August 28, 2023 10:24

resolve merge conflicts

ec4a8c5

molly-moen approved these changes Aug 30, 2023

View reviewed changes

ebeastlake added 14 commits August 30, 2023 10:51

add constant for block offset

16ed194

update comparator comment

81d6dce

Merge branch 'staging' into mike/behavior-blocks

3f0244e

update ui tests [test all browsers]

7186480

add setOutput comment

a9564e5

add generator comments

bf85072

add missed ui test update [test all browsers]

9b85243

update ui test

eeab356

dedicated getNewFunctionButtonWithCallback and render field inputs (v…

f26c934

…s. labels) unless experiment is turned on

add behaviors category callback for modal function editor

7a86711

update ui test for chrome

8868323

update blocklayoutjson ui test

b0f7026

update whenCollideGround block position in blocklayoutjson tests

65bad54

fix regression of unnamed functions by moving block definition back i…

b63843b

…nto local scope

ebeastlake merged commit d49ca37 into staging Sep 5, 2023

ebeastlake deleted the mike/behavior-blocks branch September 5, 2023 21:33

This was referenced Sep 6, 2023

[Blockly] Run blocklayout tests on chrome only #53627

Merged

[Blockly] Reenable blocklayout UI tests #53645

Merged

mikeharv mentioned this pull request Nov 27, 2023

[Google Blockly] Modal function editor - allow edit/create buttons when modal is open #55155

Closed

8 tasks

Uh oh!

Conversation

mikeharv commented Aug 3, 2023 • edited by ebeastlake Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Links

Testing story

Follow-up work

PR Checklist:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ebeastlake commented Aug 29, 2023

Uh oh!

molly-moen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ebeastlake Aug 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

mikeharv commented Aug 3, 2023 •

edited by ebeastlake

Loading

ebeastlake Aug 30, 2023 •

edited

Loading