[custom-field] Bridge `checked` and `selected` properties

[manolo]

Bridging checked and selected like is done with value would habilitate the option of using a set of vaadin components such vaadin-checkbox vaadin-radio-button vaadin-list-box and native elements like input type=checkbox and input type=radio,

[web-padawan]

The components that are currently supported are listed here: https://github.com/vaadin/vaadin-custom-field/blob/ea1e0aa163de7469dad8e58344652881dd7773f7/demo/custom-field-basic-demos.html#L10-L22

IMO, we should not encourage using checkbox and radio-button in custom field. They do have value, but that serves for different purpose and can mess things up.

Regarding vaadin-list-box, this depends on https://github.com/vaadin/vaadin-list-mixin/issues/24

[manolo]

I need to disagree here, boolean fields might be many times mandatory and validatable, and having a wrapper that provides all the boiler plate to show the required flag bullet and also be able to show the error message makes things pretty easy, especially when binding entire forms (aka ccdm forms in v17).

Note that the implementation, instead of bridging all possible fields could be configurable via an extra attribute like fieldForValue="checked"

A [ ] please check this to continue should be extremely easy and an amazing DX if they can write something like:

<vaadin-custom-field ...="${field(this.binder.model.myBooleanField)}" label="Agreement">
  <vaadin-checkbox>I agree with the terms and conditions of this web site</vaadin-checkbox>
</vaadin-custom-field>

And get the output:

[web-padawan]

Adding basic checked support should be easy, as long as we have change event in place. However there are open questions that have to be figured out.

Validation

I agree that the use case of required indicator is totally valid and we considered it some time ago. But we don't have it in vaadin-checkbox neither vaadin-radio-button yet.

Currently we expect any slotted input component to implement it: https://github.com/vaadin/vaadin-custom-field/blob/ea1e0aa163de7469dad8e58344652881dd7773f7/src/vaadin-custom-field-mixin.html#L181

If we want to avoid having to implement validation in checkbox and radio-button, we would have to detect using those components explicitly somehow 🤷‍♂️

Property binding

First, we would have to also agree on the fact that true (and maybe false too) are special string values that are supposed to be converted to the corresponding boolean literals.

And then we have two steps to be done:

1. Optional: tweak parseValue to do the conversion for all occurrences:

https://github.com/vaadin/vaadin-custom-field/blob/ea1e0aa163de7469dad8e58344652881dd7773f7/src/vaadin-custom-field-mixin.html#L58-L60

So it would be something like this:

 parseValue: function(value) { 
   return value.split('\t').map(i => i === 'true' || i === 'false' ? Boolean(i) : i);
 },

2. Update the line where we collect values: https://github.com/vaadin/vaadin-custom-field/blob/ea1e0aa163de7469dad8e58344652881dd7773f7/src/vaadin-custom-field-mixin.html#L158

3. Update the line where we actual set the value:

https://github.com/vaadin/vaadin-custom-field/blob/ea1e0aa163de7469dad8e58344652881dd7773f7/src/vaadin-custom-field-mixin.html#L203

So it would need to be something like this:

this.inputs.forEach((input, id) => {
  const hasChecked = // ??? 
  if (hasChecked) {
    input.checked = valuesArray[id] === 'true'; // assuming we don't tweak `parseValue`
  } else {
    input.value = valuesArray[id];
  }
}); 

And the main question is the same as above how do we distinguish if component has checked property? We can rely on false set by default but what if user forces undefined?

The most reliable solution would be to use instanceof CheckboxElement but that is bad:

1. we introduce direct dependency on 2 components because of instanceof only
2. we make components aware of each other rather than using proper API.

Let's discuss what options do we have to implement this feature and at the same time avoid baking any workarounds into this component.

[manolo]

As I suggested in my previous comment, it's not necessary to trust in child behaviour but in an attribute that the developer provides, like fieldForValue="checked" or fieldForValue="selected" etc.

Related with the value.split this call is the main problem of bridging non-string values as it was reported in vaadin/vaadin-custom-field#77, at this point I would check whether value is typeof string, to not perform the split and pass the value as it is to the child. If there are multiple children it would check for the presence of an array of values or a splittable string. But those are implementation details.

The main point here is to take advantage of this full featured component, to have homogeneous UIs and easy to setup forms

[web-padawan]

fieldForValue="checked" or fieldForValue="selected" etc

What about case when user tries to mix vaadin-checkbox and vaadin-text-field inside the same vaadin-custom-field instance? Sounds like this would not be an option then.

[manolo]

Yes, giving that option is exclusive.

Another option is to align all Vaadin components to follow the same API (value, required, invalid, errorMessage) , so checkeable and selectable elements have also the value property, like it is suggested for vaadin/vaadin-list-mixin#24

[web-padawan]

all Vaadin components to follow the same API (value, required, invalid, errorMessage) ,

We have not accepted an external PR for vaadin-checkbox validation because we did not decide anything on how do we show required indicator for it, see https://github.com/vaadin/vaadin-checkbox/issues/84

Also, as I mentioned, both vaadin-checkbox and vaadin-radio-button have value but it's meant for different purpose (mainly for using in corresponding -group) element.

Yes, giving that option is exclusive.

This is not what users would expect. They always try to put things inside one another 🙂

[manolo]

We have not accepted an external PR for vaadin-checkbox validation because we did not decide anything on how do we show required indicator for it

precisely this is one of the reason for custom-item to support these fields