fix: improve prompting when deleting variables (#8529)

* fix: improve variable deletion behaviors.

* fix: don't prompt about deletion of only 1 variable block when triggered programmatically.

* fix: include the triggering block in the count of referencing blocks

* fix: only count the triggering block as a referencing block if it's not in the flyout

blocks/variables.ts

@@ -165,11 +165,12 @@ const deleteOptionCallbackFactory = function (
   block: VariableBlock,
 ): () => void {
   return function () {
-    const workspace = block.workspace;
     const variableField = block.getField('VAR') as FieldVariable;
-    const variable = variableField.getVariable()!;
-    workspace.deleteVariableById(variable.getId());
-    (workspace as WorkspaceSvg).refreshToolboxSelection();
+    const variable = variableField.getVariable();
+    if (variable) {
+      Variables.deleteVariable(variable.getWorkspace(), variable, block);
+    }
+    (block.workspace as WorkspaceSvg).refreshToolboxSelection();
   };
 };
 

blocks/variables_dynamic.ts

@@ -176,11 +176,12 @@ const renameOptionCallbackFactory = function (block: VariableBlock) {
  */
 const deleteOptionCallbackFactory = function (block: VariableBlock) {
   return function () {
-    const workspace = block.workspace;
     const variableField = block.getField('VAR') as FieldVariable;
-    const variable = variableField.getVariable()!;
-    workspace.deleteVariableById(variable.getId());
-    (workspace as WorkspaceSvg).refreshToolboxSelection();
+    const variable = variableField.getVariable();
+    if (variable) {
+      Variables.deleteVariable(variable.getWorkspace(), variable, block);
+    }
+    (block.workspace as WorkspaceSvg).refreshToolboxSelection();
   };
 };
 

core/field_variable.ts

@@ -27,6 +27,7 @@ import * as fieldRegistry from './field_registry.js';
 import * as internalConstants from './internal_constants.js';
 import type {Menu} from './menu.js';
 import type {MenuItem} from './menuitem.js';
+import {WorkspaceSvg} from './workspace_svg.js';
 import {Msg} from './msg.js';
 import * as parsing from './utils/parsing.js';
 import {Size} from './utils/size.js';
@@ -514,7 +515,11 @@ export class FieldVariable extends FieldDropdown {
         return;
       } else if (id === internalConstants.DELETE_VARIABLE_ID && this.variable) {
         // Delete variable.
-        this.sourceBlock_.workspace.deleteVariableById(this.variable.getId());
+        const workspace = this.variable.getWorkspace();
+        Variables.deleteVariable(workspace, this.variable, this.sourceBlock_);
+        if (workspace instanceof WorkspaceSvg) {
+          workspace.refreshToolboxSelection();
+        }
         return;
       }
     }

core/flyout_button.ts

@@ -179,7 +179,7 @@ export class FlyoutButton implements IASTNodeLocationSvg {
       fontWeight,
       fontFamily,
     );
-    this.height = fontMetrics.height;
+    this.height = this.height || fontMetrics.height;
 
     if (!this.isFlyoutLabel) {
       this.width += 2 * FlyoutButton.TEXT_MARGIN_X;

core/variables.ts

@@ -714,15 +714,20 @@ export function getVariableUsesById(workspace: Workspace, id: string): Block[] {
  *
  * @param workspace The workspace from which to delete the variable.
  * @param variable The variable to delete.
+ * @param triggeringBlock The block from which this deletion was triggered, if
+ *     any. Used to exclude it from checking and warning about blocks
+ *     referencing the variable being deleted.
  */
 export function deleteVariable(
   workspace: Workspace,
   variable: IVariableModel<IVariableState>,
+  triggeringBlock?: Block,
 ) {
   // Check whether this variable is a function parameter before deleting.
   const variableName = variable.getName();
   const uses = getVariableUsesById(workspace, variable.getId());
-  for (let i = 0, block; (block = uses[i]); i++) {
+  for (let i = uses.length - 1; i >= 0; i--) {
+    const block = uses[i];
     if (
       block.type === 'procedures_defnoreturn' ||
       block.type === 'procedures_defreturn'
@@ -734,12 +739,15 @@ export function deleteVariable(
       dialog.alert(deleteText);
       return;
     }
+    if (block === triggeringBlock) {
+      uses.splice(i, 1);
+    }
   }
 
-  if (uses.length > 1) {
+  if ((triggeringBlock && uses.length) || uses.length > 1) {
     // Confirm before deleting multiple blocks.
     const confirmText = Msg['DELETE_VARIABLE_CONFIRMATION']
-      .replace('%1', String(uses.length))
+      .replace('%1', String(uses.length + (triggeringBlock ? 1 : 0)))
       .replace('%2', variableName);
     dialog.confirm(confirmText, (ok) => {
       if (ok && variable) {
@@ -747,7 +755,9 @@ export function deleteVariable(
       }
     });
   } else {
-    // No confirmation necessary for a single block.
+    // No confirmation necessary when the block that triggered the deletion is
+    // the only block referencing this variable or if only one block referencing
+    // this variable exists and the deletion was triggered programmatically.
     workspace.getVariableMap().deleteVariable(variable);
   }
 }