search

mit-cml / workspace-multiselect

A Blockly plugin that allows you to drag, select and manipulate multiple blocks in the workspace.
https://hollowman6.github.io/workspace-multiselect/multi-workspace
11 stars 12 forks source link

multiFieldUpdate shouldn't cause excessive field updates #58

Closed mark-friedman closed 4 months ago

mark-friedman commented 4 months ago

Check for duplicates

Description

I noticed that if I have only one block selected and I simply change a field value while in multiFieldUpdate mode that the validator for that field runs twice - once directly for the user-initiated change and again as the eventListener_ handler in the plugin calls setFieldValue when it gets the change event. Also note that there appears to be a cascade of many calls to setFieldValue if multiple blocks are selected. Since validators may have side effects, this is undesirable behavior.

Reproduction steps

Stack trace

No response

Screenshots

No response

Browsers

No response

HollowMan6 commented 4 months ago

I noticed that if I have only one block selected and I simply change a field value while in multiFieldUpdate mode that the validator for that field runs twice - once directly for the user-initiated change and again as the eventListener_ handler in the plugin calls setFieldValue when it gets the change event.

I guess this can be avoided by checking the event source and avoid calling the setFieldValue if that specific block happens to be the event source. I will implement this change.

Also note that there appears to be a cascade of many calls to setFieldValue if multiple blocks are selected. Since validators may have side effects, this is undesirable behavior.

This is interesting, currently we just blindly listen to the field change event generated from Blockly, so if the above change doesn't get this fixed, I'm not aware of any more ways to stop this from happening at our plugin side, and then I guess something might need to be done for the validators themselves / at the Blockly side to get this properly fixed.

mark-friedman commented 4 months ago

I'll check on the cascading and will reopen this if there remains an issue.

mark-friedman commented 4 months ago

Unfortunately, the cascading continues, despite the latest change. I don't think that it's reasonable for devs to have to adapt their validators, but if we need something from Blockly we can certainly ask them. Maybe @ewpatton has some ideas.

In the meantime, I think that we'll have to turn off multiFieldUpdate for App Inventor until this is resolved.

BTW, it's fairly easy to reproduce this by:

  1. Run npm start in the multiselect plugin directory
  2. In the resulting playground, make a few copies of a block (I used "math_arithmetic")
  3. Setting a breakpoint at line 264 of multiselect.js
  4. Select all the copies
  5. Change a field (e.g. the operation dropdown in "math_arithmetic")
HollowMan6 commented 4 months ago

Fixed the cascading issue by checking if the group id is empty, since all the automated operations have a group id.