Support #empty_option for the link_type select form element

Work here will include the work for ✨ Support a required link_type input form element Closed: duplicate as it is all related.

Thank you @geekygnr! Did you have any preferences on a way forward here or would you like to see me propose something?

I am throwing something together to try to make the UX a little more consistent.

If there is a specific use case that has motivated the request I would love to hear it. I can try to cater the solution towards it.

Awesome! Well, I'll share what prompted the issues. When I was trialing the module out with a multivalue field, Drupal will render the link text, uri and type inputs. The type input defaulted to the first option and drew my eye on the form. As a result, I was second guessing if the value was going to save with the form or if I should unset it. It just threw me off from a content authoring perspective. Hope that helps and thanks, Jason

geekygnr → changed the visibility of the branch 3418165-support-emptyoption-for to hidden.

geekygnr → changed the visibility of the branch 3418165-support-emptyoption-for to hidden.

geekygnr → changed the visibility of the branch 3418165-support-emptyoption-for to active.

geekygnr → closed merge request !2

geekygnr → reopened merge request !2

I put some changes in. There is a tugboat preview built for the MR.

Let me know your thoughts.

Thank you very much for considering this feature request and contributing the changes so quickly!

✓ I can see the - Select - option selected as the first option when the form is rendered
✓ The link type field is now required when the field is required

The #states usage is a nice touch! It's helpful and mirrors the behavior from Drupal core's LinkWidget behavior here when providing link text and the the URL input becomes required. However, this makes the type type required even when the field is not required (though this be expected and consistent with current link type behavior, more below).

I realize the following scenario represents an edge case, b/c I manually added the novalidate attribute to the form element to bypass clientside form validation, but I thought I'd share it. In this scenario, without clientside validation, I can submit the form and receive an error from server side validation error, "The value you selected is not a valid choice.".

I think this behavior is consistent with previous behaviors as the link type value can only be a value from the allowed list of configured options. However, if the field is not a required field, I'd expect to be able to submit the form without selecting a link type option. Have you encountered a need for this behavior, to save without a link type option?

I made the link type required because it matches the field definition.

If the type isn't set the behavior would essentially be the same as a normal link field.

Are you looking to be able to set a link in the typed link field and not set a type?

oh, I see now. Thank you for linking to the field definition; I had not seen that before.

If the type isn't set the behavior would essentially be the same as a normal link field.

Are you looking to be able to set a link in the typed link field and not set a type?

I see your point. No, I don't have a specific requirement to allow some Typed Link values with types and some values without a type. If this does come up, I'll create a separate feature request issue.

Using #required and #states seems like the way to go to visually indicate that input is required, and with the addition of the empty option definitely communicates the need to select an option b/c of client and server side validation errors.

One other observation, the empty_value in the MR uses the string '- Select -'. If instead, it uses the string '- Select a value -' to align with core's OptionsSelectWidget here, the string would benefit from using translations from https://localize.drupal.org/ for Drupal core....nevermind! It looks '- Select -' is used in several places in Drupal core and also has translations from https://localize.drupal.org/.

Looks good, thanks again!

Merged I will look at putting out a new version.

Thanks for the contribution to this.
However, the way this was orchestrated bothers me. I leave my thoughts here for consideration in the future releases.
Firstly, this change is missing tests.
On the other hand, our builds are failing because we relied on the fact that the type selection was a required field. Such change, should not be introduced in a patch release, because you change the behaviour of an existing feature.

@joevagyok I think the change that broke your tests was an unintentionally committed.

The intended changes were all non-breaking and small UX adjustments hence why it was a patch version.

I left another MR, I will leave it to you to adjust and merge.

@geekygnr, I understand your good intentions here! Please take no offence from my side!

Changing the UI/UX behaviour of the module, which is not a small change and definitely not a patch release, patch release suggests there was a bug which needed fix.
That is the reason making changes without tests is risky and if this module had been used by thousands of sites, would cause problems for them. We caught it because we have automated tests on our side and have nightly builds, but we all bear the responsibility when providing for a community.

I will take some time to write tests for the module. For example, the #states required won't work if the field is in field group or paragraphs.
Thanks for making the requested changes!

I had 2 of my colleagues review the MR. It is ready for merge.

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