Entering invalid value to DatePicker does not show DatePicker as invalid in browser

[tepi]

To reproduce the issue:

1. Create a new DatePicker
2. Enter "jksghk" and blur the DatePicker
3. DatePicker looks valid

Additionally, there is no easy way to validate invalid input from Java side and set the DatePicker into invalid mode.

[yuriy-fix]

Requires complex changes both on the components and flow counterparts logic as well as to the flow Binder behavior: https://github.com/vaadin/components-team-tasks/issues/489

[alvarezguille]

More info in epic in https://github.com/vaadin/components-team-tasks/issues/528

[tomivirkki]

Could be fixed by synchronizing the _inputValue property on event X and then setting the validity server side based on the property value and value of the component; The component should be invalid when the value is null but there is an input value. Should also validate on blur (and run the Binder validation, same as Value change?).

This isn't related to Binder necessarily. Like other component validators, this will not work with Binder before getDefaultValidator API is implemented (another issue).

[pekam]

The validation at Java side has so far been a function of the server-side value, executed on ValueChangeEvent. To resolve this issue, it should be a function of the server-side value AND the input's string-value (web component's _inputValue).

There is no 1-to-1 relationship between the server-side value and _inputValue: value null can correspond to _inputValue="" or _inputValue="asdf" (or any other string that can't be resolved to a date). This means, that it's not enough to run the validation on ValueChangeEvent: When the _inputValue changes between empty string and invalid date string, the validation status should be updated, but ValueChangeEvent is not fired, because the value remains null during these changes.

So there are two problems to solve:

• _inputValue should be synchronized to server-side
• Validation should also run on blur events

Also, we should keep in mind that similar fix migh be needed for other components (at least IntegerField and BigDecimalField, where non-parseable numbers are converted to null server-side value).

Syncing the _inputValue

We don't want to synchronize on the text-field's value-changed-event, because that would create network traffic for every keypress.
The two cases when we want to update the input value are the same as when we want to validate: date picker's value-changed and blur:

datepickerConnector.js:

datepicker.addEventListener('value-changed',
    e => datepicker.dispatchEvent(new CustomEvent('input-value-changed')));
datepicker.addEventListener('blur',
    e => datepicker.dispatchEvent(new CustomEvent('input-value-changed')));

DatePicker.java:

@Synchronize(property = "_inputValue", value = "input-value-changed")
private String getInputValue() {
    return getElement().getProperty("_inputValue", "");
}

Running validation on blur

On component level it's trivial:

addBlurListener(e -> validate());

The problem is when you use this together with Binder validators.

Once we implement HasValidator::getDefaultValidator, Binder will take also this new _inputValue validation into account, but Binder is still validating only on ValueChangeEvents.

So there would still be a bug in this scenario:

1. datepicker is configured as required through Binder
2. input value is changed to "asdf" and field is blurred -> value is null and the field is invalid (via component's blur listener and internal validation)
3. input value is changed to "" and field is blurred -> only component-level validation runs as there's no value-change to trigger Binder validation, field becomes valid although the field was set required through Binder

The logical solution would be, that if the component validates on blur events, Binder should as well.

Proposal to Flow: Change Binder to also validate on blur if field implements BlurNotifier.

• This would also introduce the validate-after-first-visit -behavior that we have on our web components to Flow users (eg. when you visit an empty field without changing the value)
  • Probably a good thing, since that's how our components have been designed to work?
  • Behavior is changing - can it be done in a patch release? Is it a breaking change?
• Possibly a problem for components implementing HasValueChangeMode: With value-change-modes LAZY and TIMEOUT, the value is synchronized after a delay, so at the time of the blur event, server-side may have the old value.
  • As described here, it might be a good idea to sync value on blur also with lazy and timeout modes for other reasons, which would also solve this problem.

Another way to approach this would be to somehow notify from the component to the Binder that validators should run.

• Eg. HasValidator::addValidationListener
• Or with a different kind of approach: HasValidator::getEventsToValidate
• It could also be in eg. HasValue and have a default implementation for notifying only on ValueChangeEvent.
• As a downside, this approach would expose public API that shouldn't really be visible to users, just because it needs to be visible to Binder (similarly to the existing HasValidator::getDefaultValidator method).
• Another wild idea, to avoid adding public methods: optional annotation on the component-class, eg. @ValidationEvents({ValueChangeEvent.class, BlurEvent.class})

[pekam]

After releasing a web component fix that blocks this, I'll open a fixing PR for DatePicker (adding the logic to component-level validation).

After this change, the component should validate correctly when NOT using Binder.

To validate correctly in all cases including Binder, two more steps are needed:

1. Binder should validate on blur: Binder should always validate after field's internal validation flow#8242
   (this one has a simple workaround: datePicker.addBlurListener(e -> binder.validate())
2. DatePicker should implement getDefaultValidator() (requires bumping the minor version)

If/when the upcoming DatePicker PR is released, but neither of the above-mentioned steps are, the following problems will occur when using Binder:

• Whenever the value changes, only Binder validation is effective and component validation dismissed
  • this is an old issue, but it's important to note that the new fix is dismissed as well, when you're changing from a non-null value to random input like "asdf" (of course it's still marked as invalid if Binder doesn't allow null values)
• Focusing and blurring the field without changing the value will trigger only the component's internal validation, dismissing Binder validation (REGRESSION)

If/when step 1 is implemented and step 2 not, the following problem will occur when using Binder:

• This new new fix (invalidate-unparseable-date-string) won't work at all, similarly to the component's validation constraints
  • Binder will always validate after the component has validated, but the Binder doesn't take component's validation implementation into account due to missing step 2.

To avoid introducing a regression, and to make this fix and the existing component-level validation work with Binder, I'd vote for releasing all of these changes in the same platform version. Since step 2 needs a minor bump, this would mean postponing the fix until Vaadin 14.3.

[pekam]

Blocked by vaadin/flow#8242
Binder validating on blur did not sound like a good idea for Flow team. There will be probably instead some kind of addValidationStatusListener in the HasValidator interface to allow components to notify Binder about re-validation.

Here's a branch with the changes excluding the implementation of the not-yet-existing event: https://github.com/vaadin/vaadin-date-picker-flow/tree/invalid-date-string-fix

[nittka]

#1778 duplicates this issue

[SonReimer]

Will this be available as a (what I think) necessary feature in LTS version 23 ?

[hfazai]

I'm using this workaround to fix the issue:

val datePicker = DatePicker()

datePicker.element.executeJs(
             """
              this.addEventListener("blur", event => {
                if (this.value == '' && this.$.input.inputElement.value != '') {
                  this.$.input.inputElement.value = ''
                }
              })
              """
)

The value of the field is cleared when it is invalid.

[nittka]

Thanks for sharing the snippet. It gives feedback to the user - indicating that something is wrong, but it is not perfect from a user experience perspective: "I entered a value and it is gone..."

The latest date picker version supports custom date formats (primary format is displayed, others are used for alternative parsing) which solves one of our problems (user enters date in other format). THANKS!!

DatePicker dp = new DatePicker();
DatePickerI18n i18n = new DatePickerI18n();
i18n.setDateFormats("yyyy-MM-dd", "yy-MM-dd", "dd.MM.yy", "dd.MM.yyyy");
dp.setI18n(i18n);

For required date fields, this is OK. A date can be entered in different formats (e.g. copy/paste). If there are format problems the date value is null, the text is still visible and the field is marked as having an error.
For optional date fields the initial problem still persists. I will consider your workaround - maybe rather prefixing the input value with "ERROR" or something.
(Right now we use a wrapper component, replacing the datepicker's text field with a regular text field and the two components inform each other about changes. This is highly fragile and not satisfactory.)

I still consider this a major bug in a fundamental component and hope for a fix.

[hfazai]

@nittka You are right, my workaround doesn't provide a good UX.
Are you using vaadin-date-picker-light as a wrapper of a text field?

[nittka]

No, we wrote our own (java) component, a layout containing textfield and (default) picker, adapting the styling so that the picker's input field is not shown.

[vursen]

FYI: We have just introduced new validation behavior that is supposed to solve many different validation issues that people were complaining about, including this one. The new behavior is available under the feature flag in v23.2 for several components including DatePicker.

To enable the feature flag in your application, add the following to vaadin-featureflags.properties:

com.vaadin.experimental.enforceFieldValidation=true

Any feedback would be highly appreciated. For more details, please refer to vaadin/platform#3066.

[cdir]

I would say it works partially or I haven't quite figured out how it's supposed to work yet.

If I use a Binder and a DatePicker and simply set an object via setBean(...), the field becomes invalid as soon as I enter an invalid value. However, nothing is written to the model - I would have expected null to be written to the model. When I then call validate on the binder, it reports OK. If I enter a different value afterwards, the field remains invalid.

However, I don't use a binder in my framework, so it's quite possible that I used it incorrectly in my little test.

Without a binder (in my framework), I can register a ClientValidatedEventListener (addClientValidatedEventListener). This is correctly notified with the feature switch enabled when an invalid value is entered in the UI. (I write null in my model in this case.) However, if I subsequently change the value back to the last valid value, I don't get an event and the field remains invalid. For example: I enter "01/01/2022". I then enter "a01/01/2022" (field becomes invalid). Then I correct the value and remove the 'a', the field remains invalid and I get no event on the server.

I would expect the internal value of the field to be set to null as soon as an invalid entry is made. The incorrectly entered text should remain, otherwise the user will not be able to understand their mistake. However, the field should not simply assume that the last valid value is still the current value.

[vursen]

I would expect the internal value of the field to be set to null as soon as an invalid entry is made.

Yes, that's how it should work and that's actually how it works for me (see the example below). I tried it both with a binder and without. I wonder what I'm missing 🤔

Screen.Recording.2022-10-05.at.17.02.22.mov

@Route(value = "date-picker")
public class DatePickerView extends Div {
    public DatePickerView() {
        Div value = new Div();

        DatePicker datePicker = new DatePicker();
        datePicker.addValueChangeListener(event -> {
            value.setText("Value: " + datePicker.getValue());
        });

        add(datePicker, value);
    }
}

P.S When I type in a1/1/2022, DatePicker automatically corrects the input to 1/1/2022 which causes it to be valid as expected. Doesn't that work the same way for you?

[cdir]

Okay, that's weird. The difference:

datePicker.setAutoOpen(false);

I hate that the picker always opens when I tab through my application, so I always set auto open to false. If I remove this line, it works. Please tell me how auto open correlates with validation behavior.

[jouni]

I hate that the picker always opens when I tab through my application,

That’s not how it is supposed to work. “Auto open” should only affect “click to focus”, not “tab to focus”. Typing a value to the field does open the popup, though. I assume that’s what you mean, that you tab through the app and type values to the fields.

[cdir]

Ok, that's right. Maybe it was different in earlier versions (10 or 14). But it remains annoying, because after entering a date I can't just tab to the next field but have to confirm the date with enter first.

But apart from that, I don't understand why the field behaves differently when I disable auto-open. This is still a bug, isn't it?

[cdir]

Is it possible that the bug will be fixed? I would like to use the validation but leave auto-open at false.

[vursen]

Is it possible that the bug will be fixed? I would like to use the validation but leave auto-open at false.

Hey @cdir! Thank you for your report. We have fixed this bug. Please, follow V24.0.0.alpha6 which will be out soon.

[cdir]

We definitely need a fix in Vaadin 23!

[vursen]

We definitely need a fix in Vaadin 23!

@cdir I apologize for the confusion. It is already available there. Please, check out V23.3.0.beta2

[cdir]

@vursen I just tried vaadin 23.3. Unfortunately I still only get an error when autoOpen=true is set.

[vursen]

Unfortunately I still only get an error when autoOpen=true is set.

What kind of error do you get? Is it the same problem? Or is it different now?

[cdir]

I get no error. There is just no value change event when autoOpen=false while it is triggered with autoOpen=true. So, no different behavior.

[cdir]

@vursen sorry for the confusion. I found the problem, there was a version mismatch in my sample. Now it works as expected. Thank you very much.

[vursen]

I found the problem, there was a version mismatch in my sample

I'm glad it worked out. Thank you for testing it.

[simasch]

The new feature flag solves this problem but adds another:
The validator is always called twice, and when doing some backend validation with the database, this can cause issues.

My situation:

• binder.setBean is called this executes the validation
• after that the validationStatusChanged event triggers the validation again

[vursen]

Hello @simasch!

Thank you for your report. It is a sort of known issue for us. However, we didn't have a ticket for that, so there it is: #4390

[SonReimer]

@TatuLund :
Enabling feature
com.vaadin.experimental.enforceFieldValidation=true
things work for me as expected
regardless of mentioned #4390 and #4352

[vursen]

The issue has been resolved by the new validation mechanism which is enabled by default in Vaadin 24. For any further bugs, please create separate issues.