Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
#1543750: Allow menu items without path wanted non-link menu link items but we can do better and solve it on a link level.
Proposed resolution
Add a <nolink> route which renders a <span> instead of an <a> in linkGenerator::generate
.
Remaining tasks
User interface changes
API changes
If someone defined a <nolink> route that breaks... but why would you define such a thing? And how could we a route ever if that's a problem.
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#24 | 2693725-24.patch | 4.6 KB | alexpott |
#24 | 13-24-interdiff.txt | 2.21 KB | alexpott |
#13 | 2693725_13.patch | 4.17 KB | chx |
Comments
Comment #2
chx CreditAttribution: chx as a volunteer and at Smartsheet commentedComment #3
chx CreditAttribution: chx as a volunteer and at Smartsheet commentedComment #4
dawehnerIf we do that, it would be nice to mention that on GeneratedLink itself
Comment #5
star-szrFrom a frontend POV
<span>
is probably the right thing to use here. I'm curious what kind of attributes would/could end up on that element though (in common scenarios). The test just shows a span with no attributes.Comment #7
chx CreditAttribution: chx as a volunteer and at Smartsheet commentedComment #8
chx CreditAttribution: chx at Smartsheet commentedComment #10
chx CreditAttribution: chx at Smartsheet commentedComment #11
dawehnerComment #12
star-szrA few of us talked about this on IRC and I think we agreed that ideally we would just use
<none>
but it's already taken. The nice thing about<nolink>
IMO is that it's theoretically in use by almost 100k Drupal 7 sites already: https://www.drupal.org/project/special_menu_itemsA more ideal solution is probably to (also) make this part of the menu link UI somehow but that's definitely out of scope here.
@file docblock is slightly out of date.
Comment #13
chx CreditAttribution: chx at Smartsheet commenteddocblock fixed.
Comment #14
dawehnerAh I was gonna say we should add test coverage for using that in the UI< but I totally believe that this will actually happen as part of #1543750: Allow menu items without path
Note: you could use assertInstanceOf if you like.
Comment #15
dawehnerSorry, I actually wanted to RTBC it.
Comment #16
Eric_A CreditAttribution: Eric_A commentedIs changing the tag from a to span in scope for this issue? An a tag without href attribute is perfectly valid HTML5, no?
Comment #17
dawehnerIf you theme that, do you think this would work as well?
Comment #18
Eric_A CreditAttribution: Eric_A commentedSorry, not sure what this means...
Anyways, the a tag use case would probably be better served by something like <placeholderlink> and the <nolink> route seems to suit the span markup.
It feels natural to address the simple placeholder hyperlink use case first...
Comment #19
Wim LeersShouldn't this also have a test for
UrlGenerator
?Comment #20
chx CreditAttribution: chx at Smartsheet commentedPerhaps. It won't be me, though.
Comment #21
dawehner@Wim Leers
What kind of test do you have in mind? The YML definition is pretty clear, this is tested thousands already for
<none>
Comment #22
Wim LeersIt just seems like all special cases should be tested explicitly. But I trust @dawehner's judgment: apparently this is less special than I thought.
Comment #23
xjmDiscussed with @Cottser, @effulgentsia, and @alexpott. This one should be targeted for 8.2.x now for the addition like #1543750: Allow menu items without path which it unblocks. (This means it can be committed at any time rather than being subject to the RC freeze.)
Thanks everyone!
Comment #24
alexpottDiscussed with @dawehner - the LinkGenerator shouldn't have to know anything about which tags to use. Whilst the solution in the attached patch is not perfect - moving the HTML generation to the value object is out of scope here.
Comment #25
dawehnerSmall improvement, still not ideal of course, but well, let's see where we can get in 8.2.x
Comment #27
catchCommitted/pushed to 8.2.x, thanks!
Comment #28
Wim LeersThis must be one of the funniest & funkiest subclasses in all of Drupal :)
"href first" -> ugh, just use actual HTML parsing rather than string parsing… But this is a pre-existing thing, this is just clarifying the comment.
Comment #29
andypostLooks this needs changes record!
Comment #30
dawehnerHere is an issue which introduces some similar value objects for URL: #2346787: Update Url to reflect that it handles both routes and non-routed URIs
Comment #31
rcodina CreditAttribution: rcodina commentedAny workaround to do this on Drupal 8.1.1?
Comment #33
risforrocket CreditAttribution: risforrocket commentedI tried as the link and I still get the error message "Manually entered paths should start with /, ? or #." in 8.2.x
Comment #34
jibran@risforrocket see #1543750-126: Allow menu items without path
Comment #35
ivriets CreditAttribution: ivriets commentedComment #36
ivriets CreditAttribution: ivriets commentedfor 8.2.x use
route:<nolink>
Comment #37
StG CreditAttribution: StG commentedThe
route:<nolink>
works more or less, but a major issue has been still present: https://www.drupal.org/node/2838351 (I use 8.2.6)EDIT
I found a solution (or a workaround) for this issue: If I set the parent menu item (with
route:<nolink>
path) to Show as expanded, it works as expected.Comment #38
platinum1 CreditAttribution: platinum1 commentedNow that the special_menu_items module is no longer supported, I tried route: on the latest 8.4.dev
I get the error message "Manually entered paths should start with /, ? or #." as well. Is there a different strategy for 8.4.dev?
Comment #39
brianwagner CreditAttribution: brianwagner commentedSeems to be some confusion here, me included:
You must type
in the form field, including the word 'route' and a colon.
Screenshot of form field: "route nolink"
Comment #40
blainelang CreditAttribution: blainelang commentedThanks! that works. Maybe it should be added to the field help text as one of the examples.
Comment #41
platinum1 CreditAttribution: platinum1 commentedPost #39 shows a space between "route:" and "".
IF someone copy & pastes this, the route is rejected, due to the space.
Comment #42
hass CreditAttribution: hass commentedThis is all very bad usability.