We have a lot of frighteningly similar theme functions that only output links, but links don't even need to be run through the theme layer. For more information on what should and what shouldn't be output / overridable via the theme system.

Functions:

Templates:

  • views-more.html.twig

Remaining tasks

#1833932: Remove theme_system_compact_link() and replace with a #type link render array
#2031301: Remove theme_more_link() and replace with #type 'more_link'
#2036195: Remove views-more.html.twig and replace with #type link render arrays
#1763964: Use #type => link for theme_aggregator_block_item()
@TODO - use route name and parameters instead of system path

completed Tasks:

#2031305: Remove theme_more_help_link() and replace with a #type link render array
#1778610: Remove the check for a link template from l(), have l() always output just a string.
#1778990: Merge theme_pager_link*() theme functions into theme_link()
#1598886: Clean up pager theme functions
#1222248: Remove theme_book_title_link() use l() instead
#1222260: Remove theme_filter_tips_more_info() from core
#1833934: Remove theme_toolbar_toggle() from core.

Related tasks

#2031341: Move theme_container into theme.inc
#1804614: [meta] Consolidate theme functions and properly use theme suggestions in core
#1812684: [meta] Consolidate all table templates and add theme_hook_suggestions
#1819284: [meta] Consolidate all form element container templates, and add theme_hook_suggestions
#1813426: [meta] Consolidate all item list templates and add theme_hook_suggestions
#1812724: Consolidate all form element templates and add theme_hook_suggestons

Files: 
CommentFileSizeAuthor
#6 1595614-6-consolidate_on_theme_link.patch2.24 KBeojthebrave
FAILED: [[SimpleTest]]: [MySQL] 47,853 pass(es), 5 fail(s), and 0 exception(s).
[ View ]

Comments

I'd even go one step further and ask:

Do we need the wrapper DIV at all nowadays?

If not, then both of them are just theme_link().

Suppose we actually need some wrapper element to help themers make their magic

I think we could just use theme_link with some additional suggestions so that if someone wanted too they could easily override theme_link__more or theme_link__more_help and add whatever wrappers they needed.

Issue summary:View changes

added another

Issue summary:View changes

added related issues

Issue summary:View changes

added more funcs and issues

Issue summary:View changes

more funcs and issues

Issue summary:View changes

added pager link

Issue summary:View changes

added book title api link

Issue summary:View changes

add theme_system_compact_link

Issue summary:View changes

added toolbar toggle

Issue summary:View changes

add pager first

Issue summary:View changes

added theme_pager issues

Issue summary:View changes

remove file

Issue summary:View changes

remove dupe issue

Title:[meta] Remove all the theme functions in core that simply output a link. Replace with l()Remove or consolidate many similar link functions in core

There are a lot of theme functions in core that only serve to output links. I'm going to update this issue to keep track of them and hopefully we can clean them all out as we move to Twig :) I love the idea of one theme_link function with more theme hook suggestions so each one can be overridden if necessary.

Issue summary:View changes

theme link link

Issue summary:View changes

summary thingy

Title:theme_more_link() vs. theme_more_help_link()Remove or consolidate many similar link functions in core
StatusFileSize
new2.24 KB
FAILED: [[SimpleTest]]: [MySQL] 47,853 pass(es), 5 fail(s), and 0 exception(s).
[ View ]

Here's an example of what this would look like to remove theme_more_link and replace with theme('link__more', $variables).

This is probably just a matter of going through and doing this with all of these functions, and figuring out any special cases.

Issue summary:View changes

Fix links.

Issue summary:View changes

added one for book title link

Issue summary:View changes

added another

Issue summary:View changes

remove dupe

Issue summary:View changes

remove other dupe

Title:Remove or consolidate many similar link functions in core[meta] Remove all the theme functions in core that simply output a link. Replace with l()

updating title

Issue summary:View changes

add more

Issue summary:View changes

remove list

Issue summary:View changes

added link to issue

Issue summary:View changes

add another related

Issue summary:View changes

move issues to match theme funcs

Title:Remove or consolidate many similar link functions in core[meta] Remove all the theme functions in core that simply output a link. Replace with l()

I've just touched on what #4 was pointing out in the parent meta issue: #1833920: [META] Markup Utility Functions. I happen to think the same on this issue as #4: that these theme functions should not be replaced with just l() but they should be replaced with link sub-pattern suggestions. However this walks a fine line between what is justifiable sub-pattern or not (see that issue for details).

Sidenote, if we could introduce a friendly pattern for all templates to use some sort of standard wrapper syntax, then we could even achieve the div "more-link" wrapper, instead of pushing the class into the Anchor tag as in #6.

Title:[meta] Remove all the theme functions in core that simply output a link. Replace with l()Remove or consolidate many similar link functions in core
Status:Active» Needs review

As #7 is going from the wrong assumption that a l() link is changeable in the template outputting it (It is not), re-titling back and marking #6 as CNR.

Sidenote, if we could introduce a friendly pattern for all templates to use some sort of standard wrapper syntax, then we could even achieve the div "more-link" wrapper, instead of pushing the class into the Anchor tag as in #6.

Yeah, this is only possible with #theme_wrappers property in a render array or mis-using #prefix and #suffix. (don't do that)

I think overall this needs to be decided on a case per case basis.

#6 could be also done as part of the theme_node_recent_content function directly and providing the values as $variables and you then need to overwrite this template to change the more link, which might be acceptable. However then you could also use a render array (brrr) directly.

I think what Jen was thinking of was a Link component, which is a printable object instead of a render array, that allows both complete display and display of all separate parts like in attributes.

Status:Needs review» Needs work

The last submitted patch, 1595614-6-consolidate_on_theme_link.patch, failed testing.

Issue summary:View changes

linky

Issue summary:View changes

remove menu link since that's not quite the same thing

Issue summary:View changes

move menus to related not remaining

I think what Jen was thinking of was a Link component, which is a printable object instead of a render array, that allows both complete display and display of all separate parts like in attributes.

This sounds a bit like #1213510: A modern t(). It'd have the advantage that you don't need to do most of the processing until __toString() is called, which could be handy. Could be worth opening a new issue for?

Title:Remove or consolidate many similar link functions in core[meta] Remove or consolidate many similar link functions in core

Issue summary:View changes

update theme link issue

Issue summary:View changes

move link to related

Title:[meta] Remove or consolidate many similar link functions in core[meta] Remove all the theme functions in core that simply output a link. Replace with #type 'link' render arrays

this seems like a better issue title based on the discussion here.

Status:Needs work» Active

meta issues are just "active" until they're closed, I believe.

Issue summary:View changes

add more related issues

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

related

Issue summary:View changes

remove self

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Strike through theme functions that are gone

Issue summary:View changes

update

Issue summary:View changes

update

Issue summary:View changes

Updated issue summary.

Issue summary:View changes

Updated issue summary.

Title:[meta] Remove all the theme functions in core that simply output a link. Replace with #type 'link' render arrays[meta] Remove all the theme functions and templates in core that simply output a link. Replace with #type 'link' render arrays

Issue summary:View changes

Updated issue summary.

Since #2042285: #theme_wrappers should be able to have a unique set of variables per wrapper hook is in, it would be possible to rebuild a lot of the theme implementations we removed as #type 'foo_link' element types based on #type 'link'.

When I say "based on #type 'link'" I mean something like this:

  $types['link'] = array(
    '#pre_render' => array('drupal_pre_render_link'),
  );
  // Links that lead directly to foo.
  $types['foo_link'] = $types['link'];
  $types['foo_link'] += array(
    '#theme' => array('link'),
    '#theme_wrappers' => array(
      'container' => array(
        '#attributes' => array('class' => 'foo-link'),
      ),
    ),
    '#title' => t('Foo here!'),
    ...
  );

We could set re-usable defaults for convenience and to help ensure consistency of markup for functionally related elements throughout core, without creating a bunch of duplicate functions/templates to render the markup. Maybe even a tiny, baby step towards #1804488: [meta] Introduce a Theme Component Library?

If contrib/custom code wants to re-implement theme_link() that we removed, it will be automatically apply to all our different link types. Unless of course, the custom/contrib code doesn't want it to, in which case it can simply implement hook_element_info() to pick out the link types it would like to have bypass theme().

Issue summary:View changes

Updated issue summary.

based on #2047619: Add a link generator service for route-based links , we should start using #route_name and #route parameters instead of #href where possible

based on #2047619: Add a link generator service for route-based links , we should start using #route_name and #route parameters instead of #href where possible

wow, that is just *not* what the conclusion of that issue was. According to that issue's summary (and discussion between myself and pwolanin in IRC and the issue queue), we're not planning to convert links to #route_name and #route parameters except where access checks for l() are currently involved. "where possible" is not right.

Issue summary:View changes

Updated issue summary.

Issue summary:View changes