[SF][DataPicker] - DataPicker's internal validation reset valueState unexpectedly

[2bno1]

Bug Description

The DatePicker component has built-in validation (https://github.com/SAP/ui5-webcomponents/blob/master/packages/main/src/DatePicker.js#L502). Each time the value is changed, the built-in validation is done and if it pass and original valueState is Error, it changes the valueState to None.

For some reason, we have some complex validation logic that is triggered when the value is changed and we don't set minDate and maxDate. valueState and valueStateMessage are set based on the validation result.

Expected Behavior

valueState is only determined by the caller of DatePicker and should not be changed internally

Steps to Reproduce

1. Go to https://codesandbox.io/s/ui5-webcomponents-forked-4bndy?file=/index.html
2. Pick a date not later than today
3. DatePicker's valueState should be kept as Error but it's changed to None

Isolated Example

https://codesandbox.io/s/ui5-webcomponents-forked-4bndy?file=/index.html

Context

• UI5 Web Components version: 0.31.12
• OS/Platform: macOS
• Browser: Chrome
• Affected component: DatePicker

Priority

• Low
• Medium
• High
• Very High

Stakeholder Info (if applicable)

• Organization: SuccessFactors

[fifoosid]

Hi @2bno1

Can you please update your isolated sample because it currently has a couple of errors and I am not sure what exact interaction are you trying to achieve

Currently if you try to interact with the sample you will see that there is a JS error thrown. Once you fix it, the Date.parse(e.target.value) method returns NaN & the if statement always evaluates to false

[phipsalabim]

I am not sure what exact interaction are you trying to achieve

@fifoosid

Not OP, but I think I can clear things up. The problem is that the DatePicker Component changes its own value state implicitly if it's in an Error State and a new date value is being set.

This behavior is incorrect, because the Datepicker component knows absolutely nothing about the validation/business logic of the consumer. For example assume I have a validation rule that only fridays are acceptable inputs, and that a value is required. On initialization I set the value state prop to Error, because a value is required. As the user selects a date, The Datepicker component internally checks if the given date is a js Date object DatePicker.js#L505 If this check passes and the value state is "Error", the component changes the value state to 'None'. This is incorrect. My validation logic is not that trivial, and the component should not make that decision for me. If I set the value state to 'Error' it should stay that way until my validation logic determines that the user selected value is valid (in this silly example, that the date is a friday).

It works correctly for value states: 'None', 'Information' and 'Warning' (the component does not set the value state to None when the date value is changed), but is wrong in the 'Error' case. You cannot handle validation for me since you cannot know my requirements.

If I use Vue and bind the value state prop to a computed method, with my custom validation rule (only accept fridays as input), this implicit internal validation ignores my validation logic and simply overwrites the value state prop, making the component unusable.

hope this helps.

[2bno1]

@phipsalabim Thanks, this is exactly what I mean. The value state should be determined by the consumer of DatePicker component instead of DatePicker itself.

@fifoosid I hope it's clear now.

[fifoosid]

Hi @phipsalabim @2bno1

You can achieve this behaviour quite easily with our current API's

I have prepared example for your use case. All you need to do is to set the valueState to Error if your validation is not successful(in this case, the selected day is not friday).

Note: In the provided example, it might not work for each timezone correctly (based on your location it might work for Thursday instead of Friday)

PS: I just noticed that the sample might not work as expected initially because of some errors related to codeSandbox. If so, just refresh the inner browser page(the one with the datepicker)

[phipsalabim]

@fifoosid

You are still missing the point. I probably explained myself poorly.

The problem is that you are overriding the value-state prop of the datepicker internally. This makes Databinding impossible.

Nobody can bind any data to the datepicker's value-state prop if the datepicker changes that prop on its own, when a date is selected.

Yes I know that I can get a dom ref to the datepicker and manually set the value state to 'Whatever', but at this point I'm back in 2004 writing jquery applications.

People want to bind their pure data models to props on ui components. And those ui components had better not mutate those props on their own, otherwise we can all forget about databinding.

The same issue has now been reported by another user: issue #3571

[fifoosid]

As you already mentioned, this issue is duplicate of #3571

After an internal discussion, I am reopening this issue and it will be worked on

[fifoosid]

FYI @SAP/ui5-webcomponents-topic-b

[ilhan007]

Hello @2bno1
we introduced a change to address your problem and it has been released in 0.31.15

With this change you can prevent the default behaviour of the DatePicker on "change" and "input" events and it's up to you to set the both the value and the valueState props, based on you app logic as the component won't set them internally. Now, you can set both it in the event handler.

The event details will provide both the selected "value" and the "valid" as params and you can use them to set the value and the valueState as shown bellow:

datePicker.addEventListener('change', function(e) {

	// Prevent setting value and valueState
	e.preventDefault();

       // Check the available event details
	e.detail.value // Returns the value typed or selected by the user
	e.detail.valid =  // Indicates if the value is valid or not ("true" or "false"), based on the format, min-date and max-date props, etc.
	
	// Set the value and the valueState
	datePicker.value = e.detail.value;
	datePicker.valueState = // "Error or None", based on internal logic or based on the e.detail.valid event parameter
});

Let me or @tsanislavgatev know if you have questions how to use this enhancement.

[2bno1]

@ilhan007 Thanks for your working on this issue. The change introduced a regression: in the event handler, e.target.dateValue does not return the current date user picked, instead, it returns the previously picked date. You can reproduce this using this sample.

[ilhan007]

Hello @2bno1 yes we tested it and we could reproduce the issue, as now the event is preventable, the event is first fired and then the state is updated, that's why the dateValue getter points to the old value. We will fix that probably in a day or two period of time.

Sorry for the inconvenience.

[ilhan007]

Hello @2bno1 the "dateValue" getter should be fixed in 0.31.16.