[Google Blockly] use 'unnamed' for functions without names#55320
Conversation
| } | ||
|
|
||
| if (fieldElement.textContent === '') { | ||
| fieldElement.textContent = 'unnamed'; |
There was a problem hiding this comment.
do we have to worry about translations or does blockly always use 'unnamed'?
There was a problem hiding this comment.
I tested this and it seems like we use 'unnamed' for every language.
There was a problem hiding this comment.
This matches our current behavior for students who rename existing functions. Did we already have a task to localize the default "do something" name as well?
There was a problem hiding this comment.
I see! I was still seeing English when I switched languages but that must just be because the translations haven't been added yet.
molly-moen
left a comment
There was a problem hiding this comment.
Looks good, couple recommendations for constants
| // Place mutator before fields, values, and other nested blocks. | ||
| blockElement.insertBefore(mutationElement, blockElement.firstChild); | ||
| if (!mutationElement.getAttribute('name')) { | ||
| mutationElement.setAttribute('name', 'unnamed'); |
There was a problem hiding this comment.
nit: add a constant for 'unnamed'
| */ | ||
| export function addNameToBlockFunctionCallBlock(blockElement) { | ||
| const blockType = blockElement.getAttribute('type'); | ||
| if (blockType !== 'procedures_callnoreturn') { |
There was a problem hiding this comment.
nit: use BLOCK_TYPES.procedureCall constant here
| */ | ||
| export function addNameToBlockFunctionDefinitionBlock(blockElement) { | ||
| const blockType = blockElement.getAttribute('type'); | ||
| if (blockType !== 'procedures_defnoreturn') { |
There was a problem hiding this comment.
use block type constant here
| "unhideAndAssignHeader": "This unit is currently hidden from the section", | ||
| "unhideUnitAndAssign": "Unhide unit and assign", | ||
| "unnamedFunction": "You have a variable or function that does not have a name. Don't forget to give everything a descriptive name.", | ||
| "unnamedFunctionName": "unnamed", |
There was a problem hiding this comment.
does Blockly set 'unnamed' as the name if it's blank (no matter the language)? If they do we may not want a translation here
There was a problem hiding this comment.
Updated to use Blockly.Msg so it should be consistent with how Blockly sets the name, no matter the language.
Fixes CDO Blockly projects that include function call blocks associated with functions with blank (
'') names.Example CDO Project:
Google Blockly Before:
Google Blockly After:
Explanation
This bug potentially affects Dance projects that:
CDO Blockly supports unnamed functions:
Google Blockly fixed unnamed functions when they are renamed by forcing a name:
If we detect that there is a function without a name, we can give the name
'unnamed'which will prevent the crash. For definition blocks, this happens in the NAME field. For call blocks, this happens in the mutator.Links
See # 1 here: https://docs.google.com/document/d/1nCJIB2UHnHt-uPjyHBR9AZxxQbaeHdz5aeBchyRlZMI/edit
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: