The allowed values storage setting of list fields should be required

Added the required field attribute for Allowed value list in list(float) field type.

I agree with @idebr on this, while it might be sensical for UX, it's a -1 for DX. And it's not hurting anybody if you leave it empty, so I don't see this as helping anybody. It's not like users will be surprised to find an empty select field if they don't supply any allowed values ¯\_(ツ)_/¯

And if somebody is populating these options using something else than an allow values callback (using a form alter or something), they might also want this empty.

@Lendude if we try to proceed without entering any values in allowed value list for list(float) field type then it cannot proceed and an error pops up as shown in the screenshot attached.We can atleast do this for list(float) field type as i did in #20.
If we don't want it for all the list types we can roll-back the changes in last commit.

This seems like something that would need test coverage.

#23 I don't fully grasp how is this UI change regressing DX? If you know how to generate options dynamically, making the field not required doesn't seem like a big ask. I also don't think we should be optimizing for the edge cases, but rather the majority use case. Having list field without allowed values is still valid config and it will be possible to create fields programmatically without the allowed values.

And it's not hurting anybody if you leave it empty, so I don't see this as helping anybody. It's not like users will be surprised to find an empty select field if they don't supply any allowed values ¯\_(ツ)_/¯

It is hurting if you don't know where you are supposed to configure the options. Since it's not required and someone doesn't understand what it's for, they might ignore it and forget that they were presented with this option. If you don't understand where you were supposed to configure the available options, an empty select field could come up as a surprise.

This issue is problematic in particular in combination with 📌 List key|label entry field is textarea, which doesn't give guidance towards the expected input Fixed .

Perhaps I'm missing something but wouldn't it be possible to address both the UX and DX concerns with this?

 public function storageSettingsForm(array &$form, FormStateInterface $form_state, $has_data) {
    $allowed_values = $this->getSetting('allowed_values');
    $allowed_values_function = $this->getSetting('allowed_values_function');

    $element['allowed_values'] = [
      '#type' => 'textarea',
      '#title' => $this->t('Allowed values list'),
      '#default_value' => $this->allowedValuesString($allowed_values),
      '#rows' => 10,
      '#access' => empty($allowed_values_function),
      '#element_validate' => [[static::class, 'validateAllowedValues']],
      '#field_has_data' => $has_data,
      '#field_name' => $this->getFieldDefinition()->getName(),
      '#entity_type' => $this->getEntity()->getEntityTypeId(),
      '#allowed_values' => $allowed_values,
      '#required' => empty($allowed_values_function), // <<<<<<< Add this line and required will only be true when no 
                                                      // allowed values function is present.
    ];

If that doesn't take care of it I also think the DX hit is negligible compared to the UX benefit. It's going to benefit a more common use case that better serves a persona who benefits more from the help. It's the kind of system feedback that effectively trains a new user through gradual exposure. The allowed-values-function-using persona that objects to having the field required is less common and likely knows how to address it, (and the allowed values function message could be tailored if there's any concerns they don't).

It missed before that it looks like the field already has a #access property defined based on whether allowed_values_function has been set or not. Because of this, it doesn't seem like there is any DX hit from this, and we don't need to add any additional checks for #required because the field is not visible when allowed_values_function is defined.

Some new open threads.

All the feedback has been addressed. The MR looks good to go.

Adding to #28, I think #4 is actually describing a valid use case where there is hit for DX from this. In this use case developer would create a field config through the UI and then modify the config to add the allowed_values_function. Even in this case the DX hit seem negligible since you could always add a single value to the options that you remove when you edit the config.

Added one nit-ish comment to the MR.

That reads a lot better! Thanks @bnjmnm 😉

Discussed with @Lendude on Slack and he pointed out ✨ Allow adding predefined lists to the options fields Needs work as a follow-up to address #23. That seems like the way to go moving forward. 👍

Committed 18432da and pushed to 11.x. Also cherry-picked to 10.1.x. Thanks!

lauriii → closed merge request !4004

Automatically closed - issue fixed for 2 weeks with no activity.