feat!: datetimepicker validation improvements (#8986)

ugur-vaadin · vursen · web-flow · commit 0770cde77ab7 · 2025-05-05T20:58:05.000+03:00
* feat: datetimepicker validation improvements * test: simplify tests and reduce the number * test: separate parametric tests * test: extract value commit tests into a separate file * refactor: always validate on blur * Update packages/date-time-picker/src/vaadin-date-time-picker-mixin.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/src/vaadin-date-time-picker-mixin.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/src/vaadin-date-time-picker.d.ts Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * chore: add incomplete cases to the change event list * fix: always validate on overlay closed by outside click * refactor: debounce value commit and refactor all tests * Update packages/date-time-picker/src/vaadin-date-time-picker-mixin.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/test/value-commit.test.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/test/value-commit.test.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/test/value-commit.test.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/test/value-commit.test.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/test/value-commit.test.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/test/value-commit.test.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/src/vaadin-date-time-picker-mixin.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/test/value-commit.test.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/test/value-commit.test.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * Update packages/date-time-picker/test/value-commit.test.js Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com> * refactor: revert adding debouncer for value commit * refactor: use microtask in outside click flag * test: add missing async * Revert "refactor: use microtask in outside click flag" This reverts commit 927bb35. * test: update assertions * test: fix tabbing related tests for firefox * test: use send mouse to element click in order to fix webkit tests * refactor: call validation on commit value change * test: add tests for required but initially empty pickers * test: move individual lines to beforeeach --------- Co-authored-by: Sergey Vinogradov <mr.vursen@gmail.com>
diff --git a/packages/date-time-picker/src/vaadin-date-time-picker-mixin.js b/packages/date-time-picker/src/vaadin-date-time-picker-mixin.js
@@ -305,6 +305,7 @@ export const DateTimePickerMixin = (superClass) =>
       // Default value for "max" property of vaadin-time-picker (for removing constraint)
       this.__defaultTimeMaxValue = '23:59:59.999';
 
+      this.__onGlobalClick = this.__onGlobalClick.bind(this);
       this.__changeEventHandler = this.__changeEventHandler.bind(this);
       this.__valueChangedEventHandler = this.__valueChangedEventHandler.bind(this);
       this.__openedChangedEventHandler = this.__openedChangedEventHandler.bind(this);
@@ -315,12 +316,38 @@ export const DateTimePickerMixin = (superClass) =>
       return [this.__datePicker, this.__timePicker];
     }
 
+    /** @private */
+    get __filledPickers() {
+      return this.__pickers.filter((picker) => picker.value || picker.__unparsableValue);
+    }
+
     /** @private */
     get __formattedValue() {
       const values = this.__pickers.map((picker) => picker.value);
       return values.every(Boolean) ? values.join('T') : '';
     }
 
+    /**
+     * Values:
+     * - ""
+     * - "fooT"
+     * - "Tbar"
+     * - "fooTbar"
+     * - "T12:00"
+     * - "fooT12:00"
+     * - "2024-01-01T"
+     * - "2024-01-01Tbar"
+     *
+     * @private
+     */
+    get __unparsableValue() {
+      if (this.__filledPickers.length > 0 && !this.__pickers.every((picker) => picker.value)) {
+        return this.__pickers.map((picker) => picker.value || picker.__unparsableValue).join('T');
+      }
+
+      return '';
+    }
+
     /** @protected */
     ready() {
       super.ready();
@@ -347,12 +374,49 @@ export const DateTimePickerMixin = (superClass) =>
       this.ariaTarget = this;
     }
 
+    /** @protected */
+    connectedCallback() {
+      super.connectedCallback();
+      document.addEventListener('click', this.__onGlobalClick, true);
+    }
+
+    /** @protected */
+    disconnectedCallback() {
+      super.disconnectedCallback();
+      document.removeEventListener('click', this.__onGlobalClick, true);
+    }
+
     focus() {
       if (this.__datePicker) {
         this.__datePicker.focus();
       }
     }
 
+    /** @private */
+    __onGlobalClick(event) {
+      const isOpened = this.__datePicker.opened || this.__timePicker.opened;
+      if (!isOpened) {
+        return;
+      }
+
+      const isOutsideClick = event.composedPath().every((node) => {
+        return ![
+          this.__datePicker,
+          this.__datePicker.$.overlay,
+          this.__timePicker,
+          this.__timePicker.$.comboBox.$.overlay,
+        ].includes(node);
+      });
+
+      if (isOutsideClick) {
+        this.__outsideClickInProgress = true;
+
+        setTimeout(() => {
+          this.__outsideClickInProgress = false;
+        });
+      }
+    }
+
     /**
      * Override method inherited from `FocusMixin` to validate on blur.
      * @param {boolean} focused
@@ -365,7 +429,7 @@ export const DateTimePickerMixin = (superClass) =>
       // Do not validate when focusout is caused by document
       // losing focus, which happens on browser tab switch.
       if (!focused && document.hasFocus()) {
-        this._requestValidation();
+        this.__commitPendingValueChange();
       }
     }
 
@@ -379,12 +443,12 @@ export const DateTimePickerMixin = (superClass) =>
      */
     _shouldRemoveFocus(event) {
       const target = event.relatedTarget;
-      const overlayContent = this.__datePicker._overlayContent;
 
       if (
+        this.__datePicker.opened ||
+        this.__timePicker.opened ||
         this.__datePicker.contains(target) ||
-        this.__timePicker.contains(target) ||
-        (overlayContent && overlayContent.contains(target))
+        this.__timePicker.contains(target)
       ) {
         return false;
       }
@@ -408,29 +472,43 @@ export const DateTimePickerMixin = (superClass) =>
     __changeEventHandler(event) {
       event.stopPropagation();
 
-      if (this.__dispatchChangeForValue === this.value) {
-        this._requestValidation();
-        this.__dispatchChange();
+      const isAlreadyInvalid = this.invalid;
+      const filledPickers = this.__filledPickers;
+      if (filledPickers.length === 1 && filledPickers[0].checkValidity() && !isAlreadyInvalid) {
+        // Skip if (a) only one picker is filled, (b) its value is valid by itself, and (c) the user
+        // is still interacting with the field. This is to give the user a chance to finish the input
+        // before giving him feedback. However, if the field is already in the invalid state due to
+        // a previous error, proceed to committing the value to get the error message updated.
+        return;
+      }
+
+      if (this.__hasPendingValueChange) {
+        this.__commitPendingValueChange();
       }
-      this.__dispatchChangeForValue = undefined;
     }
 
     /** @private */
     __openedChangedEventHandler() {
       const opened = this.__datePicker.opened || this.__timePicker.opened;
       this.style.pointerEvents = opened ? 'auto' : '';
+
+      if (!opened && this.__outsideClickInProgress) {
+        this.__commitPendingValueChange();
+      }
     }
 
     /** @private */
     __addInputListeners(node) {
       node.addEventListener('change', this.__changeEventHandler);
+      node.addEventListener('unparsable-change', this.__changeEventHandler);
       node.addEventListener('value-changed', this.__valueChangedEventHandler);
       node.addEventListener('opened-changed', this.__openedChangedEventHandler);
     }
 
     /** @private */
     __removeInputListeners(node) {
       node.removeEventListener('change', this.__changeEventHandler);
+      node.removeEventListener('unparsable-change', this.__changeEventHandler);
       node.removeEventListener('value-changed', this.__valueChangedEventHandler);
       node.removeEventListener('opened-changed', this.__openedChangedEventHandler);
     }
@@ -724,8 +802,27 @@ export const DateTimePickerMixin = (superClass) =>
      */
     checkValidity() {
       const hasInvalidPickers = this.__pickers.some((picker) => !picker.checkValidity());
+      const hasOnlyOneFilledPicker = this.__filledPickers.length === 1;
       const hasEmptyRequiredPickers = this.required && this.__pickers.some((picker) => !picker.value);
-      return !hasInvalidPickers && !hasEmptyRequiredPickers;
+      return !hasInvalidPickers && !hasEmptyRequiredPickers && !hasOnlyOneFilledPicker;
+    }
+
+    /** @private */
+    __commitPendingValueChange() {
+      this._requestValidation();
+      if (this.__committedValue !== this.value) {
+        this.dispatchEvent(new CustomEvent('change', { bubbles: true }));
+      } else if (this.__committedUnparsableValue !== this.__unparsableValue) {
+        this.dispatchEvent(new CustomEvent('unparsable-change'));
+      }
+
+      this.__committedValue = this.value;
+      this.__committedUnparsableValue = this.__unparsableValue;
+    }
+
+    /** @private */
+    get __hasPendingValueChange() {
+      return this.__committedValue !== this.value || this.__committedUnparsableValue !== this.__unparsableValue;
     }
 
     /**
@@ -769,8 +866,9 @@ export const DateTimePickerMixin = (superClass) =>
     __valueChanged(value, oldValue) {
       this.__handleDateTimeChange('value', '__selectedDateTime', value, oldValue);
 
-      if (oldValue !== undefined) {
-        this.__dispatchChangeForValue = value;
+      if (!this.__keepCommittedValue) {
+        this.__committedValue = value;
+        this.__committedUnparsableValue = '';
       }
 
       this.toggleAttribute('has-value', !!value);
@@ -839,8 +937,12 @@ export const DateTimePickerMixin = (superClass) =>
       }
 
       this.__ignoreInputValueChange = true;
+      this.__keepCommittedValue = true;
+
       this.__updateTimePickerMinMax();
       this.value = this.__formattedValue;
+
+      this.__keepCommittedValue = false;
       this.__ignoreInputValueChange = false;
     }
 
diff --git a/packages/date-time-picker/src/vaadin-date-time-picker.d.ts b/packages/date-time-picker/src/vaadin-date-time-picker.d.ts
@@ -93,7 +93,30 @@ export interface DateTimePickerEventMap extends DateTimePickerCustomEventMap, HT
  *
  * 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
+ * incomplete => empty      | unparsable-change
+ * incomplete => parsable   | change
+ * incomplete => unparsable | unparsable-change
+ * empty => incomplete      | unparsable-change
+ * parsable => incomplete   | change
+ * unparsable => incomplete | unparsable-change
+ *
  * @fires {Event} change - Fired when the user commits a value change.
+ * @fires {Event} unparsable-change - Fired when the user commits an unparsable or incomplete value change and there is no change event.
  * @fires {CustomEvent} invalid-changed - Fired when the `invalid` property changes.
  * @fires {CustomEvent} value-changed - Fired when the `value` property changes.
  * @fires {CustomEvent} validated - Fired whenever the field is validated.
diff --git a/packages/date-time-picker/src/vaadin-date-time-picker.js b/packages/date-time-picker/src/vaadin-date-time-picker.js
@@ -64,7 +64,30 @@ registerStyles('vaadin-date-time-picker', inputFieldShared, { moduleId: 'vaadin-
  *
  * 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
+ * incomplete => empty      | unparsable-change
+ * incomplete => parsable   | change
+ * incomplete => unparsable | unparsable-change
+ * empty => incomplete      | unparsable-change
+ * parsable => incomplete   | change
+ * unparsable => incomplete | unparsable-change
+ *
  * @fires {Event} change - Fired when the user commits a value change.
+ * @fires {Event} unparsable-change - Fired when the user commits an unparsable or incomplete value change and there is no change event.
  * @fires {CustomEvent} invalid-changed - Fired when the `invalid` property changes.
  * @fires {CustomEvent} value-changed - Fired when the `value` property changes.
  * @fires {CustomEvent} validated - Fired whenever the field is validated.
diff --git a/packages/date-time-picker/test/basic.test.js b/packages/date-time-picker/test/basic.test.js
@@ -247,53 +247,6 @@ describe('Basic features', () => {
     });
   });
 
-  describe('change event', () => {
-    let spy;
-
-    beforeEach(() => {
-      spy = sinon.spy();
-      dateTimePicker.addEventListener('change', spy);
-    });
-
-    it('should fire change on date picker change event', () => {
-      changeInputValue(timePicker, '16:00');
-      expect(spy.called).to.be.false;
-      changeInputValue(datePicker, '2020-01-17');
-      expect(spy.calledOnce).to.be.true;
-      changeInputValue(datePicker, '');
-      expect(spy.calledTwice).to.be.true;
-    });
-
-    it('should fire change on time picker change event', () => {
-      changeInputValue(datePicker, '2020-01-17');
-      expect(spy.called).to.be.false;
-      changeInputValue(timePicker, '16:00');
-      expect(spy.calledOnce).to.be.true;
-      changeInputValue(timePicker, '');
-      expect(spy.calledTwice).to.be.true;
-    });
-
-    it('should not fire change on programmatic value change', () => {
-      dateTimePicker.value = '2020-01-17T16:00';
-      expect(spy.called).to.be.false;
-    });
-
-    it('should not fire change on programmatic value change after manual one', () => {
-      dateTimePicker.value = '2020-01-17T16:00'; // Init with valid value
-      changeInputValue(datePicker, '2020-01-20');
-      spy.resetHistory();
-      dateTimePicker.value = '2020-01-10T12:00';
-      expect(spy.called).to.be.false;
-    });
-
-    it('should not fire change on programmatic value change after partial manual one', () => {
-      changeInputValue(datePicker, '2020-01-17');
-      // Time picker has no value so date time picker value is still empty
-      dateTimePicker.value = '2020-01-17T16:00';
-      expect(spy.called).to.be.false;
-    });
-  });
-
   describe('value property formats', () => {
     it('should accept ISO format', () => {
       const date = new Date(Date.UTC(0, 1, 3, 8, 30, 0));
diff --git a/packages/date-time-picker/test/validation.test.js b/packages/date-time-picker/test/validation.test.js
diff --git a/packages/date-time-picker/test/value-commit.test.js b/packages/date-time-picker/test/value-commit.test.js
