Binder.invalid property doesn't accurately reflect validity of form data

[DevJohnC]

Description of the bug

Binder invalid property doesn't accurately reflect the validity of data entered into the model.

The invalid property appears to be updated only when validate() is run.

Minimal reproducible example

A simple model in Java with a required field:

public class MyModel {
    @NotBlank
    private String myField;

    public String getMyField() { return this.myField; }
    public void setMyField(String myField) { this.myField = myField; }
}

Then create a Binder in a lit component:

@customElement("my-form")
export class MyForm extends View {
    private binder = new Binder(this, MyModelModel);

    render() {
        return html`
            <vaadin-text-field label="My Field" ...='${field(this.binder.model.myField)}'></vaadin-text-field>
            <vaadin-button ?disabled="${this.binder.invalid}" @click="${this.binder.submit()}">Submit</vaadin-button>
        `;
    }
}

Expected behavior

The submit button should be disabled until the data held in the bound model is valid.

Note: this doesn't mean invalid should be true by default, it means that invalid should reflect directly the validity of data in the bound model

Actual behavior

The submit button is enabled until the user attempts to submit.

Versions:

- Vaadin / Fusion version: 22.0.0
- Node version: 16.13.0
- Java version: 11
- OS version: Windows 10
- Browser version (if applicable):
- Application Server (if applicable):
- IDE/Editor (if applicable):

[platosha]

Note: this requires to add a touched state for fields.

Currently, the visual indication of invalid state does only take invalid into account. The semantics of invalid is not quite correct here, but that allows to not show initially empty form as invalid, in case of @NotBlank validation, for example.

After we fix the semantics for invalid, the initial rendering of the form should still not indicate errors until the fields are touched or a submission is attempted.

[cromoteca]

Actually the touched state already exists and is called visited. Validation should be triggered immediately on the whole form and validity update status on single fields should be skipped until visited.

[cromoteca]

I modified the code to take advantage of the visited state and I'm not sure that it is a good idea to implement this change. It doesn't look like a bug to me as those are just two different ways to implement validation feedback.

Disabling the submit button until form data is valid might be confusing for the user. Imagine a form having some required fields and some other validation patterns like a range. By default, all fields are empty, some of them are marked as required and the Submit button would be disabled. The user can guess that required fields must be filled in, but after that, the button would still be disabled as not everything is valid and the user does not know why unless he "visits" all fields by hand.

On the contrary, being able to click on the Submit button would activate visual feedback for all invalid fields.

[DevJohnC]

On the contrary, being able to click on the Submit button would activate visual feedback for all invalid fields.

Having to click a submit button for the user to have indicators that their form is valid is poor UX design. The disabled submit button is clear and obvious feedback to the user that their form isn't complete or valid. Personally, it's quite aggravating to click submit and to get a bunch of errors suddenly appear or, even worse, for a bunch of errors to appear above the scroll and not even be aware of it.

[platosha]

@DevJohnC please note that you can disable submitting unchanged forms with something like:

<vaadin-button ?disabled="${this.binder.invalid || !this.binder.dirty}" @click="${this.binder.submit()}">Submit</vaadin-button>

This is not exactly what you’re asking for, but still a similar UX behaviour with initially disabled submit until the form is complete and valid. The slight difference is that this disables submitting unchanged form even if values are initially valid, which could be a good (I think) or a bad thing. Would this fit your use case?

[DevJohnC]

@DevJohnC please note that you can disable submitting unchanged forms with something like:

<vaadin-button ?disabled="${this.binder.invalid || !this.binder.dirty}" @click="${this.binder.submit()}">Submit</vaadin-button>

This is not exactly what you’re asking for, but still a similar UX behaviour with initially disabled submit until the form is complete and valid. The slight difference is that this disables submitting unchanged form even if values are initially valid, which could be a good (I think) or a bad thing. Would this fit your use case?

This doesn't seem to cover even the most obvious of use cases, let alone potential edge cases. The clear example being: what if the form is pre-populated with data in, say, an edit form?

[DevJohnC]

For reference, this bug is reported because it's in conflict with hilla documentation:

https://hilla.dev/docs/data-binding/form-status-tracking

There seems to be some disconnect somewhere on how it's intended to function.

[platosha]

This doesn't seem to cover even the most obvious of use cases, let alone potential edge cases. The clear example being: what if the form is pre-populated with data in, say, an edit form?

The dirty state is based on comparison of binder.value and binder.defaultValue, the latter is not necessarily empty, but could hold the user-defined initial data.

For edit forms, the binder needs to be initialised so that binder.value and binder.defaultValue both hold the pre-populated data. A convenient way of doing so is to call binder.read(data);, this automatically sets both value and defaultValue at the same time, thus making dirty state false.

You can also reset the edit form to the initial pre-populated data value with binder.reset() (which uses binder.defaultValue).