respond to code review

This commit is contained in:
Christopher Willis-Ford 2019-06-17 21:00:35 -07:00
parent 18b1551cdb
commit 3c28f714bb
3 changed files with 12 additions and 10 deletions

View file

@ -213,7 +213,8 @@ A few of these considerations include:
* In particular, changing languages should never break a working project.
* The average Scratch user should be able to figure out the valid values for this input without referring to extension
documentation.
* One way to ensure this is to make an item's text match or include the item's value.
* One way to ensure this is to make an item's text match or include the item's value. For example, the official Music
extension contains menu items with names like "(1) Piano" with value 1, "(8) Cello" with value 8, and so on.
* The block should accept any value as input, even "invalid" values.
* Scratch has no concept of a runtime error!
* For a command block, sometimes the best option is to do nothing.

View file

@ -847,10 +847,10 @@ class Runtime extends EventEmitter {
for (const menuName in extensionInfo.menus) {
if (extensionInfo.menus.hasOwnProperty(menuName)) {
const menuValue = extensionInfo.menus[menuName];
const convertedMenu = this._buildMenuForScratchBlocks(menuName, menuValue, categoryInfo);
const menuInfo = extensionInfo.menus[menuName];
const convertedMenu = this._buildMenuForScratchBlocks(menuName, menuInfo, categoryInfo);
categoryInfo.menus.push(convertedMenu);
categoryInfo.menuInfo[menuName] = menuValue;
categoryInfo.menuInfo[menuName] = menuInfo;
}
}
for (const fieldTypeName in extensionInfo.customFieldTypes) {
@ -1263,8 +1263,7 @@ class Runtime extends EventEmitter {
context.inputList.push(`<shadow type="${shadowType}">`);
}
// <field> is a text field that the user can type into. Some shadows, like the color picker, don't allow
// text input and therefore don't need a field element.
// A <field> displays a dynamic value: a user-editable text field, a drop-down menu, etc.
if (fieldName) {
context.inputList.push(`<field name="${fieldName}">${defaultValue}</field>`);
}

View file

@ -312,19 +312,21 @@ class ExtensionManager {
for (let i = 0; i < menuNames.length; i++) {
const menuName = menuNames[i];
let menuInfo = menus[menuName];
// If the menu description is in short form (items only) then normalize it to general form: an object with
// its items listed in an `items` property.
if (!menuInfo.items) {
menuInfo = {
items: menuInfo
};
menus[menuName] = menuInfo;
}
// If the value is a string, it should be the name of a function in the
// extension object to call to populate the menu whenever it is opened.
// Set up the binding for the function object here so
// we can use it later when converting the menu for Scratch Blocks.
// If `items` is a string, it should be the name of a function in the extension object. Calling the
// function should return an array of items to populate the menu when it is opened.
if (typeof menuInfo.items === 'string') {
const menuItemFunctionName = menuInfo.items;
const serviceObject = dispatch.services[serviceName];
// Bind the function here so we can pass a simple item generation function to Scratch Blocks later.
menuInfo.items = this._getExtensionMenuItems.bind(this, serviceObject, menuItemFunctionName);
}
}