Checkbox should implement HasValidation

[mvysny]

Describe your motivation

The "I agree with EULA"/"I agree with the following terms"/"I agree" checkbox at the bottom of the form. Unless the Checkbox is checked, the form can not continue. This can easily be implemented by a Checkbox+Binder+Validator which only passes the "true" value. Or @javax.validation.constraints.AssertTrue(message="Please accept the terms before continuing").

Describe the solution you'd like

Checkbox should implement HasValidation and should be able to mark itself invalid and show a validation error, exactly as other HasValue components.

Describe alternatives you've considered

No response

Additional context

No response

[web-padawan]

Depends on https://github.com/vaadin/web-components/issues/938 to be implemented in the web component.

Here is one related concern: https://github.com/vaadin/vaadin-checkbox/issues/188#issuecomment-701408779

So, the HasValidation interface bring in the setErrorMessage() method? That sounds a bit problematic, as that would imply that Check Box should be able to show that message somewhere. But the Check Box web component not a similar field by itself as the others, as it doesn’t have a similar kind of field label or error message.

[mvysny]

Thank you. Meanwhile the workaround is to wrap the Combobox in CustomField:

public class CheckboxWithValidation extends CustomField<Boolean> {
    private final Checkbox checkbox = new Checkbox();

    public CheckboxWithValidation() {
        checkbox.addValueChangeListener(e -> updateValue());
        add(checkbox);
    }

    public CheckboxWithValidation(String label) {
        this();
        setLabel(label);
    }

    @Override
    public void setLabel(String label) {
        checkbox.setLabel(label);
    }

    @Override
    public String getLabel() {
        return checkbox.getLabel();
    }

    @Override
    protected Boolean generateModelValue() {
        return checkbox.getValue();
    }

    @Override
    protected void setPresentationValue(Boolean value) {
        checkbox.setValue(value);
    }
}

Then:

@Route("")
public class MainView extends VerticalLayout {

    public MainView() {
        final CheckboxWithValidation cb = new CheckboxWithValidation("I agree");
        cb.addValueChangeListener(e -> {
            cb.setInvalid(!cb.getValue());
            cb.setErrorMessage(cb.isInvalid() ? "You need to agree to EULA" : null);
        });
        add(cb);
    }
}

[jouni]

I just want to state my opinion, for future reference – not saying this is the best solution.

I think Checkbox should be more of an input control rather than a form field. The form field features (label, required indicator, description, error message, validation) should rather be implemented by a separate component which can be used together with any input control.

Therefore, I think it might not be a good idea to implement HasValidation for Checkbox, as that might complicate the former idea in the future. Custom Field is a then perhaps the “correct” solution, but would benefit from a different name and could maybe be made less laborious (less boilerplate)?

[mvysny]

@jouni do you perhaps have in mind some sort of decorator that would wrap around the component and would work with any component? Sounds like a good reusable idea - composition is a very good pattern indeed. Perhaps FormLayout.addItem could automatically wrap fields in this decorator component, since that FormLayout is intended for forms.

However, at the moment (nearly) all field components do implement the HasValidation interface at Java side . For consistency reasons, given that Checkbox is also a form component, it would make sense for it to implement HasValidation. Alternatively, all form components should then consistently stop implementing HasValidation. Deciding otherwise feels inconsistent.

[jouni]

do you perhaps have in mind some sort of decorator that would wrap around the component and would work with any component?

Yeah, pretty much.

Perhaps FormLayout.addItem could automatically wrap fields in this decorator component, since that FormLayout is intended for forms.

Yeah, that seems like a nice default behavior for the Java API.

Deciding otherwise feels inconsistent.

And yes, I agree with this as well. As we are probably going to live with the existing form fields for quite a while still, perhaps it’s nicer to make them behave consistently.

I suppose, if we ever do such a change, we can find a way to migrate from the "form field" Checkbox class to a “input control” Checkbox class, possibly via a feature flag. That’s my main concern, breaking backwards compatibility in the future.

[yuriy-fix]

It would make sense to implement required state on the checkbox firstly. It's probably not a good idea to support HasValidation (in the specified scenario we are depending on the value of the component and not on its validation status) and instead CustomField should be used as mentioned above.

[mstahv]

Should this be closed? Will be released in 24.4 🤔

[web-padawan]

Yes, this is implemented by #6167. Closing as completed per above comment.