Handling Fields that Reference Variables

[BeksOmega]

Problem statement

Currently there is a referencesVariables function that is supposed to tell blockly whether a field references variables. But the way blockly gets the variables after it knows they are being referenced is pretty inflexible.

Blockly.Block.prototype.getVars = function() {
  var vars = [];
  for (var i = 0, input; input = this.inputList[i]; i++) {
    for (var j = 0, field; field = input.fieldRow[j]; j++) {
      if (field.referencesVariables()) {
        vars.push(field.getValue());
      }
    }
  }
  return vars;
};

This expects the value of the field to be a variable name, but this is not necessarily true of custom fields. The value could be two variable names, or a variable name and a const, really anything.

Proposals

1. Add a getReferencedVariables() function that fields could override instead of referencesVariables().
2. Remove referencesVariables(), instead add a type property to fields, and then have getVars check if the field is a variable field before calling getValue(). This means only variable fields would be allowed to reference variables.

Additional Information

N/A

[abhinavsagar]

Could I work on this ?

[BeksOmega]

Could I work on this ?

Yeah definitely! Once one of the blockly people gives a reply about which proposal they like.

[RoboErikG]

Did you have a specific use case in mind, Beka? I can think of some contrived examples but nothing's coming to mind that would be a better experience with more complex variable fields.

[BeksOmega]

The use case I had in mind was someone who had a multipart field, and wanted the parts to be able to be variable controlled.

For example, say someone has a shirt configuration field with color, size, and fabric properties. They then want to allow someone to be able to set the color part to a variable.

But honestly I don't think this should be allowed. I think variables should be handled at the block level and not the field level, allowing both just gets messy and confusing.

This is why I think option 2 should be done, because it makes it clear that only variable fields handle variables. Plus rachel mentioned here that she thought a type property would be nice.

(But if you don't think it's a big deal, the referencesVariables function can just be left undocumented, and no one will be the wiser)

[RoboErikG]

Hmm...Unless there's a specific case where this is causing confusion or preventing a use case it might be better to just clarify the docs on referencesVariables to say something like, "True if this field's value references a variable."

That APIs been out for about a year and I'm not sure there's value in changing it unless it's affecting someone.

[RoboErikG]

@abhinavsagar feel free to update the jsdoc (or let us know if it's affecting you in some way). =)