Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hasInputValue is not always updated accurately #5639

Open
vursen opened this issue Mar 7, 2023 · 1 comment
Open

hasInputValue is not always updated accurately #5639

vursen opened this issue Mar 7, 2023 · 1 comment

Comments

@vursen
Copy link
Contributor

vursen commented Mar 7, 2023

Describe your motivation

The hasInputValue property was introduced as part of the Flow validation improvements to help the Flow counterparts identify bad user input. They check for this property during server-side validation. When the property indicates true and there is no component value at the same time, the Flow counterpart recognizes that the user has entered something that the Flow server has been unable to parse, so the field should be invalidated.

However, hasInputValue is not always updated 100% accurately due to some workarounds.

DatePicker allows you to select and deselect dates with Space in the dropdown. When you deselect a date, the component's value resets but the date is still displayed in the input element because the date is still focused. If we went strictly by the book, we would set hasInputValue to true in that case. Unfortunately, that would cause Flow to think that there is bad input. So, we had to make a workaround in DatePicker that resets hasInputValue to false when selecting a date with Space to not confuse the Flow server.

The workaround contradicts the idea of the hasInputValue property.

Solution 1

Change DatePicker's behavior so that it will not commit the date selected with Space until the dropdown is closed.

This will allow us to stick to the original hasInputValue idea and will guarantee that the Flow server won't get an empty value while hasInputValue is true. With that, we will be able to replace the current workaround in DatePicker with the original fix that was proposed in #5410:

In addition to updating hasInputValue on input, update it in the InputMixin._inputElementValue setter.

Solution 2

An alternative could be Introducing a hasBadInput property in InputConstraintsMixin that would be updated accordingly on both user and programmatic input changes. The new property could replace hasInputValue in InputMixin.

One downside is that this property is going to be updated more frequently than hasInputValue which means more Flow round-trips.

Prototype: https://github.com/vaadin/web-components/compare/prototype/has-bad-input

Additional context

Related to vaadin/platform#3066

@vursen vursen added refactor Internal improvement validation labels Mar 7, 2023
@vursen vursen changed the title Consider alternatives to the hasInputValue property DatePicker does not update hasInputValue accurately Mar 8, 2023
@vursen vursen changed the title DatePicker does not update hasInputValue accurately hasInputValue is not updated accurately Mar 20, 2023
@vursen vursen changed the title hasInputValue is not updated accurately hasInputValue is not aways updated accurately Mar 20, 2023
@vursen vursen changed the title hasInputValue is not aways updated accurately hasInputValue is not always updated accurately Mar 20, 2023
@vursen
Copy link
Contributor Author

vursen commented Mar 21, 2023

If we had a hasBadInput property, we could align the approach to clearing bad input when setting an empty value.

Example:

if (Objects.equals(oldValue, getEmptyValue())
        && Objects.equals(value, getEmptyValue())
        && isInputValuePresent()) {
    // Clear the input element from possible bad input.
    getElement().executeJs("this._inputElementValue = ''");
    getElement().setProperty("_hasInputValue", false);
    fireEvent(new ClientValidatedEvent(this, false));
} else {
    // Restore the input element's value in case it was cleared
    // in the above branch. That can happen when setValue(null)
    // and setValue(...) are subsequently called within one round-trip
    // and there was bad input.
    getElement().executeJs("this._inputElementValue = this.value");
}

would become:

if (Objects.equals(oldValue, getEmptyValue()) && 
    Objects.equals(value, getEmptyValue()) && 
    isBadInputPresent()) {
    getElement().executeJs("if (this._hasBadInput) this._inputElementValue = ''");
    getElement().setProperty("_hasBadInput", false);
    fireEvent(new ClientValidatedEvent(this, false));
}

(from https://github.com/vaadin/flow-components/pull/4834/files#r1142364781)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant