From a4d588e5383410e558dece566fac6c46ab281894 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Tue, 3 Oct 2023 18:02:58 +0300 Subject: [PATCH 01/12] fix: do not validate on Enter if no value commit occurred update JSDoc improve naming --- .../src/vaadin-date-picker-mixin.js | 111 +++++++++++++----- .../value-commit-auto-open-disabled.common.js | 8 +- .../date-picker/test/value-commit.common.js | 8 +- 3 files changed, 90 insertions(+), 37 deletions(-) diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js index 2fb2de2ccf..dae0aee7d1 100644 --- a/packages/date-picker/src/vaadin-date-picker-mixin.js +++ b/packages/date-picker/src/vaadin-date-picker-mixin.js @@ -427,6 +427,17 @@ export const DatePickerMixin = (subclass) => return null; } + /** + * The input element's value if the component has been unable to parse it, an empty string otherwise. + */ + get _unparsableValue() { + if (this._inputElementValue && !this.__parseDate(this._inputElementValue)) { + return this._inputElementValue; + } + + return ''; + } + /** * Override an event listener from `DelegateFocusMixin` * @protected @@ -447,11 +458,11 @@ export const DatePickerMixin = (subclass) => super._onBlur(event); if (!this.opened) { - this.__commitParsedOrFocusedDate(); + const didValueCommitOccur = this.__commitParsedOrFocusedDate(); // Do not validate when focusout is caused by document // losing focus, which happens on browser tab switch. - if (document.hasFocus()) { + if (!didValueCommitOccur && document.hasFocus()) { this.validate(); } } @@ -645,27 +656,65 @@ export const DatePickerMixin = (subclass) => this._shouldKeepFocusRing = focused && this._keyboardActive; } - /** @private */ - __dispatchChange() { - this.validate(); - this.dispatchEvent(new CustomEvent('change', { bubbles: true })); + /** + * Depending on the type of value change that has taken place + * since the last commit attempt, triggers validation and fires + * an event: + * + * ```text + * +------------------------+--------+ + * | Type of change | Event | + * +------------------------+--------+ + * | empty => parsable | change | + * | empty => unparsable | | + * | parsable => empty | change | + * | parsable => unparsable | change | + * | unparsable => empty | | + * | unparsable => parsable | | + * +------------------------+--------+ + * ``` + * + * If no value change is detected, the method returns false. + * + * (the above table has been generated with ChatGPT) + * + * @private + * @return {boolean} whether a change has been detected and committed. + */ + __commitValueChange() { + let result = false; + + if (this.__committedValue !== this.value) { + this.dirty = true; + this.validate(); + this.dispatchEvent(new CustomEvent('change', { bubbles: true })); + result = true; + } else if (this.__committedUnparsableValue !== this._unparsableValue) { + this.validate(); + result = true; + } + + this.__committedValue = this.value; + this.__committedUnparsableValue = this._unparsableValue; + + return result; } /** - * Sets the given date as the value and fires a change event - * if the value has changed. + * Sets the given date as the value and tries to commit it. * * @param {Date} date + * @return {boolean} whether an actual commit occurred. * @private */ __commitDate(date) { - const prevValue = this.value; - + // Prevent the value observer from treating the following value change + // as initiated programmatically by the developer, and therefore + // from automatically committing it without a change event. + this.__keepCommittedValue = true; this._selectedDate = date; - - if (prevValue !== this.value) { - this.__dispatchChange(); - } + this.__keepCommittedValue = false; + return this.__commitValueChange(); } /** @private */ @@ -801,6 +850,11 @@ export const DatePickerMixin = (subclass) => this._selectedDate = null; } + if (!this.__keepCommittedValue) { + this.__committedValue = this.value; + this.__committedUnparsableValue = ''; + } + this._toggleHasValue(this._hasValue); } @@ -893,13 +947,16 @@ export const DatePickerMixin = (subclass) => /** * Tries to parse the input element's value as a date. When succeeds, - * sets the resulting date as the value and fires a change event - * (if the value has changed). If no i18n parser is provided, sets - * the focused date as the value. + * sets the resulting date as the value and tries to commit it. When fails, + * resets the value to an empty string and tries to commit it. + * If no i18n parser is provided, sets the focused date as the value. * * @private + * @return {boolean} whether an actual commit occurred. */ __commitParsedOrFocusedDate() { + let result = false; + // Select the parsed input or focused date this._ignoreFocusedDateChange = true; if (this.i18n.parseDate) { @@ -907,17 +964,18 @@ export const DatePickerMixin = (subclass) => const parsedDate = this.__parseDate(inputValue); if (parsedDate) { - this.__commitDate(parsedDate); + result = this.__commitDate(parsedDate); } else { this.__keepInputValue = true; - this.__commitDate(null); - this._selectedDate = null; + result = this.__commitDate(null); this.__keepInputValue = false; } } else if (this._focusedDate) { - this.__commitDate(this._focusedDate); + result = this.__commitDate(this._focusedDate); } this._ignoreFocusedDateChange = false; + + return result; } /** @protected */ @@ -927,18 +985,17 @@ export const DatePickerMixin = (subclass) => this.__showOthers(); this.__showOthers = null; } - window.removeEventListener('scroll', this._boundOnScroll, true); - this.__commitParsedOrFocusedDate(); + const didValueCommitOccur = this.__commitParsedOrFocusedDate(); if (this._nativeInput && this._nativeInput.selectionStart) { this._nativeInput.selectionStart = this._nativeInput.selectionEnd; } - // No need to revalidate the value after `_selectedDateChanged` + // No need to revalidate the value after it has been just committed. // Needed in case the value was not changed: open and close dropdown, // especially on outside click. On Esc key press, do not validate. - if (!this.value && !this._keyboardActive) { + if (!didValueCommitOccur && !this.value && !this._keyboardActive) { this.validate(); } } @@ -1080,16 +1137,12 @@ export const DatePickerMixin = (subclass) => * @override */ _onEnter(_event) { - const oldValue = this.value; if (this.opened) { // Closing will implicitly select parsed or focused date this.close(); } else { this.__commitParsedOrFocusedDate(); } - if (oldValue === this.value) { - this.validate(); - } } /** diff --git a/packages/date-picker/test/value-commit-auto-open-disabled.common.js b/packages/date-picker/test/value-commit-auto-open-disabled.common.js index 154e613a6e..ad1e77ad5b 100644 --- a/packages/date-picker/test/value-commit-auto-open-disabled.common.js +++ b/packages/date-picker/test/value-commit-auto-open-disabled.common.js @@ -50,9 +50,9 @@ describe('value commit - autoOpenDisabled', () => { expectValidationOnly(); }); - it('should not commit but validate on Enter', async () => { + it('should not commit on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectNoValueCommit(); }); it('should not commit but validate on outside click', () => { @@ -201,9 +201,9 @@ describe('value commit - autoOpenDisabled', () => { expectValidationOnly(); }); - it('should not commit but validate on Enter', async () => { + it('should not commit on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectNoValueCommit(); }); it('should not commit on Escape', async () => { diff --git a/packages/date-picker/test/value-commit.common.js b/packages/date-picker/test/value-commit.common.js index 5ccad26272..139f7326f4 100644 --- a/packages/date-picker/test/value-commit.common.js +++ b/packages/date-picker/test/value-commit.common.js @@ -57,9 +57,9 @@ describe('value commit', () => { expectValidationOnly(); }); - it('should not commit but validate on Enter', async () => { + it('should not commit on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectNoValueCommit(); }); it('should not commit on Escape', async () => { @@ -300,9 +300,9 @@ describe('value commit', () => { expectValidationOnly(); }); - it('should not commit but validate on Enter', async () => { + it('should not commit on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectNoValueCommit(); }); it('should not commit on Escape', async () => { From 3062576c6b98a3fdeedeb7cb9b55b7aa72f7b4b4 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 4 Oct 2023 11:50:42 +0300 Subject: [PATCH 02/12] fire unparsable-change event --- .../src/vaadin-date-picker-mixin.js | 69 ++++++++----------- .../date-picker/src/vaadin-date-picker.d.ts | 8 +++ .../date-picker/src/vaadin-date-picker.js | 1 + .../value-commit-auto-open-disabled.common.js | 8 +-- .../date-picker/test/value-commit.common.js | 8 +-- 5 files changed, 46 insertions(+), 48 deletions(-) diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js index dae0aee7d1..9eff4ce612 100644 --- a/packages/date-picker/src/vaadin-date-picker-mixin.js +++ b/packages/date-picker/src/vaadin-date-picker-mixin.js @@ -458,11 +458,11 @@ export const DatePickerMixin = (subclass) => super._onBlur(event); if (!this.opened) { - const didValueCommitOccur = this.__commitParsedOrFocusedDate(); + this.__commitParsedOrFocusedDate(); // Do not validate when focusout is caused by document // losing focus, which happens on browser tab switch. - if (!didValueCommitOccur && document.hasFocus()) { + if (document.hasFocus()) { this.validate(); } } @@ -662,49 +662,39 @@ export const DatePickerMixin = (subclass) => * an event: * * ```text - * +------------------------+--------+ - * | Type of change | Event | - * +------------------------+--------+ - * | empty => parsable | change | - * | empty => unparsable | | - * | parsable => empty | change | - * | parsable => unparsable | change | - * | unparsable => empty | | - * | unparsable => parsable | | - * +------------------------+--------+ + * +------------------------+-------------------+ + * | Value change type | Event | + * +------------------------+-------------------+ + * | empty => parsable | change | + * | empty => unparsable | unparsable-change | + * | parsable => empty | change | + * | parsable => unparsable | change | + * | unparsable => empty | unparsable-change | + * | unparsable => parsable | unparsable-change | + * +------------------------+-------------------+ * ``` * - * If no value change is detected, the method returns false. - * - * (the above table has been generated with ChatGPT) + * If no value change is detected, the method returns. * * @private * @return {boolean} whether a change has been detected and committed. */ __commitValueChange() { - let result = false; - if (this.__committedValue !== this.value) { - this.dirty = true; this.validate(); this.dispatchEvent(new CustomEvent('change', { bubbles: true })); - result = true; } else if (this.__committedUnparsableValue !== this._unparsableValue) { - this.validate(); - result = true; + this.dispatchEvent(new CustomEvent('unparsable-change')); } this.__committedValue = this.value; this.__committedUnparsableValue = this._unparsableValue; - - return result; } /** - * Sets the given date as the value and tries to commit it. + * Sets the given date as the value and commits it. * * @param {Date} date - * @return {boolean} whether an actual commit occurred. * @private */ __commitDate(date) { @@ -714,7 +704,7 @@ export const DatePickerMixin = (subclass) => this.__keepCommittedValue = true; this._selectedDate = date; this.__keepCommittedValue = false; - return this.__commitValueChange(); + this.__commitValueChange(); } /** @private */ @@ -946,17 +936,14 @@ export const DatePickerMixin = (subclass) => } /** - * Tries to parse the input element's value as a date. When succeeds, - * sets the resulting date as the value and tries to commit it. When fails, - * resets the value to an empty string and tries to commit it. - * If no i18n parser is provided, sets the focused date as the value. + * Tries to parse the input element's value as a date. If the input value + * is parsable, commits the resulting date as the value. Otherwise, commits + * an empty string as the value. If no i18n parser is provided, commits + * the focused date as the value. * * @private - * @return {boolean} whether an actual commit occurred. */ __commitParsedOrFocusedDate() { - let result = false; - // Select the parsed input or focused date this._ignoreFocusedDateChange = true; if (this.i18n.parseDate) { @@ -964,18 +951,16 @@ export const DatePickerMixin = (subclass) => const parsedDate = this.__parseDate(inputValue); if (parsedDate) { - result = this.__commitDate(parsedDate); + this.__commitDate(parsedDate); } else { this.__keepInputValue = true; - result = this.__commitDate(null); + this.__commitDate(null); this.__keepInputValue = false; } } else if (this._focusedDate) { - result = this.__commitDate(this._focusedDate); + this.__commitDate(this._focusedDate); } this._ignoreFocusedDateChange = false; - - return result; } /** @protected */ @@ -987,7 +972,7 @@ export const DatePickerMixin = (subclass) => } window.removeEventListener('scroll', this._boundOnScroll, true); - const didValueCommitOccur = this.__commitParsedOrFocusedDate(); + this.__commitParsedOrFocusedDate(); if (this._nativeInput && this._nativeInput.selectionStart) { this._nativeInput.selectionStart = this._nativeInput.selectionEnd; @@ -995,7 +980,7 @@ export const DatePickerMixin = (subclass) => // No need to revalidate the value after it has been just committed. // Needed in case the value was not changed: open and close dropdown, // especially on outside click. On Esc key press, do not validate. - if (!didValueCommitOccur && !this.value && !this._keyboardActive) { + if (!this.value && !this._keyboardActive) { this.validate(); } } @@ -1137,12 +1122,16 @@ export const DatePickerMixin = (subclass) => * @override */ _onEnter(_event) { + const oldValue = this.value; if (this.opened) { // Closing will implicitly select parsed or focused date this.close(); } else { this.__commitParsedOrFocusedDate(); } + if (oldValue === this.value) { + this.validate(); + } } /** diff --git a/packages/date-picker/src/vaadin-date-picker.d.ts b/packages/date-picker/src/vaadin-date-picker.d.ts index 960f7b5d94..9b359757b4 100644 --- a/packages/date-picker/src/vaadin-date-picker.d.ts +++ b/packages/date-picker/src/vaadin-date-picker.d.ts @@ -16,6 +16,11 @@ export type DatePickerChangeEvent = Event & { target: DatePicker; }; +/** + * Fired when the user commits an unparsable value change and there is no change event. + */ +export type DatePickerUnparsableChangeEvent = CustomEvent; + /** * Fired when the `opened` property changes. */ @@ -43,6 +48,8 @@ export interface DatePickerCustomEventMap { 'value-changed': DatePickerValueChangedEvent; + 'unparsable-change': DatePickerUnparsableChangeEvent; + validated: DatePickerValidatedEvent; } @@ -149,6 +156,7 @@ export interface DatePickerEventMap extends HTMLElementEventMap, DatePickerCusto * See [Styling Components](https://vaadin.com/docs/latest/styling/styling-components) documentation. * * @fires {Event} change - Fired when the user commits a value change. + * @fires {Event} unparsable-change Fired when the user commits an unparsable value change and there is no change event. * @fires {CustomEvent} invalid-changed - Fired when the `invalid` property changes. * @fires {CustomEvent} opened-changed - Fired when the `opened` property changes. * @fires {CustomEvent} value-changed - Fired when the `value` property changes. diff --git a/packages/date-picker/src/vaadin-date-picker.js b/packages/date-picker/src/vaadin-date-picker.js index e3b063f215..5def62320e 100644 --- a/packages/date-picker/src/vaadin-date-picker.js +++ b/packages/date-picker/src/vaadin-date-picker.js @@ -119,6 +119,7 @@ registerStyles('vaadin-date-picker', [inputFieldShared, datePickerStyles], { mod * See [Styling Components](https://vaadin.com/docs/latest/styling/styling-components) documentation. * * @fires {Event} change - Fired when the user commits a value change. + * @fires {Event} unparsable-change Fired when the user commits an unparsable value change and there is no change event. * @fires {CustomEvent} invalid-changed - Fired when the `invalid` property changes. * @fires {CustomEvent} opened-changed - Fired when the `opened` property changes. * @fires {CustomEvent} value-changed - Fired when the `value` property changes. diff --git a/packages/date-picker/test/value-commit-auto-open-disabled.common.js b/packages/date-picker/test/value-commit-auto-open-disabled.common.js index ad1e77ad5b..154e613a6e 100644 --- a/packages/date-picker/test/value-commit-auto-open-disabled.common.js +++ b/packages/date-picker/test/value-commit-auto-open-disabled.common.js @@ -50,9 +50,9 @@ describe('value commit - autoOpenDisabled', () => { expectValidationOnly(); }); - it('should not commit on Enter', async () => { + it('should not commit but validate on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectNoValueCommit(); + expectValidationOnly(); }); it('should not commit but validate on outside click', () => { @@ -201,9 +201,9 @@ describe('value commit - autoOpenDisabled', () => { expectValidationOnly(); }); - it('should not commit on Enter', async () => { + it('should not commit but validate on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectNoValueCommit(); + expectValidationOnly(); }); it('should not commit on Escape', async () => { diff --git a/packages/date-picker/test/value-commit.common.js b/packages/date-picker/test/value-commit.common.js index 139f7326f4..5ccad26272 100644 --- a/packages/date-picker/test/value-commit.common.js +++ b/packages/date-picker/test/value-commit.common.js @@ -57,9 +57,9 @@ describe('value commit', () => { expectValidationOnly(); }); - it('should not commit on Enter', async () => { + it('should not commit but validate on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectNoValueCommit(); + expectValidationOnly(); }); it('should not commit on Escape', async () => { @@ -300,9 +300,9 @@ describe('value commit', () => { expectValidationOnly(); }); - it('should not commit on Enter', async () => { + it('should not commit but validate on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectNoValueCommit(); + expectValidationOnly(); }); it('should not commit on Escape', async () => { From 3731fa171bb20d8b33302c312521789e5f1ac575 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 4 Oct 2023 17:02:25 +0300 Subject: [PATCH 03/12] update tests --- .../src/vaadin-date-picker-mixin.js | 59 ++++++++----- .../value-commit-auto-open-disabled.common.js | 87 ++++++++++++++----- .../date-picker/test/value-commit.common.js | 69 +++++++++++---- 3 files changed, 153 insertions(+), 62 deletions(-) diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js index 9eff4ce612..af5b0e14a7 100644 --- a/packages/date-picker/src/vaadin-date-picker-mixin.js +++ b/packages/date-picker/src/vaadin-date-picker-mixin.js @@ -458,11 +458,11 @@ export const DatePickerMixin = (subclass) => super._onBlur(event); if (!this.opened) { - this.__commitParsedOrFocusedDate(); + const didValueCommitOccur = this.__commitParsedOrFocusedDate(); // Do not validate when focusout is caused by document // losing focus, which happens on browser tab switch. - if (document.hasFocus()) { + if (!didValueCommitOccur && document.hasFocus()) { this.validate(); } } @@ -662,39 +662,49 @@ export const DatePickerMixin = (subclass) => * an event: * * ```text - * +------------------------+-------------------+ - * | Value change type | Event | - * +------------------------+-------------------+ - * | empty => parsable | change | - * | empty => unparsable | unparsable-change | - * | parsable => empty | change | - * | parsable => unparsable | change | - * | unparsable => empty | unparsable-change | - * | unparsable => parsable | unparsable-change | - * +------------------------+-------------------+ + * +--------------------------+-------------------+ + * | Type of value change | Event | + * +--------------------------+-------------------+ + * | empty => parsable | change | + * | parsable => empty | change | + * | parsable => parsable | change | + * | parsable => unparsable | change | + * | empty => unparsable | unparsable-change | + * | unparsable => empty | unparsable-change | + * | unparsable => parsable | unparsable-change | + * | unparsable => unparsable | unparsable-change | + * +--------------------------+-------------------+ * ``` * - * If no value change is detected, the method returns. + * If no value change is detected, the method returns false. * * @private - * @return {boolean} whether a change has been detected and committed. + * @return {boolean} whether a change was detected and committed. */ __commitValueChange() { + let result = false; + if (this.__committedValue !== this.value) { + result = true; this.validate(); this.dispatchEvent(new CustomEvent('change', { bubbles: true })); } else if (this.__committedUnparsableValue !== this._unparsableValue) { + result = true; + this.validate(); this.dispatchEvent(new CustomEvent('unparsable-change')); } this.__committedValue = this.value; this.__committedUnparsableValue = this._unparsableValue; + + return result; } /** * Sets the given date as the value and commits it. * * @param {Date} date + * @return {boolean} whether there was an actual value change to commit. * @private */ __commitDate(date) { @@ -704,7 +714,7 @@ export const DatePickerMixin = (subclass) => this.__keepCommittedValue = true; this._selectedDate = date; this.__keepCommittedValue = false; - this.__commitValueChange(); + return this.__commitValueChange(); } /** @private */ @@ -942,8 +952,11 @@ export const DatePickerMixin = (subclass) => * the focused date as the value. * * @private + * @return {boolean} whether there was an actual value change to commit. */ __commitParsedOrFocusedDate() { + let result = false; + // Select the parsed input or focused date this._ignoreFocusedDateChange = true; if (this.i18n.parseDate) { @@ -951,16 +964,18 @@ export const DatePickerMixin = (subclass) => const parsedDate = this.__parseDate(inputValue); if (parsedDate) { - this.__commitDate(parsedDate); + result = this.__commitDate(parsedDate); } else { this.__keepInputValue = true; - this.__commitDate(null); + result = this.__commitDate(null); this.__keepInputValue = false; } } else if (this._focusedDate) { - this.__commitDate(this._focusedDate); + result = this.__commitDate(this._focusedDate); } this._ignoreFocusedDateChange = false; + + return result; } /** @protected */ @@ -972,7 +987,7 @@ export const DatePickerMixin = (subclass) => } window.removeEventListener('scroll', this._boundOnScroll, true); - this.__commitParsedOrFocusedDate(); + const didValueCommitOccur = this.__commitParsedOrFocusedDate(); if (this._nativeInput && this._nativeInput.selectionStart) { this._nativeInput.selectionStart = this._nativeInput.selectionEnd; @@ -980,7 +995,7 @@ export const DatePickerMixin = (subclass) => // No need to revalidate the value after it has been just committed. // Needed in case the value was not changed: open and close dropdown, // especially on outside click. On Esc key press, do not validate. - if (!this.value && !this._keyboardActive) { + if (!didValueCommitOccur && !this.value && !this._keyboardActive) { this.validate(); } } @@ -1122,16 +1137,12 @@ export const DatePickerMixin = (subclass) => * @override */ _onEnter(_event) { - const oldValue = this.value; if (this.opened) { // Closing will implicitly select parsed or focused date this.close(); } else { this.__commitParsedOrFocusedDate(); } - if (oldValue === this.value) { - this.validate(); - } } /** diff --git a/packages/date-picker/test/value-commit-auto-open-disabled.common.js b/packages/date-picker/test/value-commit-auto-open-disabled.common.js index 154e613a6e..7c821cf827 100644 --- a/packages/date-picker/test/value-commit-auto-open-disabled.common.js +++ b/packages/date-picker/test/value-commit-auto-open-disabled.common.js @@ -6,7 +6,7 @@ import sinon from 'sinon'; const TODAY_DATE = new Date().toISOString().split('T')[0]; describe('value commit - autoOpenDisabled', () => { - let datePicker, valueChangedSpy, validateSpy, changeSpy; + let datePicker, valueChangedSpy, validateSpy, changeSpy, unparsableChangeSpy; function expectNoValueCommit() { expect(valueChangedSpy).to.be.not.called; @@ -16,14 +16,22 @@ describe('value commit - autoOpenDisabled', () => { function expectValueCommit(value) { expect(valueChangedSpy).to.be.calledOnce; - // TODO: Optimize the number of validation runs. - expect(validateSpy).to.be.called; - expect(validateSpy.firstCall).to.be.calledAfter(valueChangedSpy.firstCall); + expect(validateSpy).to.be.calledOnce; + expect(validateSpy).to.be.calledAfter(valueChangedSpy); + expect(unparsableChangeSpy).to.be.not.called; expect(changeSpy).to.be.calledOnce; - expect(changeSpy.firstCall).to.be.calledAfter(validateSpy.firstCall); + expect(changeSpy).to.be.calledAfter(validateSpy); expect(datePicker.value).to.equal(value); } + function expectUnparsableValueCommit() { + expect(valueChangedSpy).to.be.not.called; + expect(validateSpy).to.be.calledOnce; + expect(changeSpy).to.be.not.called; + expect(unparsableChangeSpy).to.be.calledOnce; + expect(unparsableChangeSpy).to.be.calledAfter(validateSpy); + } + function expectValidationOnly() { expect(valueChangedSpy).to.be.not.called; expect(validateSpy).to.be.calledOnce; @@ -41,6 +49,9 @@ describe('value commit - autoOpenDisabled', () => { changeSpy = sinon.spy().named('changeSpy'); datePicker.addEventListener('change', changeSpy); + unparsableChangeSpy = sinon.spy().named('unparsableChangeSpy'); + datePicker.addEventListener('unparsable-change', unparsableChangeSpy); + datePicker.focus(); }); @@ -50,9 +61,9 @@ describe('value commit - autoOpenDisabled', () => { expectValidationOnly(); }); - it('should not commit but validate on Enter', async () => { + it('should not commit on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectNoValueCommit(); }); it('should not commit but validate on outside click', () => { @@ -130,21 +141,21 @@ describe('value commit - autoOpenDisabled', () => { await sendKeys({ type: 'foo' }); }); - it('should not commit but validate on blur', () => { + it('should commit as unparsable value change on blur', () => { datePicker.blur(); - expectValidationOnly(); + expectUnparsableValueCommit(); expect(datePicker.inputElement.value).to.equal('foo'); }); - it('should not commit but validate on Enter', async () => { + it('should commit as unparsable value change on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectUnparsableValueCommit(); expect(datePicker.inputElement.value).to.equal('foo'); }); - it('should not commit but validate on outside click', () => { + it('should commit as unparsable value change on outside click', () => { outsideClick(); - expectValidationOnly(); + expectUnparsableValueCommit(); expect(datePicker.inputElement.value).to.equal('foo'); }); @@ -160,6 +171,7 @@ describe('value commit - autoOpenDisabled', () => { await sendKeys({ type: 'foo' }); await sendKeys({ press: 'Enter' }); validateSpy.resetHistory(); + unparsableChangeSpy.resetHistory(); }); describe('input cleared with Backspace', () => { @@ -168,19 +180,46 @@ describe('value commit - autoOpenDisabled', () => { await sendKeys({ press: 'Backspace' }); }); - it('should not commit but validate on blur', () => { + it('should commit as unparsable value change on blur', () => { datePicker.blur(); - expectValidationOnly(); + expectUnparsableValueCommit(); }); - it('should not commit but validate on Enter', async () => { + it('should commit as unparsable value change on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectUnparsableValueCommit(); }); - it('should not commit but validate on outside click', () => { + it('should commit as unparsable value change on outside click', () => { outsideClick(); - expectValidationOnly(); + expectUnparsableValueCommit(); + }); + + it('should clear and commit as unparsable value change on Escape', async () => { + await sendKeys({ press: 'Escape' }); + expectUnparsableValueCommit(); + expect(datePicker.inputElement.value).to.equal(''); + }); + }); + + describe('unparsable input changed', () => { + beforeEach(async () => { + await sendKeys({ type: 'bar' }); + }); + + it('should commit as unparsable value change on blur', () => { + datePicker.blur(); + expectUnparsableValueCommit(); + }); + + it('should commit as unparsable value change on Enter', async () => { + await sendKeys({ press: 'Enter' }); + expectUnparsableValueCommit(); + }); + + it('should commit as unparsable value change on outside click', () => { + outsideClick(); + expectUnparsableValueCommit(); }); }); }); @@ -201,9 +240,9 @@ describe('value commit - autoOpenDisabled', () => { expectValidationOnly(); }); - it('should not commit but validate on Enter', async () => { + it('should not commit on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectNoValueCommit(); }); it('should not commit on Escape', async () => { @@ -246,6 +285,12 @@ describe('value commit - autoOpenDisabled', () => { await sendKeys({ type: 'foo' }); }); + it('should commit an empty value on blur', () => { + datePicker.blur(); + expectValueCommit(''); + expect(datePicker.inputElement.value).to.equal('foo'); + }); + it('should commit an empty value on Enter', async () => { await sendKeys({ press: 'Enter' }); expectValueCommit(''); diff --git a/packages/date-picker/test/value-commit.common.js b/packages/date-picker/test/value-commit.common.js index 5ccad26272..447f3fb2cd 100644 --- a/packages/date-picker/test/value-commit.common.js +++ b/packages/date-picker/test/value-commit.common.js @@ -13,7 +13,7 @@ const TODAY_DATE = formatDateISO(new Date()); const YESTERDAY_DATE = formatDateISO(new Date(Date.now() - 3600 * 1000 * 24)); describe('value commit', () => { - let datePicker, valueChangedSpy, validateSpy, changeSpy; + let datePicker, valueChangedSpy, validateSpy, changeSpy, unparsableChangeSpy; function expectNoValueCommit() { expect(valueChangedSpy).to.be.not.called; @@ -23,14 +23,22 @@ describe('value commit', () => { function expectValueCommit(value) { expect(valueChangedSpy).to.be.calledOnce; - // TODO: Optimize the number of validation runs. - expect(validateSpy).to.be.called; - expect(validateSpy.firstCall).to.be.calledAfter(valueChangedSpy.firstCall); + expect(validateSpy).to.be.calledOnce; + expect(validateSpy).to.be.calledAfter(valueChangedSpy); + expect(unparsableChangeSpy).to.be.not.called; expect(changeSpy).to.be.calledOnce; - expect(changeSpy.firstCall).to.be.calledAfter(validateSpy.firstCall); + expect(changeSpy).to.be.calledAfter(validateSpy); expect(datePicker.value).to.equal(value); } + function expectUnparsableValueCommit() { + expect(valueChangedSpy).to.be.not.called; + expect(validateSpy).to.be.calledOnce; + expect(changeSpy).to.be.not.called; + expect(unparsableChangeSpy).to.be.calledOnce; + expect(unparsableChangeSpy).to.be.calledAfter(validateSpy); + } + function expectValidationOnly() { expect(valueChangedSpy).to.be.not.called; expect(validateSpy).to.be.calledOnce; @@ -48,6 +56,9 @@ describe('value commit', () => { changeSpy = sinon.spy().named('changeSpy'); datePicker.addEventListener('change', changeSpy); + unparsableChangeSpy = sinon.spy().named('unparsableChangeSpy'); + datePicker.addEventListener('unparsable-change', unparsableChangeSpy); + datePicker.focus(); }); @@ -57,9 +68,9 @@ describe('value commit', () => { expectValidationOnly(); }); - it('should not commit but validate on Enter', async () => { + it('should not commit on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectNoValueCommit(); }); it('should not commit on Escape', async () => { @@ -151,15 +162,15 @@ describe('value commit', () => { await waitForOverlayRender(); }); - it('should not commit but validate on Enter', async () => { + it('should commit as unparsable value change on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectUnparsableValueCommit(); expect(datePicker.inputElement.value).to.equal('foo'); }); - it('should not commit but validate on close with outside click', () => { + it('should commit as unparsable value change on close with outside click', () => { outsideClick(); - expectValidationOnly(); + expectUnparsableValueCommit(); expect(datePicker.inputElement.value).to.equal('foo'); }); @@ -176,6 +187,7 @@ describe('value commit', () => { await sendKeys({ press: 'Enter' }); await waitForOverlayRender(); validateSpy.resetHistory(); + unparsableChangeSpy.resetHistory(); }); describe('input cleared with Backspace', () => { @@ -184,14 +196,37 @@ describe('value commit', () => { await sendKeys({ press: 'Backspace' }); }); - it('should not commit but validate on Enter', async () => { + it('should commit as unparsable value change on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectUnparsableValueCommit(); }); - it('should not commit but validate on outside click', () => { + it('should commit as unparsable value change on outside click', () => { outsideClick(); - expectValidationOnly(); + expectUnparsableValueCommit(); + }); + }); + + describe('unparsable input changed', () => { + beforeEach(async () => { + await sendKeys({ type: 'bar' }); + await waitForOverlayRender(); + }); + + it('should commit as unparsable value change on Enter', async () => { + await sendKeys({ press: 'Enter' }); + expectUnparsableValueCommit(); + }); + + it('should commit as unparsable value change on close with outside click', () => { + outsideClick(); + expectUnparsableValueCommit(); + }); + + it('should clear and commit as unparsable value change on close with Escape', async () => { + await sendKeys({ press: 'Escape' }); + expectUnparsableValueCommit(); + expect(datePicker.inputElement.value).to.equal(''); }); }); }); @@ -300,9 +335,9 @@ describe('value commit', () => { expectValidationOnly(); }); - it('should not commit but validate on Enter', async () => { + it('should not commit on Enter', async () => { await sendKeys({ press: 'Enter' }); - expectValidationOnly(); + expectNoValueCommit(); }); it('should not commit on Escape', async () => { From 4606b0ccfbfc0037ae3ae12d29f84172821f00eb Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 4 Oct 2023 17:45:11 +0300 Subject: [PATCH 04/12] fix the code comment --- packages/date-picker/src/vaadin-date-picker-mixin.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js index af5b0e14a7..ffe8c3b6fa 100644 --- a/packages/date-picker/src/vaadin-date-picker-mixin.js +++ b/packages/date-picker/src/vaadin-date-picker-mixin.js @@ -657,21 +657,20 @@ export const DatePickerMixin = (subclass) => } /** - * Depending on the type of value change that has taken place - * since the last commit attempt, triggers validation and fires - * an event: + * Based on the type of value change that has occurred since + * the last commit attempt, triggers validation and fires an event: * * ```text * +--------------------------+-------------------+ * | Type of value change | Event | * +--------------------------+-------------------+ * | empty => parsable | change | + * | empty => unparsable | unparsable-change | * | parsable => empty | change | * | parsable => parsable | change | * | parsable => unparsable | change | - * | empty => unparsable | unparsable-change | * | unparsable => empty | unparsable-change | - * | unparsable => parsable | unparsable-change | + * | unparsable => parsable | change | * | unparsable => unparsable | unparsable-change | * +--------------------------+-------------------+ * ``` From 8be6ae4d3ccf137f3a62b243ba5ea602ed32715c Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 4 Oct 2023 19:22:56 +0300 Subject: [PATCH 05/12] fix code comment --- packages/date-picker/src/vaadin-date-picker-mixin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js index ffe8c3b6fa..cabef23813 100644 --- a/packages/date-picker/src/vaadin-date-picker-mixin.js +++ b/packages/date-picker/src/vaadin-date-picker-mixin.js @@ -657,7 +657,7 @@ export const DatePickerMixin = (subclass) => } /** - * Based on the type of value change that has occurred since + * Depending on the type of value change that has occurred since * the last commit attempt, triggers validation and fires an event: * * ```text From 4bfa6186683360402e6840e67ccc5bd06fc21141 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 4 Oct 2023 19:47:07 +0300 Subject: [PATCH 06/12] make __unparsableValue private --- .../src/vaadin-date-picker-mixin.js | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js index cabef23813..f44632dd09 100644 --- a/packages/date-picker/src/vaadin-date-picker-mixin.js +++ b/packages/date-picker/src/vaadin-date-picker-mixin.js @@ -428,14 +428,17 @@ export const DatePickerMixin = (subclass) => } /** - * The input element's value if the component has been unable to parse it, an empty string otherwise. + * The input element's value if it is unparsable as a date, and an empty string otherwise. + * + * @return {string} + * @private */ - get _unparsableValue() { - if (this._inputElementValue && !this.__parseDate(this._inputElementValue)) { - return this._inputElementValue; + get __unparsableValue() { + if (!this._inputElementValue || this.__parseDate(this._inputElementValue)) { + return ''; } - return ''; + return this._inputElementValue; } /** @@ -687,14 +690,14 @@ export const DatePickerMixin = (subclass) => result = true; this.validate(); this.dispatchEvent(new CustomEvent('change', { bubbles: true })); - } else if (this.__committedUnparsableValue !== this._unparsableValue) { + } else if (this.__committedUnparsableValue !== this.__unparsableValue) { result = true; this.validate(); this.dispatchEvent(new CustomEvent('unparsable-change')); } this.__committedValue = this.value; - this.__committedUnparsableValue = this._unparsableValue; + this.__committedUnparsableValue = this.__unparsableValue; return result; } @@ -950,8 +953,8 @@ export const DatePickerMixin = (subclass) => * an empty string as the value. If no i18n parser is provided, commits * the focused date as the value. * - * @private * @return {boolean} whether there was an actual value change to commit. + * @private */ __commitParsedOrFocusedDate() { let result = false; From a2f522f1b00c949f61eceda79e9424848dd4c090 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Thu, 5 Oct 2023 23:55:02 +0300 Subject: [PATCH 07/12] simplify the solution --- .../src/vaadin-date-picker-mixin.js | 33 +++++-------------- .../value-commit-auto-open-disabled.common.js | 13 +++++--- .../date-picker/test/value-commit.common.js | 13 +++++--- 3 files changed, 25 insertions(+), 34 deletions(-) diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js index f44632dd09..ba66a14d81 100644 --- a/packages/date-picker/src/vaadin-date-picker-mixin.js +++ b/packages/date-picker/src/vaadin-date-picker-mixin.js @@ -428,7 +428,7 @@ export const DatePickerMixin = (subclass) => } /** - * The input element's value if it is unparsable as a date, and an empty string otherwise. + * The input element's value when it cannot be parsed as a date, and an empty string otherwise. * * @return {string} * @private @@ -461,11 +461,11 @@ export const DatePickerMixin = (subclass) => super._onBlur(event); if (!this.opened) { - const didValueCommitOccur = this.__commitParsedOrFocusedDate(); + this.__commitParsedOrFocusedDate(); // Do not validate when focusout is caused by document // losing focus, which happens on browser tab switch. - if (!didValueCommitOccur && document.hasFocus()) { + if (document.hasFocus()) { this.validate(); } } @@ -678,35 +678,25 @@ export const DatePickerMixin = (subclass) => * +--------------------------+-------------------+ * ``` * - * If no value change is detected, the method returns false. - * * @private - * @return {boolean} whether a change was detected and committed. */ __commitValueChange() { - let result = false; - if (this.__committedValue !== this.value) { - result = true; this.validate(); this.dispatchEvent(new CustomEvent('change', { bubbles: true })); } else if (this.__committedUnparsableValue !== this.__unparsableValue) { - result = true; this.validate(); this.dispatchEvent(new CustomEvent('unparsable-change')); } this.__committedValue = this.value; this.__committedUnparsableValue = this.__unparsableValue; - - return result; } /** * Sets the given date as the value and commits it. * * @param {Date} date - * @return {boolean} whether there was an actual value change to commit. * @private */ __commitDate(date) { @@ -716,7 +706,7 @@ export const DatePickerMixin = (subclass) => this.__keepCommittedValue = true; this._selectedDate = date; this.__keepCommittedValue = false; - return this.__commitValueChange(); + this.__commitValueChange(); } /** @private */ @@ -953,12 +943,9 @@ export const DatePickerMixin = (subclass) => * an empty string as the value. If no i18n parser is provided, commits * the focused date as the value. * - * @return {boolean} whether there was an actual value change to commit. * @private */ __commitParsedOrFocusedDate() { - let result = false; - // Select the parsed input or focused date this._ignoreFocusedDateChange = true; if (this.i18n.parseDate) { @@ -966,18 +953,16 @@ export const DatePickerMixin = (subclass) => const parsedDate = this.__parseDate(inputValue); if (parsedDate) { - result = this.__commitDate(parsedDate); + this.__commitDate(parsedDate); } else { this.__keepInputValue = true; - result = this.__commitDate(null); + this.__commitDate(null); this.__keepInputValue = false; } } else if (this._focusedDate) { - result = this.__commitDate(this._focusedDate); + this.__commitDate(this._focusedDate); } this._ignoreFocusedDateChange = false; - - return result; } /** @protected */ @@ -989,7 +974,7 @@ export const DatePickerMixin = (subclass) => } window.removeEventListener('scroll', this._boundOnScroll, true); - const didValueCommitOccur = this.__commitParsedOrFocusedDate(); + this.__commitParsedOrFocusedDate(); if (this._nativeInput && this._nativeInput.selectionStart) { this._nativeInput.selectionStart = this._nativeInput.selectionEnd; @@ -997,7 +982,7 @@ export const DatePickerMixin = (subclass) => // No need to revalidate the value after it has been just committed. // Needed in case the value was not changed: open and close dropdown, // especially on outside click. On Esc key press, do not validate. - if (!didValueCommitOccur && !this.value && !this._keyboardActive) { + if (!this.value && !this._keyboardActive) { this.validate(); } } diff --git a/packages/date-picker/test/value-commit-auto-open-disabled.common.js b/packages/date-picker/test/value-commit-auto-open-disabled.common.js index 7c821cf827..b0ec6d0e07 100644 --- a/packages/date-picker/test/value-commit-auto-open-disabled.common.js +++ b/packages/date-picker/test/value-commit-auto-open-disabled.common.js @@ -16,17 +16,19 @@ describe('value commit - autoOpenDisabled', () => { function expectValueCommit(value) { expect(valueChangedSpy).to.be.calledOnce; - expect(validateSpy).to.be.calledOnce; - expect(validateSpy).to.be.calledAfter(valueChangedSpy); + // TODO: Optimize the number of validation runs. + expect(validateSpy).to.be.called; + expect(validateSpy.firstCall).to.be.calledAfter(valueChangedSpy.firstCall); expect(unparsableChangeSpy).to.be.not.called; expect(changeSpy).to.be.calledOnce; - expect(changeSpy).to.be.calledAfter(validateSpy); + expect(changeSpy.firstCall).to.be.calledAfter(validateSpy.firstCall); expect(datePicker.value).to.equal(value); } function expectUnparsableValueCommit() { expect(valueChangedSpy).to.be.not.called; - expect(validateSpy).to.be.calledOnce; + // TODO: Optimize the number of validation runs. + expect(validateSpy).to.be.called; expect(changeSpy).to.be.not.called; expect(unparsableChangeSpy).to.be.calledOnce; expect(unparsableChangeSpy).to.be.calledAfter(validateSpy); @@ -34,7 +36,8 @@ describe('value commit - autoOpenDisabled', () => { function expectValidationOnly() { expect(valueChangedSpy).to.be.not.called; - expect(validateSpy).to.be.calledOnce; + // TODO: Optimize the number of validation runs. + expect(validateSpy).to.be.called; expect(changeSpy).to.be.not.called; } diff --git a/packages/date-picker/test/value-commit.common.js b/packages/date-picker/test/value-commit.common.js index 447f3fb2cd..9537301b51 100644 --- a/packages/date-picker/test/value-commit.common.js +++ b/packages/date-picker/test/value-commit.common.js @@ -23,17 +23,19 @@ describe('value commit', () => { function expectValueCommit(value) { expect(valueChangedSpy).to.be.calledOnce; - expect(validateSpy).to.be.calledOnce; - expect(validateSpy).to.be.calledAfter(valueChangedSpy); + // TODO: Optimize the number of validation runs. + expect(validateSpy).to.be.called; + expect(validateSpy.firstCall).to.be.calledAfter(valueChangedSpy.firstCall); expect(unparsableChangeSpy).to.be.not.called; expect(changeSpy).to.be.calledOnce; - expect(changeSpy).to.be.calledAfter(validateSpy); + expect(changeSpy.firstCall).to.be.calledAfter(validateSpy.firstCall); expect(datePicker.value).to.equal(value); } function expectUnparsableValueCommit() { expect(valueChangedSpy).to.be.not.called; - expect(validateSpy).to.be.calledOnce; + // TODO: Optimize the number of validation runs. + expect(validateSpy).to.be.called; expect(changeSpy).to.be.not.called; expect(unparsableChangeSpy).to.be.calledOnce; expect(unparsableChangeSpy).to.be.calledAfter(validateSpy); @@ -41,7 +43,8 @@ describe('value commit', () => { function expectValidationOnly() { expect(valueChangedSpy).to.be.not.called; - expect(validateSpy).to.be.calledOnce; + // TODO: Optimize the number of validation runs. + expect(validateSpy).to.be.called; expect(changeSpy).to.be.not.called; } From 4defa9d9522818dea250321ee63a9d062238df74 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Fri, 6 Oct 2023 09:31:13 +0300 Subject: [PATCH 08/12] revert code comment --- packages/date-picker/src/vaadin-date-picker-mixin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js index ba66a14d81..ee67854047 100644 --- a/packages/date-picker/src/vaadin-date-picker-mixin.js +++ b/packages/date-picker/src/vaadin-date-picker-mixin.js @@ -979,7 +979,7 @@ export const DatePickerMixin = (subclass) => if (this._nativeInput && this._nativeInput.selectionStart) { this._nativeInput.selectionStart = this._nativeInput.selectionEnd; } - // No need to revalidate the value after it has been just committed. + // No need to revalidate the value after `_selectedDateChanged` // Needed in case the value was not changed: open and close dropdown, // especially on outside click. On Esc key press, do not validate. if (!this.value && !this._keyboardActive) { From 375df5aa639c65682cc983572eeafc6fcf30495d Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Mon, 9 Oct 2023 16:34:05 +0300 Subject: [PATCH 09/12] improve JSDoc --- .../src/vaadin-date-picker-mixin.js | 26 ++++++++----------- .../date-picker/src/vaadin-date-picker.d.ts | 16 ++++++++++++ .../date-picker/src/vaadin-date-picker.js | 16 ++++++++++++ 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js index ee67854047..44e200eb11 100644 --- a/packages/date-picker/src/vaadin-date-picker-mixin.js +++ b/packages/date-picker/src/vaadin-date-picker-mixin.js @@ -660,23 +660,19 @@ export const DatePickerMixin = (subclass) => } /** - * Depending on the type of value change that has occurred since + * Depending on the nature of value change that has occurred since * the last commit attempt, triggers validation and fires an event: * - * ```text - * +--------------------------+-------------------+ - * | Type of value change | Event | - * +--------------------------+-------------------+ - * | empty => parsable | change | - * | empty => unparsable | unparsable-change | - * | parsable => empty | change | - * | parsable => parsable | change | - * | parsable => unparsable | change | - * | unparsable => empty | unparsable-change | - * | unparsable => parsable | change | - * | unparsable => unparsable | unparsable-change | - * +--------------------------+-------------------+ - * ``` + * Value change | Event + * :------------------------|:------------------ + * empty => parsable | change + * empty => unparsable | unparsable-change + * parsable => empty | change + * parsable => parsable | change + * parsable => unparsable | change + * unparsable => empty | unparsable-change + * unparsable => parsable | change + * unparsable => unparsable | unparsable-change * * @private */ diff --git a/packages/date-picker/src/vaadin-date-picker.d.ts b/packages/date-picker/src/vaadin-date-picker.d.ts index 9b359757b4..1812c09d38 100644 --- a/packages/date-picker/src/vaadin-date-picker.d.ts +++ b/packages/date-picker/src/vaadin-date-picker.d.ts @@ -155,6 +155,22 @@ export interface DatePickerEventMap extends HTMLElementEventMap, DatePickerCusto * * See [Styling Components](https://vaadin.com/docs/latest/styling/styling-components) documentation. * + * ### Change events + * + * Depending on the nature of the value change that the user attempts to commit e.g. by pressing Enter, + * the component can fire either a `change` event or an `unparsable-change` event: + * + * Value change | Event + * :------------------------|:------------------ + * empty => parsable | change + * empty => unparsable | unparsable-change + * parsable => empty | change + * parsable => parsable | change + * parsable => unparsable | change + * unparsable => empty | unparsable-change + * unparsable => parsable | change + * unparsable => unparsable | unparsable-change + * * @fires {Event} change - Fired when the user commits a value change. * @fires {Event} unparsable-change Fired when the user commits an unparsable value change and there is no change event. * @fires {CustomEvent} invalid-changed - Fired when the `invalid` property changes. diff --git a/packages/date-picker/src/vaadin-date-picker.js b/packages/date-picker/src/vaadin-date-picker.js index 5def62320e..29977849c3 100644 --- a/packages/date-picker/src/vaadin-date-picker.js +++ b/packages/date-picker/src/vaadin-date-picker.js @@ -118,6 +118,22 @@ registerStyles('vaadin-date-picker', [inputFieldShared, datePickerStyles], { mod * * See [Styling Components](https://vaadin.com/docs/latest/styling/styling-components) documentation. * + * ### Change events + * + * Depending on the nature of the value change that the user attempts to commit e.g. by pressing Enter, + * the component can fire either a `change` event or an `unparsable-change` event: + * + * Value change | Event + * :------------------------|:------------------ + * empty => parsable | change + * empty => unparsable | unparsable-change + * parsable => empty | change + * parsable => parsable | change + * parsable => unparsable | change + * unparsable => empty | unparsable-change + * unparsable => parsable | change + * unparsable => unparsable | unparsable-change + * * @fires {Event} change - Fired when the user commits a value change. * @fires {Event} unparsable-change Fired when the user commits an unparsable value change and there is no change event. * @fires {CustomEvent} invalid-changed - Fired when the `invalid` property changes. From 67a74e5cf1b7592f99b57bb63f34810812ac7d6d Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Wed, 11 Oct 2023 10:56:59 +0300 Subject: [PATCH 10/12] add more unit tests --- .../date-picker/test/value-commit.common.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/date-picker/test/value-commit.common.js b/packages/date-picker/test/value-commit.common.js index 9537301b51..722ed79627 100644 --- a/packages/date-picker/test/value-commit.common.js +++ b/packages/date-picker/test/value-commit.common.js @@ -157,6 +157,24 @@ describe('value commit', () => { expectValueCommit(''); }); }); + + describe('value set programmatically', () => { + beforeEach(() => { + datePicker.value = TODAY_DATE; + valueChangedSpy.resetHistory(); + validateSpy.resetHistory(); + }); + + it('should not commit but validate on blur', () => { + datePicker.blur(); + expectValidationOnly(); + }); + + it('should not commit on Enter', async () => { + await sendKeys({ press: 'Enter' }); + expectNoValueCommit(); + }); + }); }); describe('unparsable input entered', () => { From 3b84d9e537d1a42b78b1222ddb2d38899230d5c4 Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Thu, 12 Oct 2023 11:33:15 +0300 Subject: [PATCH 11/12] store unparsableValue in a variable --- packages/date-picker/src/vaadin-date-picker-mixin.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js index 44e200eb11..256d89d54d 100644 --- a/packages/date-picker/src/vaadin-date-picker-mixin.js +++ b/packages/date-picker/src/vaadin-date-picker-mixin.js @@ -677,16 +677,18 @@ export const DatePickerMixin = (subclass) => * @private */ __commitValueChange() { + const unparsableValue = this.__unparsableValue; + if (this.__committedValue !== this.value) { this.validate(); this.dispatchEvent(new CustomEvent('change', { bubbles: true })); - } else if (this.__committedUnparsableValue !== this.__unparsableValue) { + } else if (this.__committedUnparsableValue !== unparsableValue) { this.validate(); this.dispatchEvent(new CustomEvent('unparsable-change')); } this.__committedValue = this.value; - this.__committedUnparsableValue = this.__unparsableValue; + this.__committedUnparsableValue = unparsableValue; } /** From 5eade8ca80fa00ee0328e37ceab3ffa425b18f5f Mon Sep 17 00:00:00 2001 From: Sergey Vinogradov Date: Thu, 12 Oct 2023 11:35:59 +0300 Subject: [PATCH 12/12] Update packages/date-picker/src/vaadin-date-picker-mixin.js --- packages/date-picker/src/vaadin-date-picker-mixin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js index 256d89d54d..0e88964fde 100644 --- a/packages/date-picker/src/vaadin-date-picker-mixin.js +++ b/packages/date-picker/src/vaadin-date-picker-mixin.js @@ -660,7 +660,7 @@ export const DatePickerMixin = (subclass) => } /** - * Depending on the nature of value change that has occurred since + * Depending on the nature of the value change that has occurred since * the last commit attempt, triggers validation and fires an event: * * Value change | Event