From 171befa7462b4a5288ce0fa775814c267f424e2c Mon Sep 17 00:00:00 2001 From: John Nesky Date: Thu, 2 May 2024 18:57:57 -0700 Subject: [PATCH] fix!: Only fire intermediate events when editing input with invalid text. (#8054) * fix: Fire intermediate events only when editing text input. * Prefix unused arg with underscore. * Fix tests. --- core/field.ts | 22 ++++++++++++++++++---- core/field_angle.ts | 2 +- core/field_input.ts | 24 ++++++++++++++++++------ tests/mocha/field_angle_test.js | 3 ++- tests/mocha/field_number_test.js | 3 ++- tests/mocha/field_textinput_test.js | 1 + 6 files changed, 42 insertions(+), 13 deletions(-) diff --git a/core/field.ts b/core/field.ts index 7522cde93..fb6c078e9 100644 --- a/core/field.ts +++ b/core/field.ts @@ -1086,14 +1086,22 @@ export abstract class Field } const classValidation = this.doClassValidation_(newValue); - const classValue = this.processValidation_(newValue, classValidation); + const classValue = this.processValidation_( + newValue, + classValidation, + fireChangeEvent, + ); if (classValue instanceof Error) { doLogging && console.log('invalid class validation, return'); return; } const localValidation = this.getValidator()?.call(this, classValue); - const localValue = this.processValidation_(classValue, localValidation); + const localValue = this.processValidation_( + classValue, + localValidation, + fireChangeEvent, + ); if (localValue instanceof Error) { doLogging && console.log('invalid local validation, return'); return; @@ -1135,14 +1143,16 @@ export abstract class Field * * @param newValue New value. * @param validatedValue Validated value. + * @param fireChangeEvent Whether to fire a change event if the value changes. * @returns New value, or an Error object. */ private processValidation_( newValue: AnyDuringMigration, validatedValue: T | null | undefined, + fireChangeEvent: boolean, ): T | Error { if (validatedValue === null) { - this.doValueInvalid_(newValue); + this.doValueInvalid_(newValue, fireChangeEvent); if (this.isDirty_) { this.forceRerender(); } @@ -1209,8 +1219,12 @@ export abstract class Field * No-op by default. * * @param _invalidValue The input value that was determined to be invalid. + * @param _fireChangeEvent Whether to fire a change event if the value changes. */ - protected doValueInvalid_(_invalidValue: AnyDuringMigration) {} + protected doValueInvalid_( + _invalidValue: AnyDuringMigration, + _fireChangeEvent: boolean = true, + ) {} // NOP /** diff --git a/core/field_angle.ts b/core/field_angle.ts index e65ef16a9..9e94ea4d8 100644 --- a/core/field_angle.ts +++ b/core/field_angle.ts @@ -366,7 +366,7 @@ export class FieldAngle extends FieldInput { // 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); + this.setEditorValue_(angle, /* fireChangeEvent= */ false); if ( this.sourceBlock_ && eventUtils.isEnabled() && diff --git a/core/field_input.ts b/core/field_input.ts index 513047054..5c26f42b3 100644 --- a/core/field_input.ts +++ b/core/field_input.ts @@ -166,17 +166,26 @@ export abstract class FieldInput extends Field< * value while allowing the display text to be handled by the htmlInput_. * * @param _invalidValue The input value that was determined to be invalid. - * This is not used by the text input because its display value is stored - * on the htmlInput_. + * This is not used by the text input because its display value is stored + * on the htmlInput_. + * @param fireChangeEvent Whether to fire a change event if the value changes. */ - protected override doValueInvalid_(_invalidValue: AnyDuringMigration) { + protected override doValueInvalid_( + _invalidValue: AnyDuringMigration, + fireChangeEvent: boolean = true, + ) { if (this.isBeingEdited_) { this.isDirty_ = true; this.isTextValid_ = false; const oldValue = this.value_; // Revert value when the text becomes invalid. - this.value_ = this.htmlInput_!.getAttribute('data-untyped-default-value'); - if (this.sourceBlock_ && eventUtils.isEnabled()) { + this.value_ = this.valueWhenEditorWasOpened_; + if ( + this.sourceBlock_ && + eventUtils.isEnabled() && + this.value_ !== oldValue && + fireChangeEvent + ) { eventUtils.fire( new (eventUtils.get(eventUtils.BLOCK_CHANGE))( this.sourceBlock_, @@ -566,7 +575,10 @@ export abstract class FieldInput extends Field< // 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); + this.setValue( + this.getValueFromEditorText_(this.htmlInput_!.value), + /* fireChangeEvent= */ false, + ); if ( this.sourceBlock_ && eventUtils.isEnabled() && diff --git a/tests/mocha/field_angle_test.js b/tests/mocha/field_angle_test.js index 62afd8d27..d2630581f 100644 --- a/tests/mocha/field_angle_test.js +++ b/tests/mocha/field_angle_test.js @@ -138,6 +138,7 @@ suite('Angle Fields', function () { suite('Validators', function () { setup(function () { this.field = new Blockly.FieldAngle(1); + this.field.valueWhenEditorWasOpened_ = this.field.getValue(); this.field.htmlInput_ = document.createElement('input'); this.field.htmlInput_.setAttribute('data-old-value', '1'); this.field.htmlInput_.setAttribute('data-untyped-default-value', '1'); @@ -153,7 +154,7 @@ suite('Angle Fields', function () { return null; }, value: 2, - expectedValue: '1', + expectedValue: 1, }, { title: 'Force Mult of 30 Validator', diff --git a/tests/mocha/field_number_test.js b/tests/mocha/field_number_test.js index 4dcc930da..c6737668d 100644 --- a/tests/mocha/field_number_test.js +++ b/tests/mocha/field_number_test.js @@ -261,6 +261,7 @@ suite('Number Fields', function () { suite('Validators', function () { setup(function () { this.field = new Blockly.FieldNumber(1); + this.field.valueWhenEditorWasOpened_ = this.field.getValue(); this.field.htmlInput_ = document.createElement('input'); this.field.htmlInput_.setAttribute('data-old-value', '1'); this.field.htmlInput_.setAttribute('data-untyped-default-value', '1'); @@ -276,7 +277,7 @@ suite('Number Fields', function () { return null; }, value: 2, - expectedValue: '1', + expectedValue: 1, }, { title: 'Force End with 6 Validator', diff --git a/tests/mocha/field_textinput_test.js b/tests/mocha/field_textinput_test.js index 5a8a82479..f68006e87 100644 --- a/tests/mocha/field_textinput_test.js +++ b/tests/mocha/field_textinput_test.js @@ -129,6 +129,7 @@ suite('Text Input Fields', function () { suite('Validators', function () { setup(function () { this.field = new Blockly.FieldTextInput('value'); + this.field.valueWhenEditorWasOpened_ = this.field.getValue(); this.field.htmlInput_ = document.createElement('input'); this.field.htmlInput_.setAttribute('data-old-value', 'value'); this.field.htmlInput_.setAttribute('data-untyped-default-value', 'value');