Closed (duplicate)
Project:
Drupal core
Version:
8.0.x-dev
Component:
theme system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
7 Oct 2013 at 20:09 UTC
Updated:
23 Jul 2015 at 16:35 UTC
Jump to comment: Most recent
Comments
Comment #1
cosmicdreams commentedThe fact that your example shows
mean it has a code smell. Anytime I see an empty argument I think it has a bad code smell. Why not structure your arguments so that the unneeded argument is last and therefore could be omitted?
Comment #2
markhalliwell@cosmicdreams, I updated the issue summary to reflect that this issue was simply a follow up to a comment posted by @Fabianx. I doubt we'll be using that syntax at all. It will likely follow a similar pattern using routes (like the follow-up issue). The blanks argument is actually for text of the link (which is 99% used). In this case, I think @Fabianx was just giving an example of what is possible with setting variables via Twig and not needing to provide the text. In all actuality, it will probably be something similar to:
Comment #2.0
markhalliwellUpdated issue summary.
Comment #2.1
markhalliwellUpdated issue summary.
Comment #3
star-szr#2073811: Add a url generator twig extension was not a major task so I don't think this should be. Can we outline the main benefits/use cases and the plan in the issue summary? The related issue talks about active class handling for example and I assume this would be an extension for l(). Thanks!
Comment #4
dawehnerWell, one usecase is consistency. If you work example need the active class on there, it is impossible to get it set automatically.
Comment #5
joelpittet@dawehner Can/could the urlGenerator object provide that active route/path state?
Pardon my ignorance, I've not got a chance to dig into the routing/urlGenerator internals too deep yet.
Comment #6
dawehner@joelpittet
Yeah links are tricky, sadly.
There are a couple of problems with just using direct html, as you suggested
hook_link_alterComment #7
joelpittetOuch yeah they are tricky now eh? JS to set the active class:S That sounds like a last ditch resort to deal with caching... hmm.
Maybe we should talk to Wim and see where that is going, if he knows?
If a linkGenerator is the only way we can ATM deal with active links in templates... that will need to go in but I'd much rather see a smart Url object could let the template know of it's state I think.
Comment #8
dawehnerWhat we could also do is to use the new link object we have and put the needed information into that.
Comment #9
wim leersIt's all explained in https://www.drupal.org/node/2167077, specifically in the "Solution" part, second point.
In short: you must use
LinkGenerator. AFAIK/IIRCUrlobjects are simple value objects, so this definitely would not be great:Why can't this "link generator twig extension"… use
LinkGeneratorunder the hood? The names match perfectly even!Comment #10
wim leersComment #11
dawehnerNote: In order to solve #1777332: Replace theme_menu_link() and menu-tree.html.twig with a single Twig template I introduced such a link generator.
Comment #12
pivica commentedNote that #1777332: Replace theme_menu_link() and menu-tree.html.twig with a single Twig template introduced very basic link implementation, meaning you can only pass path text and url but not the rest stuff like attributes. This link implementation is already used in menu.html.twig which means that currently it is not possible to inject additional classes or any other attributes while rendering menus with twig.
UPDATE: just found separate issue for attribute problem #2342745: Allow Twig link function to pass in HTML attributes.
Comment #13
joelpittetGlad you found it:)
Comment #14
Crell commentedSo um, sounds like this is done, then, no? You *can* add links from a template, right...? We can close this?
Comment #15
joelpittetYes this is a duplicate thanks.
Comment #16
joelpittetDuplicate of... #1777332: Replace theme_menu_link() and menu-tree.html.twig with a single Twig template where it snuck in;)