ui5-datetime-picker: "Error" value-state cleared on change

[ee92]

Bug Description

The datetime picker reverts "Error" value-state back to "None" when a valid date is selected.
This only happens for "Error" and not for "Warning", "Information", or "Success".

Expected Behavior

Expect to be able to set the valueState to "Error" programmatically based on custom validation, without the component reverting it to "None" for what it considers is a valid date.

Steps to Reproduce

Steps to reproduce the behavior:

1. Go to https://codesandbox.io/s/ui5-webcomponents-forked-bp70dk
2. Select a date using the date picker
3. Notice in the console the value-state gets overridden from "Error" to "None"

Context

• UI5 Web Components version: 1.0.1
• Affected component: ui5-date-picker, ui5-datetime-picker

Priority

• Low
• Medium
• High
• Very High

Stakeholder Info (if applicable)

• Organization: SuccessFactors
• Business impact: This is causing an issue in SuccessFactors where a datepicker needs to show an error when the selected datetime is before some other datetime. The lack of error message is leading to customer issues.

Thanks for taking a look!

[MarcusNotheis]

Thanks for reporting! I'll forward this issue to our UI5 Web Components Colleagues as the affected component is developed in their repository.

[ilhan007]

Can you check this issue @SAP/ui5-webcomponents-topic-b it is reproducible although the sample is with the react wrappers. I think it won't take much to re-create it with pure ui5-webcomps.

[eswortmaler]

Here is a demonstration of the issue with pure ui5-webcomponents
https://codesandbox.io/s/ui5-webcomponents-forked-l8e24?file=/src/index.js

The breaking change seems to have been introduced between 1.0.0-rc.15 and 1.0.0-rc.16
The linked example works as expected when choosing @ui5/webcomponents 1.0.0-rc.15

[ee92]

The issue seems to be caused by this line: https://github.com/SAP/ui5-webcomponents/blob/master/packages/main/src/DatePicker.js#L549 (Error value state being changed to None)

Is there any update on a fix?

[tsanislavgatev]

Hello @ee92 ,

Sorry for the late response!

Have you tried to prevent the default behaviour of the control? The change and input events of the DatePicker are preventable, this will prevent the change of the value and the internal verification, so you will have the full control over the value change and the value state change. You will still receive the value in the event and that the value is changed, but if you prevent it, the value and the value state won't be touched by the control itself.

Can you please verify if this will fix your issue?

Best Regards,
Tsanislav

[ee92]

@tsanislavgatev the issue is not fixed by using preventDefault on the event. This line still runs and causes the valueState to be changed to "None" https://github.com/SAP/ui5-webcomponents/blob/master/packages/main/src/DatePicker.js#L558

[tsanislavgatev]

Screen.Recording.2022-04-27.at.10.48.42.mov

@ee92 Hello,
I've made a quick recording of both cases.

The method you are refering to is called after we check if the event is prevented.

ui5-webcomponents/packages/main/src/DatePicker.js

Line 540 in 532d676

if (!executeEvent) {

And then called here:

ui5-webcomponents/packages/main/src/DatePicker.js

Line 549 in 532d676

this._updateValueState(); // Change the value state to Error/None, but only if needed

Can you please check the behaviour again?
And if still failing, to provide an example again so I can investigate.

Best Regards,
Tsanislav

[ee92]

@tsanislavgatev

This is true for DatePicker, but not for DateTimePicker. I updated the ticket description with a new sandbox that uses DateTimePicker instead.

Just by looking at the code you can see this line runs unconditionally:

ui5-webcomponents/packages/main/src/DateTimePicker.js

Line 373 in 3236d56

this._updateValueState();

We had no issue with this previous to upgrading to 1.x.x so hoping that the solution does not require preventing the event.

Thanks

[tsanislavgatev]

Hello,

The change that introduced the secondary check is to improve the checks that the component provides. The control itself provides a bahaviour with predefined validations of dates, ranges, formats of date/time. It comes by default with the control, that's why you receive this removing of the value state. Because of this we've introduced the preventable behaviour, for the developers who don't need our checks, to give them the opportunity to implement their validation and value setting. That's why we are talking about preventing the default behaviour and why we decided to provide a full control option.

Best Regards,
Tsanislav