Problem/Motivation

When trying to generate a link to a Drupal page in a template, it would be preferable to use the route name (the immutable machine name) rather than a system path that may be altered if the route path pattern is altered.

Proposed resolution

TBD

Remaining tasks

TBD

User interface changes

None

API changes

None

Follow-up from #2073811-59: Add a url generator twig extension (comment #59).

Original report by @Fabianx

Links are complicated. A link function could also use an OOP style approach - or just return variables or even just a link render array.

Example:

{{ link() }}

but also

{% set my_link = link(node.title, 'node.view', { 'nid': node.nid }) %}
<a href="{{ link.url }}" {{ link.attributes }}>{{ link.content }}</a>

But that is another discussion that needs to be done in a wider scope and also discuss how to support render arrays existing and not introduced here in the change of url() in twig to support routes.

Comments

cosmicdreams’s picture

The fact that your example shows

link('', 'node/1', is_path = true)

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?

markcarver’s picture

@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:

{% set my_link = link(node.title, 'node.view', { 'nid': node.nid }) %}
<a href="{{ link.url }}" {{ link.attributes }}>{{ link.content }}</a>
markcarver’s picture

Issue summary: View changes

Updated issue summary.

markcarver’s picture

Issue summary: View changes

Updated issue summary.

Cottser’s picture

Priority: Major » Normal
Issue summary: View changes
Issue tags: +Needs issue summary update
Related issues: +#2073811: Add a url generator twig extension

#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!

dawehner’s picture

Well, one usecase is consistency. If you work example need the active class on there, it is impossible to get it set automatically.

joelpittet’s picture

@dawehner Can/could the urlGenerator object provide that active route/path state?

<a href="{{ urlObj }}" class="{{ urlObj.isActive() ? 'is-active' }}">

Pardon my ignorance, I've not got a chance to dig into the routing/urlGenerator internals too deep yet.

dawehner’s picture

@joelpittet
Yeah links are tricky, sadly.
There are a couple of problems with just using direct html, as you suggested

  • You can alter links at the moment via hook_link_alter
  • In order to cache these outputs properly, you need to determine the active class via javascript, so you kind of similar logic like:
    // Set the "active" class if the 'set_active_class' option is not empty.
    if (!empty($variables['options']['set_active_class']) && !$url->isExternal()) {
      // Add a "data-drupal-link-query" attribute to let the
      // drupal.active-link library know the query in a standardized manner.
      if (!empty($variables['options']['query'])) {
        $query = $variables['options']['query'];
        ksort($query);
        $variables['options']['attributes']['data-drupal-link-query'] = Json::encode($query);
      }
    
      // Add a "data-drupal-link-system-path" attribute to let the
      // drupal.active-link library know the path in a standardized manner.
      if (!isset($variables['options']['attributes']['data-drupal-link-system-path'])) {
        // @todo System path is deprecated - use the route name and parameters.
        $system_path = $url->getInternalPath();
        // Special case for the front page.
        $variables['options']['attributes']['data-drupal-link-system-path'] = $system_path == '' ? '' : $system_path;
      }
    }
    
  • at some time in the future maybe the link generator will include some access checking on top
joelpittet’s picture

Assigned: Unassigned » Wim Leers

Ouch 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.

dawehner’s picture

What we could also do is to use the new link object we have and put the needed information into that.

Wim Leers’s picture

It's all explained in https://www.drupal.org/node/2167077, specifically in the "Solution" part, second point.

In short: you must use LinkGenerator. AFAIK/IIRC Url objects are simple value objects, so this definitely would not be great:

[…] I'd much rather see a smart Url object could let the template know of it's state I think.

Why can't this "link generator twig extension"… use LinkGenerator under the hood? The names match perfectly even!

Wim Leers’s picture

Assigned: Wim Leers » Unassigned
dawehner’s picture

Note: 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.

pivica’s picture

Note 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.

joelpittet’s picture

Glad you found it:)

Crell’s picture

Status: Active » Postponed (maintainer needs more info)

So um, sounds like this is done, then, no? You *can* add links from a template, right...? We can close this?

joelpittet’s picture

Status: Postponed (maintainer needs more info) » Closed (duplicate)

Yes this is a duplicate thanks.

joelpittet’s picture