feat: use new intermediate block change event for field edits, normal block change event for closing editor. #7105 (#7151)

* Copy core/events/events_block_change.ts to core/events/events_block_field_intermediate_change.ts

* New intermediate event type for field edits.

* Addressing PR feedback.

* Ran npm run format.

* Fixed procedure mutator responding to param edits.

* Intermediate events now inherit from BlockBase.

* Addressing feedback on PR.

* chore: format

core/events/events.ts

@@ -13,6 +13,10 @@ import {BlockChange, BlockChangeJson} from './events_block_change.js';
 import {BlockCreate, BlockCreateJson} from './events_block_create.js';
 import {BlockDelete, BlockDeleteJson} from './events_block_delete.js';
 import {BlockDrag, BlockDragJson} from './events_block_drag.js';
+import {
+  BlockFieldIntermediateChange,
+  BlockFieldIntermediateChangeJson,
+} from './events_block_field_intermediate_change.js';
 import {BlockMove, BlockMoveJson} from './events_block_move.js';
 import {BubbleOpen, BubbleOpenJson, BubbleType} from './events_bubble_open.js';
 import {Click, ClickJson, ClickTarget} from './events_click.js';
@@ -54,6 +58,8 @@ export {BlockDelete};
 export {BlockDeleteJson};
 export {BlockDrag};
 export {BlockDragJson};
+export {BlockFieldIntermediateChange};
+export {BlockFieldIntermediateChangeJson};
 export {BlockMove};
 export {BlockMoveJson};
 export {Click};
@@ -97,6 +103,8 @@ export const BLOCK_CREATE = eventUtils.BLOCK_CREATE;
 export const BLOCK_DELETE = eventUtils.BLOCK_DELETE;
 export const BLOCK_DRAG = eventUtils.BLOCK_DRAG;
 export const BLOCK_MOVE = eventUtils.BLOCK_MOVE;
+export const BLOCK_FIELD_INTERMEDIATE_CHANGE =
+  eventUtils.BLOCK_FIELD_INTERMEDIATE_CHANGE;
 export const BUBBLE_OPEN = eventUtils.BUBBLE_OPEN;
 export type BumpEvent = eventUtils.BumpEvent;
 export const BUMP_EVENTS = eventUtils.BUMP_EVENTS;

core/events/events_block_field_intermediate_change.ts

@@ -0,0 +1,138 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/**
+ * Class for an event representing an intermediate change to a block's field's
+ * value.
+ *
+ * @class
+ */
+import * as goog from '../../closure/goog/goog.js';
+goog.declareModuleId('Blockly.Events.BlockFieldIntermediateChange');
+
+import type {Block} from '../block.js';
+import * as registry from '../registry.js';
+import {Workspace} from '../workspace.js';
+
+import {BlockBase, BlockBaseJson} from './events_block_base.js';
+import * as eventUtils from './utils.js';
+
+/**
+ * Notifies listeners when the value of a block's field has changed but the
+ * change is not yet complete, and is expected to be followed by a block change
+ * event.
+ */
+export class BlockFieldIntermediateChange extends BlockBase {
+  override type = eventUtils.BLOCK_FIELD_INTERMEDIATE_CHANGE;
+
+  // Intermediate events do not undo or redo. They may be fired frequently while
+  // the field editor widget is open. A separate BLOCK_CHANGE event is fired
+  // when the editor is closed, which combines all of the field value changes
+  // into a single change that is recorded in the undo history instead. The
+  // intermediate changes are important for reacting to immediate changes, but
+  // some event handlers would prefer to handle the less frequent final events,
+  // like when triggering workspace serialization. Technically, this method of
+  // grouping changes can result in undo perfoming actions out of order if some
+  // other event occurs between opening and closing the field editor, but such
+  // events are unlikely to cause a broken state.
+  override recordUndo = false;
+
+  /** The name of the field that changed. */
+  name?: string;
+
+  /** The original value of the element. */
+  oldValue: unknown;
+
+  /** The new value of the element. */
+  newValue: unknown;
+
+  /**
+   * @param opt_block The changed block. Undefined for a blank event.
+   * @param opt_name Name of the field affected.
+   * @param opt_oldValue Previous value of element.
+   * @param opt_newValue New value of element.
+   */
+  constructor(
+    opt_block?: Block,
+    opt_name?: string,
+    opt_oldValue?: unknown,
+    opt_newValue?: unknown
+  ) {
+    super(opt_block);
+    if (!opt_block) {
+      return; // Blank event to be populated by fromJson.
+    }
+
+    this.name = opt_name;
+    this.oldValue = opt_oldValue;
+    this.newValue = opt_newValue;
+  }
+
+  /**
+   * Encode the event as JSON.
+   *
+   * @returns JSON representation.
+   */
+  override toJson(): BlockFieldIntermediateChangeJson {
+    const json = super.toJson() as BlockFieldIntermediateChangeJson;
+    if (!this.name) {
+      throw new Error(
+        'The changed field name is undefined. Either pass a ' +
+          'name to the constructor, or call fromJson.'
+      );
+    }
+    json['name'] = this.name;
+    json['oldValue'] = this.oldValue;
+    json['newValue'] = this.newValue;
+    return json;
+  }
+
+  /**
+   * Deserializes the JSON event.
+   *
+   * @param event The event to append new properties to. Should be a subclass
+   *     of BlockFieldIntermediateChange, but we can't specify that due to the
+   *     fact that parameters to static methods in subclasses must be supertypes
+   *     of parameters to static methods in superclasses.
+   * @internal
+   */
+  static fromJson(
+    json: BlockFieldIntermediateChangeJson,
+    workspace: Workspace,
+    event?: any
+  ): BlockFieldIntermediateChange {
+    const newEvent = super.fromJson(
+      json,
+      workspace,
+      event ?? new BlockFieldIntermediateChange()
+    ) as BlockFieldIntermediateChange;
+    newEvent.name = json['name'];
+    newEvent.oldValue = json['oldValue'];
+    newEvent.newValue = json['newValue'];
+    return newEvent;
+  }
+
+  /**
+   * Does this event record any change of state?
+   *
+   * @returns False if something changed.
+   */
+  override isNull(): boolean {
+    return this.oldValue === this.newValue;
+  }
+}
+
+export interface BlockFieldIntermediateChangeJson extends BlockBaseJson {
+  name: string;
+  newValue: unknown;
+  oldValue: unknown;
+}
+
+registry.register(
+  registry.Type.EVENT,
+  eventUtils.BLOCK_FIELD_INTERMEDIATE_CHANGE,
+  BlockFieldIntermediateChange
+);

core/events/utils.ts

@@ -79,6 +79,13 @@ export const CHANGE = 'change';
  */
 export const BLOCK_CHANGE = CHANGE;
 
+/**
+ * Name of event representing an in-progress change to a field of a block, which
+ * is expected to be followed by a block change event.
+ */
+export const BLOCK_FIELD_INTERMEDIATE_CHANGE =
+  'block_field_intermediate_change';
+
 /**
  * Name of event that moves a block. Will be deprecated for BLOCK_MOVE.
  */

core/field.ts

@@ -1042,9 +1042,12 @@ export abstract class Field<T = any>
    * than this method.
    *
    * @param newValue New value.
+   * @param fireChangeEvent Whether to fire a change event. Defaults to true.
+   *     Should usually be true unless the change will be reported some other
+   *     way, e.g. an intermediate field change event.
    * @sealed
    */
-  setValue(newValue: AnyDuringMigration) {
+  setValue(newValue: AnyDuringMigration, fireChangeEvent = true) {
     const doLogging = false;
     if (newValue === null) {
       doLogging && console.log('null, return');
@@ -1080,7 +1083,7 @@ export abstract class Field<T = any>
     }
 
     this.doValueUpdate_(localValue);
-    if (source && eventUtils.isEnabled()) {
+    if (fireChangeEvent && source && eventUtils.isEnabled()) {
       eventUtils.fire(
         new (eventUtils.get(eventUtils.BLOCK_CHANGE))(
           source,

core/field_angle.ts

@@ -16,6 +16,7 @@ import {BlockSvg} from './block_svg.js';
 import * as browserEvents from './browser_events.js';
 import * as Css from './css.js';
 import * as dropDownDiv from './dropdowndiv.js';
+import * as eventUtils from './events/utils.js';
 import {Field, UnattachedFieldError} from './field.js';
 import * as fieldRegistry from './field_registry.js';
 import {
@@ -363,7 +364,26 @@ export class FieldAngle extends FieldInput<number> {
     }
     angle = this.wrapValue(angle);
     if (angle !== this.value_) {
-      this.setEditorValue_(angle);
+      // Intermediate value changes from user input are not confirmed until the
+      // user closes the editor, and may be numerous. Inhibit reporting these as
+      // normal block change events, and instead report them as special
+      // intermediate changes that do not get recorded in undo history.
+      const oldValue = this.value_;
+      this.setEditorValue_(angle, false);
+      if (
+        this.sourceBlock_ &&
+        eventUtils.isEnabled() &&
+        this.value_ !== oldValue
+      ) {
+        eventUtils.fire(
+          new (eventUtils.get(eventUtils.BLOCK_FIELD_INTERMEDIATE_CHANGE))(
+            this.sourceBlock_,
+            this.name || null,
+            oldValue,
+            this.value_
+          )
+        );
+      }
     }
   }
 

core/field_input.ts

@@ -72,6 +72,13 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
    */
   protected isTextValid_ = false;
 
+  /**
+   * The intial value of the field when the user opened an editor to change its
+   * value. When the editor is disposed, an event will be fired that uses this
+   * as the event's oldValue.
+   */
+  protected valueWhenEditorWasOpened_: string | T | null = null;
+
   /** Key down event data. */
   private onKeyDownWrapper_: browserEvents.Data | null = null;
 
@@ -328,6 +335,7 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
     WidgetDiv.show(this, block.RTL, this.widgetDispose_.bind(this));
     this.htmlInput_ = this.widgetCreate_() as HTMLInputElement;
     this.isBeingEdited_ = true;
+    this.valueWhenEditorWasOpened_ = this.value_;
 
     if (!quietInput) {
       (this.htmlInput_ as HTMLElement).focus({
@@ -410,6 +418,29 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
     // Make sure the field's node matches the field's internal value.
     this.forceRerender();
     this.onFinishEditing_(this.value_);
+
+    if (
+      this.sourceBlock_ &&
+      eventUtils.isEnabled() &&
+      this.valueWhenEditorWasOpened_ !== null &&
+      this.valueWhenEditorWasOpened_ !== this.value_
+    ) {
+      // When closing a field input widget, fire an event indicating that the
+      // user has completed a sequence of changes. The value may have changed
+      // multiple times while the editor was open, but this will fire an event
+      // containing the value when the editor was opened as well as the new one.
+      eventUtils.fire(
+        new (eventUtils.get(eventUtils.BLOCK_CHANGE))(
+          this.sourceBlock_,
+          'field',
+          this.name || null,
+          this.valueWhenEditorWasOpened_,
+          this.value_
+        )
+      );
+      this.valueWhenEditorWasOpened_ = null;
+    }
+
     eventUtils.setGroup(false);
 
     // Actual disposal.
@@ -499,7 +530,29 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
    * @param _e Keyboard event.
    */
   private onHtmlInputChange_(_e: Event) {
-    this.setValue(this.getValueFromEditorText_(this.htmlInput_!.value));
+    // Intermediate value changes from user input are not confirmed until the
+    // user closes the editor, and may be numerous. Inhibit reporting these as
+    // normal block change events, and instead report them as special
+    // intermediate changes that do not get recorded in undo history.
+    const oldValue = this.value_;
+    // Change the field's value without firing the normal change event.
+    this.setValue(this.getValueFromEditorText_(this.htmlInput_!.value), false);
+    if (
+      this.sourceBlock_ &&
+      eventUtils.isEnabled() &&
+      this.value_ !== oldValue
+    ) {
+      // Fire a special event indicating that the value changed but the change
+      // isn't complete yet and normal field change listeners can wait.
+      eventUtils.fire(
+        new (eventUtils.get(eventUtils.BLOCK_FIELD_INTERMEDIATE_CHANGE))(
+          this.sourceBlock_,
+          this.name || null,
+          oldValue,
+          this.value_
+        )
+      );
+    }
 
     // Resize the widget div after the block has finished rendering.
     renderManagement.finishQueuedRenders().then(() => {
@@ -513,8 +566,14 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
    * value whilst editing.
    *
    * @param newValue New value.
+   * @param fireChangeEvent Whether to fire a change event. Defaults to true.
+   *     Should usually be true unless the change will be reported some other
+   *     way, e.g. an intermediate field change event.
    */
-  protected setEditorValue_(newValue: AnyDuringMigration) {
+  protected setEditorValue_(
+    newValue: AnyDuringMigration,
+    fireChangeEvent = true
+  ) {
     this.isDirty_ = true;
     if (this.isBeingEdited_) {
       // In the case this method is passed an invalid value, we still
@@ -523,7 +582,7 @@ export abstract class FieldInput<T extends InputTypes> extends Field<
       // with what's shown to the user.
       this.htmlInput_!.value = this.getEditorText_(newValue);
     }
-    this.setValue(newValue);
+    this.setValue(newValue, fireChangeEvent);
   }
 
   /** Resize the editor to fit the text. */

core/procedures.ts

@@ -389,7 +389,8 @@ function mutatorChangeListener(e: Abstract) {
   if (
     e.type !== eventUtils.BLOCK_CREATE &&
     e.type !== eventUtils.BLOCK_DELETE &&
-    e.type !== eventUtils.BLOCK_CHANGE
+    e.type !== eventUtils.BLOCK_CHANGE &&
+    e.type !== eventUtils.BLOCK_FIELD_INTERMEDIATE_CHANGE
   ) {
     return;
   }

tests/mocha/event_block_field_intermediate_change_test.js

@@ -0,0 +1,40 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+goog.declareModuleId('Blockly.test.eventBlockFieldIntermediateChange');
+
+import {
+  sharedTestSetup,
+  sharedTestTeardown,
+} from './test_helpers/setup_teardown.js';
+
+suite('Field Intermediate Change Event', function () {
+  setup(function () {
+    sharedTestSetup.call(this);
+    this.workspace = new Blockly.Workspace();
+  });
+
+  teardown(function () {
+    sharedTestTeardown.call(this);
+  });
+
+  suite('Serialization', function () {
+    test('events round-trip through JSON', function () {
+      const block = this.workspace.newBlock('text', 'block_id');
+      const origEvent = new Blockly.Events.BlockFieldIntermediateChange(
+        block,
+        'TEXT',
+        'old value',
+        'new value'
+      );
+
+      const json = origEvent.toJson();
+      const newEvent = new Blockly.Events.fromJson(json, this.workspace);
+
+      chai.assert.deepEqual(newEvent, origEvent);
+    });
+  });
+});

tests/mocha/event_test.js

@@ -488,6 +488,19 @@ suite('Events', function () {
           blocks: [thisObj.block],
         }),
       },
+      {
+        title: 'Field Edit Intermediate Change',
+        class: Blockly.Events.BlockFieldIntermediateChange,
+        getArgs: (thisObj) => [thisObj.block, 'test', 'old value', 'new value'],
+        getExpectedJson: (thisObj) => ({
+          type: 'block_field_intermediate_change',
+          group: '',
+          blockId: thisObj.block.id,
+          name: 'test',
+          oldValue: 'old value',
+          newValue: 'new value',
+        }),
+      },
       {
         title: 'null to Block Marker move',
         class: Blockly.Events.MarkerMove,

tests/mocha/index.html

@@ -51,6 +51,7 @@
           'Blockly.test.eventBlockCreate',
           'Blockly.test.eventBlockDelete',
           'Blockly.test.eventBlockDrag',
+          'Blockly.test.eventBlockFieldIntermediateChange',
           'Blockly.test.eventBlockMove',
           'Blockly.test.eventBubbleOpen',
           'Blockly.test.eventClick',