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 interacts poorly with dependent fields #57

Closed mark-friedman closed 4 months ago

mark-friedman commented 4 months ago

Check for duplicates

Description

The implementation of the multiFieldUpdate feature has surprising effects when multiple fields on the same block are dependent on one another. Consider something like App Inventor's 'math_number_radix' block. It has a radix dropdown field and a number text field. When a radix value is changed the validator on the radix dropdown will cause a change to the number value (i.e. to conform to the new radix).

If I have have multiple such blocks selected, potentially with different radix and number values, I would expect that changing the value of the radix on one block would just change all the blocks to have that radix, and that the number values on each block would change to match its proper value with the new radix, and would be different for each block if the original values were different.. However, with the current implementation all the blocks would end up with the same value. I think (at least part of) the problem is that initiated changes (like the dropdown selection) need to be treated differently than the programmatic changes to dependent fields caused by the validator.

Reproduction steps

Stack trace

No response

Screenshots

No response

Browsers

No response

HollowMan6 commented 4 months ago

I think (at least part of) the problem is that initiated changes (like the dropdown selection) need to be treated differently than the programmatic changes to dependent fields caused by the validator.

Currently we just blindly listen to the field change event generated from Blockly. I'm not aware of any way to distinguish between the user initiated changes and validator programmatic changes since they share the same event type at the Blockly side. Maybe something can be done at the Blockly side to add more granular level of event types for this to work.

mark-friedman commented 4 months ago

If we can't figure this out then I would suggest making the default for multiFieldUpdate be false and documenting the potential for problems if the developer turns it on.

HollowMan6 commented 4 months ago

Although we share the same event type at the Blockly side for field updates, I just found that user initiated changes usually has an empty group id, so this can be distinguished if the validator set the group id when they do the validation related changes. Also this avoid the cascading related issue since all the automated operations have a group id.

mark-friedman commented 4 months ago

I don't think that it is all reasonable for the user of the plugin to have to change their validators in order to incorporate the plugin. How would they even know that they need to do it? Isn't it a goal of the plugin for it to be pretty much just plug and play?

HollowMan6 commented 4 months ago

I don't think we have any other choices... Let me know if you have any better ideas.

mark-friedman commented 4 months ago

I don't think we have any other choices... Let me know if you have any better ideas.

Ok, I've come the belief that the multiselect plugin cannot by itself handle the problem with blocks that have dependent fields. I'll send a PR out soon that creates a wrapper function in the plugin that the plugin user can call to bypass the multi-field update when setting dependent fields. It essentially just sets and restores the event group around the code.

mark-friedman commented 4 months ago

@HollowMan6 See PR #61