feat: date time picker validation improvements #7346

ugur-vaadin · 2025-04-24T20:19:30Z

Description

This PR implements DateTimePicker validation improvements for the Flow counterpart. The changes include:

Use the new unparsable-change event from the web component
Replace the validated event
Prevent multiple validations during a single round-trip
Improve validation consistency similar to the web component
Postpone validation during navigation between DP and TP unless we are certain the value will be invalid (unless it is invalid or out of the range set using mix/max)
Introduce a dedicated error message for incomplete values
Handle the input element value changes caused by programmatically setting values

Based on the prototype.

Depends on vaadin/web-components#8986.

Part of #6697

Type of change

Bugfix
Feature

Checklist

I have read the contribution guide: https://vaadin.com/docs/latest/contributing/overview
I have added a description following the guideline.
The issue is created in the corresponding repository and I have referenced it.
I have added tests to ensure my change is effective and works as intended.
New and existing tests are passing locally with my change.
I have performed self-review and corrected misspellings.
Enhancement / new feature was discussed in a corresponding GitHub issue and Acceptance Criteria were created.

vursen · 2025-04-29T14:55:27Z

...-time-picker-flow/src/main/java/com/vaadin/flow/component/datetimepicker/DateTimePicker.java

+        // Notify the server in order to use the formatted values in validation
+        pendingInputElementValueSyncs++;
+        getElement().executeJs(
+                "this.dispatchEvent(new CustomEvent('value-programmatically-set'));");


Could you please explain why this additional event is necessary? What scenario is it intended to cover?

This approach might not be correct since isInvalid() will still report true on the server side until the next round-trip occurs. I explored this a bit on my own and may have found a solution that avoids a round-trip: cbd6285. Please let me know if you see any issues with it.

This solution is quite good overall. Updated the PR based in this approach and applied a few fixes.

vursen · 2025-05-06T03:48:52Z

...sts/src/test/java/com/vaadin/flow/component/datetimepicker/validation/BasicValidationIT.java

+    @Test
+    public void setValueProgrammatically_fieldValidatedOnce() {
+        clickElementWithJs(SET_VALUE_PROGRAMMATICALLY);
+        assertValidationCount(1);
+    }
+
+    @Test
+    public void clearAndSetValueProgrammatically_fieldValidatedTwice() {
+        clickElementWithJs(CLEAR_AND_SET_VALUE_PROGRAMMATICALLY);
+        assertValidationCount(2);
+    }


This could be tested using unit tests in datetimepicker/validation/BasicValidationTest.java

Removed from ITs and converted to unit tests.

vursen · 2025-05-06T03:52:25Z

...sts/src/test/java/com/vaadin/flow/component/datetimepicker/validation/BasicValidationIT.java

+    public void triggerBlurWithNoChange_fieldNotValidated() {
+        dateInput.sendKeys(Keys.TAB);
+        timeInput.sendKeys(Keys.TAB);
+        assertValidationCount(0);


I suggest combining the tests that assert validity with those that assert the validation count. The validation count should be asserted in every test that verifies validity

Added validation count assertions.

vursen · 2025-05-06T03:55:11Z

...-time-picker-flow/src/main/java/com/vaadin/flow/component/datetimepicker/DateTimePicker.java

+                && value == null
+                && (isInputUnparsable() || isInputIncomplete());
+        synchronizeChildComponentValues(value);
+        validate(shouldFireValidationStatusChangeEvent);


Components must be validated before user-defined ValueChangeEvent listeners are triggered to ensure that those listeners have access to the updated invalid state. That's why validate() was originally called inside addValueChangeListener.

Changing the order seems to handle this issue. Added a unit test that should cover this case.

I'm not sure... it looks like validation is now running with the previous value.

You are absolutely right. The test passing was a coincidence. I reverted the change and replaced change event listener with value change listener.

vursen · 2025-05-06T03:59:39Z

...-time-picker-flow/src/main/java/com/vaadin/flow/component/datetimepicker/DateTimePicker.java

@@ -327,6 +353,11 @@ public DateTimePicker(LocalDateTime initialDateTime, Locale locale) {
        setLocale(locale);
    }

+    private void addValidationListeners() {
+        getElement().addEventListener("change", e -> validate(true));


Calling validate(true) on change will result in duplicate validation when using Binder, since Binder already listens to ValueChangeEvent and triggers validation automatically. To prevent similar regressions in the future, each test in BinderValidationIT should assert the validation count.

The validation count seems to be in line with regular validation in my testing. Can you provide some example cases where the binder validation count is doubled?

vursen · 2025-05-06T04:03:15Z

...sts/src/test/java/com/vaadin/flow/component/datetimepicker/validation/BasicValidationIT.java

@@ -193,7 +211,7 @@ public void setValue_clearValue_assertValidity() {

    @Test
    public void badInput_changeValue_assertValidity() {
-        setInputValue(dateInput, "INVALID");
+        setFieldInvalid();


Let's use explicit setInputValue calls throughout to avoid potential side-effects and make the test flow more obvious.

Updated to set the values explicitly.

vursen · 2025-05-06T04:04:44Z

...ts/src/test/java/com/vaadin/flow/component/datetimepicker/validation/BinderValidationIT.java

-        assertErrorMessage(REQUIRED_ERROR_MESSAGE);
+        assertServerValid();
+        assertClientValid();
+        assertErrorMessage(null);


Each test case should also assert the validation count.

Added count assertions.

…nit tests

sonarqubecloud · 2025-05-07T17:02:48Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
6.8% Duplication on New Code

See analysis details on SonarQube Cloud

ugur-vaadin force-pushed the feat-datetimepicker-validation-improvements branch from 2a8f775 to 548332e Compare April 24, 2025 20:20

ugur-vaadin marked this pull request as ready for review April 25, 2025 07:07

ugur-vaadin requested review from vursen and yuriy-fix April 25, 2025 07:14

vursen reviewed Apr 29, 2025

View reviewed changes

ugur-vaadin force-pushed the feat-datetimepicker-validation-improvements branch from 4faeebe to 0fc0548 Compare May 5, 2025 17:58

vursen reviewed May 6, 2025

View reviewed changes

ugur-vaadin force-pushed the feat-datetimepicker-validation-improvements branch from 000387f to cb34131 Compare May 7, 2025 11:43

ugur-vaadin added 9 commits May 7, 2025 19:40

feat: date time picker validation improvements

9de03e8

refactor: add more tests and keep number of programmatically set values

6715f9d

refactor: avoid roundtrip and use input data from pickers

c24fb6f

chore: add deprecated message

f2b390a

test: convert validation on programmatically changed value tests to u…

0cbc651

…nit tests

test: assert validation count on validation result tests

64f24f8

test: add validation count assertions to binder validation tests

8465a48

fix: provide correct validation state in value change listener

ee5de4a

refactor: revert set value order change and validate on value change

e8df2b8

ugur-vaadin force-pushed the feat-datetimepicker-validation-improvements branch from cb34131 to e8df2b8 Compare May 7, 2025 16:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: date time picker validation improvements #7346

feat: date time picker validation improvements #7346

ugur-vaadin commented Apr 24, 2025 •

edited

Loading

vursen Apr 29, 2025

vursen Apr 29, 2025

ugur-vaadin May 5, 2025

vursen May 6, 2025

ugur-vaadin May 6, 2025

vursen May 6, 2025

ugur-vaadin May 6, 2025

vursen May 6, 2025 •

edited

Loading

ugur-vaadin May 6, 2025

vursen May 7, 2025 •

edited

Loading

ugur-vaadin May 7, 2025

vursen May 6, 2025

ugur-vaadin May 6, 2025

vursen May 6, 2025 •

edited

Loading

ugur-vaadin May 6, 2025

vursen May 6, 2025

ugur-vaadin May 6, 2025

sonarqubecloud bot commented May 7, 2025

feat: date time picker validation improvements #7346

Are you sure you want to change the base?

feat: date time picker validation improvements #7346

Conversation

ugur-vaadin commented Apr 24, 2025 • edited Loading

Description

Type of change

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vursen May 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vursen May 7, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vursen May 6, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented May 7, 2025

Quality Gate passed

ugur-vaadin commented Apr 24, 2025 •

edited

Loading

vursen May 6, 2025 •

edited

Loading

vursen May 7, 2025 •

edited

Loading

vursen May 6, 2025 •

edited

Loading