Convert workspace.resizesEnabled_ into a counter

[rachel-fenichel]

Is your feature request related to a problem? Please describe.

Right now it's a boolean, which limits its usefulness since it is set and unset in multiple places.

Describe the solution you'd like

Change it from a boolean to a counter, following the model we use for events.

[adroitwhiz]

I got started on this, and in the process uncovered two places in the codebase where resizes are disabled and never re-enabled, both in flyout_base.js: in createBlock and in placeNewBlock_ (a search for terminateDrag reveals nothing other than that one comment).

[rachel-fenichel]

createBlock is called in Blockly.Gesture.updateIsDraggingFromFlyout_. That's called in updateIsDraggingBlock_, and if it returned true, startDraggingBlock_ is called.

terminateDrag is probably referring to Blockly.BlockDragger.prototype.endBlockDrag, which does re-enable resizes.

Having the disable in both createBlock and placeNewBlock_ does seem wrong, since placeNewBlock_ is only called from createBlock.

`createBlock is also called in one other place, Blockly.navigation.insertFromFlyout, and that one is definitely not balanced correctly.

With that extra information, how do you suggest balancing enable and disable calls here?

[adroitwhiz]

I'm iffy on the idea of tracking a resizes-enabled counter across functions-- as far as I can tell, all Blockly.Events.disable() and Blockly.Events.enable() pairs (the existing usages of a counter-based system) occur within the same function.

Already, many calls to disable/enable events are wrapped in try-finally blocks to prevent events from being permanently disabled just in case any exceptions are thrown.

I'm not sure why "weird jumps" would occur during domToBlock--possibly because the parsed block starts out at (0, 0)? If that's the case, then resizes could be re-enabled after the moveBy call, but that's just a guess.

[rachel-fenichel]

I'm not sure why "weird jumps" would occur during domToBlock--possibly because the parsed block starts out at (0, 0)? If that's the case, then resizes could be re-enabled after the moveBy call, but that's just a guess.

Yup, because if the bounding box of the blocks on the workspace doesn't include 0,0, placing the block at 0,0 causes a change in the scrollbars.

Already, many calls to disable/enable events are wrapped in try-finally blocks to prevent events from being permanently disabled just in case any exceptions are thrown.

Yes, and that's a good practice for this as well. Especially if there's serialization involved.

I'm iffy on the idea of tracking a resizes-enabled counter across functions-- as far as I can tell, all Blockly.Events.disable() and Blockly.Events.enable() pairs (the existing usages of a counter-based system) occur within the same function.

The big difference here is that the gesture code is event-driven, and so it's not all happening in a single function. Blocking workspace resizes during a drag is important, because insertion markers would otherwise cause resizes.

But we do have some guarantees about the lifecycle of a gesture, and so at minimum I'm a fan of pulling lots of these calls up into either the gesture or block dragger, and making sure that paired calls are in the same class.