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.

Sign up for GitHub

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

Jump to bottom

Update Blockly to v10 #3275

Open
wants to merge 223 commits into
base: master
Choose a base branch
from
Open

Update Blockly to v10 #3275

wants to merge 223 commits into from
+35,903 −12,513

Conversation

ewpatton
Copy link
Member

@ewpatton ewpatton commented Dec 5, 2024 •
edited
Loading

General items:

  • I have updated the relevant documentation files under docs/

  • My code follows the:

  • ant tests passes on my machine

  • I branched from master

  • My pull request has master as the base

What does this PR accomplish?

This PR updates App Inventor from Blockly circa May 2017 to Blockly v10.

Resolves #2686

ewpatton added 30 commits May 1, 2023 17:42
@ewpatton
Switch to using compiled version of Blockly as source
1940037 
Change-Id: I7e5416732f574de36c4558dfa6f90b7fb1a8f1ca
@ewpatton
Switch to using latest Blockly compressed
c16a126 
Change-Id: I661c58f1db76f20510df0f5fab6b74ed52b864c0
@ewpatton
Switch to UiBase from Ui for event super
80a4f1d 
Change-Id: I3327832eec806888294275c7b0d7adb2d848a5ac
@ewpatton
Update code to use default Blockly.inject
5087c7a 
Change-Id: I6bb65ec0a6bdb3be4d7769617646a644b02b0c9e
@ewpatton
Migrate from Blockly.Xml to Blockly.utils.xml
9a2d0b0 
Change-Id: I00e5a57e318d114f25c5f0d7763f11870e4dc4b6
@ewpatton
Remove errant call to hasOwnProperty for Blockly.Blocks
2c535a4 
Change-Id: I319d4fcce6b7971a5290c58aa7ee531a80471e35
@ewpatton
Assume indented inputs are regular value inputs for now
5741dd7 
Change-Id: Ie5784962df6276ed58c750175ba558c95cbb78f1
@ewpatton
Remove monkeypatches that may not be needed
558d64a 
Change-Id: Ie5f96d41534be8b05dd172774ce849e51e7bf273
@ewpatton
Revert rename of Blockly.Xml.domToWorkspace
ad82da0 
Change-Id: I9b393a93a604711f19f73ae6c82d5cd08ede7e25
@ewpatton
Set clearing flag when loading workspace
b1d26a4 
Change-Id: I920ad6e21cff0ab28f044f246c1583a6395ac99a
@ewpatton
Use newer technique for manipulating scrollbar visibility
8d7833a 
Change-Id: I37f93abf2aea184e43f3e41ca89ea93192eb1a42
@ewpatton
Update Error icon to ES6 class
c53e91f 
Change-Id: I8d554a9d8dc0ba427d6a0d5b5e3418138d00df15
@ewpatton
TEMP: Make aiblockly have short expiration time for coding
6e10ac3 
Change-Id: Id3fb7a482058d18584270f6b8ad1c2e4ea2d3e36
@ewpatton
Run Blockly migration script
692d421 
Change-Id: Id5fa531daa6c08fd8d38fa23dcf928f4253b07a2
@ewpatton
Rewrite some fields as classes
57876b7 
Change-Id: I52805803fede2e105525ac8f71a3ea6da9bdfc9f
@ewpatton
Switch from targetWorkspace_ to targetWorkspace
35e2986 
Change-Id: Icb041cc0376571a3715cdb99d7dd4aacec8dd5df
@ewpatton
Make workspace always rendered
f2b10cf 
Change-Id: I10dfcb9bf6bb5233e03bf1971f526239e0137bdc
@ewpatton
Update events to newer Blockly event model
9492159 
Change-Id: I7a6186862de76aee97b3239ce70dac6db22b6bac
@ewpatton
Move to using now-public APIs
950a218 
Change-Id: Id4e0bd6dfd948600be5cf276111024ecd1912d7d
@ewpatton
Fix drawer not opening
5314dd5 
Change-Id: I75554acf9a151c477ce82597d9487a87433d765b
@ewpatton
Fix errors in math blocks due to missing i18n
6fc40e5 
Change-Id: I6fe4830895e9079bfb003a3527ac291d7245e1df
@ewpatton
Retract some duplicate inject functionality
cc11216 
Change-Id: I05627b37b4ee313088d3b064dd7b4b134a4559c0
@ewpatton
Break stack overflow in component blocks
2ed11fc 
Change-Id: Ia792718e059e0a5f57eb8b2fc35d79a1c9a38db1
@ewpatton
Comment out some App Inventor monkeypatches
2398fe4 
Change-Id: I1291e958ee463455cc5f41f10918ce1fb4e87f2d
@ewpatton
Fix NPE in lexical variable dropdown change
659297d 
Change-Id: Ie94ef6fff8e11b329a0e28bc163389d163613732
@ewpatton
Break stack overflow in procedure field
bd2cec9 
Change-Id: I54ed0a6de1ba19afe91e71777aef05666a0f879d
@ewpatton
Fix dom class manipulation functions not caught by migration script
f8d90a4 
Change-Id: I7c4accf21d55fa2f0fdc7613247c5863a4b43af7
@ewpatton
Turn off renderHere/renderDown
19c599e 
Change-Id: I08192cf0ff41602649c46469c425cc06213fe520
@ewpatton
Fix more event renames
87bdd80 
Change-Id: I3c63ef268abb1e24037619cdec59b7e2be0f6f70
@ewpatton
Temporarily turn off event parameter i18n
9702233 
Change-Id: Id044cbe8eebcd4b8dca0792d269f27aacf47bf6e
@ewpatton
Copy link
Member Author

ewpatton commented Jan 6, 2025

Multiselect seems to be behaving poorly on the test server. If I select a block then hold shift to select another block, sometimes it works and sometimes it doesn't. Also, selecting the second block regardless turns off multiselect for further blocks. You can see this reflected in the state change in the UI of the multiselect button. I think the correct behavior should be that until the user releases Shift each new block should be added. It's unclear whether this is a bug in the plugin itself or some integration in this PR.

cc @HollowMan6

@ewpatton
Rename Blockly.ai_inject to AI.inject to get it out of the Blockly na…
d364945 
…mespace

Change-Id: Ib911249c5d909cd7d7f03e86a25866773f71b84b
@HollowMan6
Copy link
Contributor

HollowMan6 commented Jan 6, 2025 •
edited
Loading

Multiselect seems to be behaving poorly on the test server. If I select a block then hold shift to select another block, sometimes it works and sometimes it doesn't. Also, selecting the second block regardless turns off multiselect for further blocks. You can see this reflected in the state change in the UI of the multiselect button. I think the correct behavior should be that until the user releases Shift each new block should be added. It's unclear whether this is a bug in the plugin itself or some integration in this PR.

Haven't had time to check the modification here, but firstly, we need to confirm if you are using the latest commit that support Blockly v10 instead of the v0.1.12 one, as some bugs were fixed later and we forgot to release a new version for Blockly v10 before we merged the support for Blockly v11.

AFAIK, the plugin was working fine for Blockly v10 before we decide to proceed into v11, and the correct behavior applies to the plugin. If the situation you mentioned only happens in rare situations, it sounds somewhat similar to the transparent SVG issue mit-cml/workspace-multiselect#56, but it shouldn't cause the multislect state to go off. In any case, I would suggest you to check if you have any key shortcut overlap / focus change when you do multiselect/selecting the block, as those can all cause the multislect state to go off unexpectedly.

@ewpatton
Copy link
Member Author

ewpatton commented Jan 6, 2025

@HollowMan6 Thanks. I'll build that commit and see if it addresses the issue.

@ewpatton
Remove old block-lexical-variable plugins
4f1491c 
Change-Id: I355450fb4aca26ac0f2d81dc9e53c37c834c3d0f
@ewpatton
Copy link
Member Author

ewpatton commented Jan 7, 2025

Ok, so that wasn't it but I figured out the root cause. In our mouseDown handler code we focus on the Blockly div, which triggers the onBlur function in multiselect. I don't really remember why the focus call was needed in the last Blockly update but maybe it is safe to take it out.

@ewpatton
Update to last workspace-multiselect before Blockly v11 support
df688bf 
Change-Id: I124985896013a854cb954137558f3e7ab3cfa1d8
@ewpatton
Add caching rules for new Blockly files
d8cd1cf 
Change-Id: Idf883d7417a1460bcf88523a43cdf2bd53004722
@ewpatton
Clean up focus and other changes no longer needed with latest Blockly
d3afd79 
Change-Id: I4e053455cfb47adac6e2adbd2db836f98f6d4501
@ewpatton
Inline code given injected is always true
1bf7c6e 
Change-Id: I0dd35c4fa1329a4155a5530f867cfffd0d4bbac5
@ewpatton
Restore svgResize call in BlocklyPanel
ccba519 
Change-Id: If9fcf83c4aa753efa7e368d51b58ce4ee6b40f17
@ewpatton
Restore workspace change listener
e4f32c6 
Change-Id: I54357cd8da6ed89cb52dc509bcbda5517883211d
@ewpatton
Make isShowWhenCollapsed match semantics from Blockly 2017 update
8a01408 
Change-Id: I57b7fb734f37119ffa702d1442f0016356b29134
@ewpatton
Do not use optional chaining for older browser support
dbe545e 
Change-Id: Ida259ca6825f0646e3f5a8025179ba0e182a2788
@ewpatton
Copy link
Member Author

ewpatton commented Jan 7, 2025

Event parameter translation is broken...

@ewpatton
Copy link
Member Author

ewpatton commented Jan 7, 2025

Screenshot 2025-01-07 at 12 00 47

@ewpatton
Merge new text output handling into obfuscated_text block
78406e9 
Change-Id: Ia86f814aee167038e8a954880a8b7f59a495e5c5
@ewpatton
Fix blockly compile check
6a29f21 
Change-Id: I79414e9e644efff787095e1a5a2354b768a52558
@ewpatton
Fix event parameter internationalization
fe8cb68 
Change-Id: I03e4aa66126a1ce20ab9058fede5af44411af61f
@ewpatton ewpatton mentioned this pull request Jan 8, 2025
Open
ewpatton and others added 3 commits January 8, 2025 13:13
ewpatton
ewpatton commented Jan 8, 2025
View reviewed changes
appinventor/blocklyeditor/src/blocks/text.js Outdated
inputValidator: function (newVal) {
var block = this.getSourceBlock();
if (block.workspace.isLoading) {
Blockly.Blocks.text.setOutputOnFinishEdit.bind(block)(newVal);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it is more efficient to call .call(block, newVal) rather than creating a new bound function for every text. Also, since the functions are copied to the block object couldn't we just do this.setOutputOnFinishEdit(...)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the bind to a call. Using this doesn't work because this is bound to the field in this function and I can't use block because setOutputOnFinishEdit isn't defined on the obfuscated_text block.

I also took the opportunity to document some of the logic of using the field validator and the onchange function, since it's not immediately obvious.

ewpatton
ewpatton commented Jan 8, 2025
View reviewed changes
appinventor/blocklyeditor/src/blocks/text.js Outdated
((event.type === Blockly.Events.BLOCK_CHANGE) ||
event.type === Blockly.Events.BLOCK_CREATE)) {
var text = this.getFieldValue('TEXT');
Blockly.Blocks.text.setOutputOnFinishEdit.bind(this)(text);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment on this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my previous reply.

ewpatton
ewpatton commented Jan 8, 2025
View reviewed changes
appinventor/blocklyeditor/src/blocks/text.js Outdated
@@ -55,8 +58,8 @@ Blockly.Blocks.text.setOutputOnFinishEdit = function(newValue) {
this.outputConnection.setCheck(AI.BlockUtils.YailTypeToBlocklyType("text", AI.BlockUtils.OUTPUT));
} else {
// Remove the Number type from the output connection check if it exists.
const check = Array.from(AI.BlockUtils.YailTypeToBlocklyType("text", AI.BlockUtils.OUTPUT));
const numberIndex = check.indexOf('Number');
var check = Array.from(AI.BlockUtils.YailTypeToBlocklyType("text", AI.BlockUtils.OUTPUT));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like these could have been left as const?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've made me paranoid about using "modern" JavaScript features ;-). I'll change them back.

ewpatton
ewpatton commented Jan 8, 2025
View reviewed changes
appinventor/blocklyeditor/src/blocks/text.js Outdated
@@ -70,8 +73,8 @@ Blockly.Blocks.text.setOutputOnFinishEdit = function(newValue) {
* @block Blockly.Block
*/
function maybeBumpBlockOnFinishEdit(block) {
const outputConnection = block.outputConnection;
const targetConnection = outputConnection.targetConnection;
var outputConnection = block.outputConnection;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

ewpatton
ewpatton commented Jan 8, 2025
View reviewed changes
appinventor/blocklyeditor/src/blocks/text.js
@@ -609,7 +612,7 @@ Blockly.Blocks['obfuscated_text'] = {
this.setColour(Blockly.TEXT_CATEGORY_HUE);
var label = Blockly.Msg.LANG_TEXT_TEXT_OBFUSCATE + " " +
Blockly.Msg.LANG_TEXT_TEXT_LEFT_QUOTE
var textInput = new Blockly.FieldTextInput('');
var textInput = new Blockly.FieldTextInput('', Blockly.Blocks['text'].inputValidator);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to adopt the logic of onchange from the text block?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did. See the definition of onchange around line 634.

I also realized that I can remove the call to onFinishEditing_ as I did for the text block.

@mark-friedman
Made changes to code review comments.
bc21b91 
Also added some comments clarifying the logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mark-friedman mark-friedman mark-friedman left review comments

Assignees
No one assigned
Labels
affects: master pull request: needs review technical debt
Projects
None yet
Milestone
nb200
Development

Successfully merging this pull request may close these issues.

Update to latest Blockly with plugin support
4 participants
@ewpatton @HollowMan6 @mark-friedman @BeksOmega