-
Notifications
You must be signed in to change notification settings - Fork 483
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
[Google Blockly] fixes for flappy validation #36331
Conversation
apps/src/required_block_utils.js
Outdated
@@ -1,5 +1,6 @@ | |||
/* global Text */ | |||
|
|||
var experiments = require('@cdo/apps/util/experiments'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we switch based on which wrapper we are using, rather than which experiment?
apps/src/required_block_utils.js
Outdated
// Google's Blockly has slightly different tag names, but our validation code | ||
// expects all the tag names to be consistent with our Blockly. This mapping | ||
// ensures we will validate correctly in both versions of Blockly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be consistent with naming old & new Blockly..
blocklyWrapper.Xml.originalBlockToDom = blocklyWrapper.Xml.blockToDom; | ||
blocklyWrapper.Xml.blockToDom = function(block, ignoreChildBlocks) { | ||
const blockXml = blocklyWrapper.Xml.originalBlockToDom(block); | ||
if (ignoreChildBlocks) { | ||
Blockly.Xml.deleteNext(blockXml); | ||
} | ||
return blockXml; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat trick. Might be worth calling out with a comment..
Fixes two issues that were preventing validation from working properly:
<title>
elements, while in Google Blockly they're in<field>
elements.ignoreChildBlocks
forBlockly.Xml.blockToDom
does not exist in Google Blockly.With these fixes, I can complete the Flappy HOC:
Links
Testing story
Reviewer Checklist: