Add support for <nolink> and <none> to the LinkWidget UI

This implementation seems reasonable. IMHO some integration test for the actual rendered menu link would be great, IMHO

I added support for and also for empty values.

I think we should open a followup issue to make these special values to be accepted when set the option External links only is set for Allowed link type at the field config, too.

The empty value option needs more work to pass tests.

first it needs to differentiate "empty" and "none" values and effect of "title" #2634232: In case of only 'link text' is required, then remove the required mark if the user deletes the text entered for URL

I don't think we should support "". It's atypical for a menu link not to have a href, so more often than not I feel like it should trigger an error reminding people to enter something. <none> or <nolink> is a nice explicit decision.

There's a problem with the patch in 4: If you create a link field and let the default value of the url field empty, for some reason will be assumed as value. If this happens on a multivalue field, a second empty link field is printed in the entity form and the widget default value form, because the first default value containing is interpreted as value. If you do another save on the field settings page, another default value with is added etc.

This could probably be avoided by not interpreting empty values as nolink values, so +1 for the comment in #13.

Closed #2880632: Allow 'link text' without 'URL' (URL optional) as a duplicate issue.

We also should be able to use the routes and but we can't replace one from them to another. For example when I wish to display the link without url and I wish to use the Superfish module, than the mobile menu will be broken if I try to use . Using in this case work perfectly.

So, within the issue the admin should decide which router he want to use or .

Two things in this patch:

1. Rerolled against 8.5.x dev.
2. Removed support for the empty string, which has been suggested and causes an issue described above.

Here's the patch rerolled against 8.3.x dev. I also removed support for the empty string, which has been suggested and causes an issue described above.

Apologies, the patch from #20 had a missing bracket. Here is the 8.3.x dev patch resolved.

#22 works great! Thank you, ryanissamson! It would be nice to have help text, too, so that users know to use <nolink>. I made a patch for that.

Thanks, @angelamnr.
Patch #23 works great.

In order to commit a bug fix we need an automated to test to prove that we've fixed the bug and ensure that we don't break it again in the future. For more information about writing tests in Drupal 8 see the following links:

1. https://www.drupal.org/docs/8/testing
2. https://api.drupal.org/api/drupal/core%21core.api.php/group/testing/8.5.x

@dawehner pointed this out in #3.

Attached functional tests.
Also updating the IS because:

• currently doesn't clearly state what this issue is about, the title is the only clue.
• Has still some reference to allowing "empty" URL links, which has been dropped during the work.

In these transitional periods I never know what the right D8 branch target is. Leaving 8.5.x just for the sake of running the tests, but I believe this should be 8.7.x .

What about <current>? Can we add that here? If not, that's the only special case we leave unsupported.

Tests already added

@Jacine the <front> already done, only none and nolink missing

+++ b/core/modules/link/tests/src/Functional/LinkFieldUITest.php
@@ -196,7 +196,7 @@ public function runFieldUIItem($cardinality, $link_type, $title, $label, $field_
-      LinkItemInterface::LINK_GENERIC => 'You can also enter an internal path such as <em class="placeholder">/node/add</em> or an external URL such as <em class="placeholder">http://example.com</em>. Enter <em class="placeholder">&lt;front&gt;</em> to link to the front page.',
+      LinkItemInterface::LINK_GENERIC => 'You can also enter an internal path such as <em class="placeholder">/node/add</em> or an external URL such as <em class="placeholder">http://example.com</em>. Enter <em class="placeholder">&lt;front&gt;</em> to link to the front page. Enter <em class="placeholder">&lt;nolink&gt;</em> to display link text only',

extended with *nolink*

Hi @andypost! :)

I'm asking about <current>, not <front>. AFAICT, that's the only "special" one left from the list.

'<front>':
  path: '/'
  defaults:
    _title: Home
  requirements:
    _access: 'TRUE'

'<none>':
  path: ''
  options:
    _no_path: TRUE
  requirements:
    _access: 'TRUE'

'<nolink>':
  path: ''
  options:
    _no_path: TRUE
  requirements:
    _access: 'TRUE'

'<current>':
  path: '<current>'

Hey @Jacine sorry I misread( Yep - nice catch!
It would be great addition, btw It may affect caching

So back to work in order to allow <current>

This patch adds ability to use <current> special route in the Link widget, as well as <nolink>, <none> and <front>.

As I really don't see a use case for using this, and due the probable issue with cached versions of the rendered field, I have NOT mentioned <current> in the widget description/helper.
This is currently still reading: "[...] Enter <front> to link to the front page. Enter <nolink> to display link text only."

Users can use the route, but as it's discouraged they can at their own risk. Thoughts?

For a time I used patch #26. With this patch I could use <none> in every Drupal menu. That works great btw :)

Upgrade a develop site to use the latest patch #35. Now i encounter a problem: After I safe a menu item with it is now converted to <nolink>

This come with a problem. However the menu item still doesn't have a menu link, at the front-page the menu item with <none> -> (converted to) <nolink> is now an active link according to the system. So it get also the styling of an active menu link, what it of course not is.

Using Drupal 8.6.3. Checked this behaviour not only in my custom theme also with Bartik

Ok it seems that there is an other issue for: https://www.drupal.org/node/2838351. Added it to related issues.

I think we should split <current> into its own issue. The caching side of that one makes it complex whereas <nolink> and <none> don't have this problem. Also the <nolink> and <none> are the 80%+ case.

@alexpott splitting makes sense.

Current patch is now #26. I'm createing the follow-up and link it here.
Also updating the title to remove <current> and reference to "empty values" (see #13 and #19 for why and when they have been excluded from the work).

Created #3015319: Add support for <current> to the LinkWidget UI.

Used this patch on our project. Works as expected.

Sorry, bunch of nits.

1. <current> has not yet been removed per @alexpott's request in #38.

2. +++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
   @@ -110,6 +116,14 @@ protected static function getUserEnteredStringAsUri($string) {
   +    elseif (in_array($string, array('<nolink>', '<none>'))) {
   

   s/array()/[]/

   Also, could be made a strict check.

3. +++ b/core/modules/link/tests/src/Functional/LinkFieldTest.php
   @@ -131,6 +132,15 @@ public function testURLValidation() {
   +      // Text only links.
   

   s/Text only/Text-only/

4. +++ b/core/modules/link/tests/src/Functional/LinkFieldTest.php
   @@ -726,6 +736,87 @@ public function testEditNonNodeEntityLink() {
   +  public function testSpecialRoutesAsLinkUri() {
   +
   +    $field_name = mb_strtolower($this->randomMachineName());
   

   Nit: extraneous newline.

+++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
@@ -110,6 +113,10 @@ protected static function getUserEnteredStringAsUri($string) {
+    // Support linking to nothing.
+    elseif (in_array($string, array('<nolink>', '<none>'))) {
+      $uri = 'route:<nolink>';
+    }

This results in changing user input. I.e. if you enter <none> - I'm not sure of the difference between none and nolink but if there is a difference then this should maintained. Also swapping out user-input like this is not how we manage this elsewhere.

Re #43 there's also a difference between how none and nolink render.

See...

>>> \Drupal\Core\Link::createFromRoute('test', '<nolink>')->toString();
=> Drupal\Core\GeneratedNoLink {#1832
     markup: "<span>test</span>",
   }
>>> \Drupal\Core\Link::createFromRoute('test', '<none>')->toString();
=> Drupal\Core\GeneratedLink {#1835
     markup: "<a href="">test</a>",
   }

#44: thanks, I was wondering about exactly that; I was wondering why they were being treated the same by #42.2.

Created a patch that can be applied to 8.8.x.

Didn't do anything with comment 44/45

Sorry, "needs work" to address comments from Alex and Wim.

Interdiff for #35 -> #47

#42.1 <current> was removed in #47, see interdiff in #50.

#42.2 Applied short array notation, added strict parameter.

#42.3 Replaced 'Text only' with 'Text-only'

#42.4 Removed extraneous newline.

#43 <nolink> and <none> are now treated as separate values.

Nice tweak Ide !

+1 :)

#42 and #43 have been addressed, tests a green, looks RTBC to me. Thanks!!!

Failure are not related, see #3055648: Frequent random fail in \Drupal\Tests\media_library\FunctionalJavascript\MediaLibraryTest . RTBCing again.

+++ b/core/modules/link/src/Plugin/Field/FieldWidget/LinkWidget.php
@@ -209,17 +216,17 @@ public function formElement(FieldItemListInterface $items, $delta, array $elemen
     elseif ($this->supportsExternalLinks() && !$this->supportsInternalLinks()) {
-      $element['uri']['#description'] = $this->t('This must be an external URL such as %url.', ['%url' => 'http://example.com']);
+      $element['uri']['#description'] = $this->t('This must be an external URL such as %url. Enter %nolink to display link text only.', ['%url' => 'http://example.com', '%nolink' => '<nolink>']);
     }

This description is incorrect: route:<nolink> is an internal link, so it will not be accepted as valid input.

This description should therefore not be changed here.

#56 Removed the change to the field description for external link fields.

@idebr the <nolink> route is neither external nor internal? Is a way for editors to display a no-link. The same reasons editor may have to requiring a no-link in a LINK_INTERNAL widget may be the same one may have in a LINK_EXTERNAL one?

@idebr I saw what you mean: from a validation point of view that can't be used. Shame, as a no-link link should be nice to have on both external and internal only widgets.
Maybe in a follow up? RTBCing for now, as to me doesn't look like a big blocker for what the issue is trying to achieve.

Committed f4fbbd1 and pushed to 8.8.x. Thanks!

I think this might warrant a change record?

@Wim Leers yeah I debated that. The reason I ended up not asking for one if that the UI makes it clear that this is now allowed. And who has to update something as a result of this? I.e. what is there to change?

I guess the menu link should be updated now #3062883: Unify nolink