theme_book_title_link() (http://api.drupal.org/api/function/theme_book_title_link/7)

We have a theme function that makes a block title a link. Clearly this indicates a lack of functionality in block.tpl.php that's needed and should be addressed.

This theme function needs to be removed and the functionality that it is enabling needs to be standardized.

Similar Issues

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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Jacine’s picture

Maybe we should just add the link in a preprocess function and call it a day.

RobLoach’s picture

I was really confused as to why we added like 10 of these different link theming functions that did exactly the same thing. Decided to stay out of that conversation. In any case, I'm a +1 here.

Jacine’s picture

@RobLoach Do you know of anymore that are lurking around? Let's kill them too. :)

Jacine’s picture

Title: Remove theme_book_title_link() and standardize its functionality » Remove theme_book_title_link()
Component: theme system » markup
Issue tags: -html5 +theme system cleanup

In an effort to get a better picture of issues remaining in the HTML5 Initiative, we are removing the "html5" tag from issues that are not directly HTML5-related. Tagging this "theme system cleanup" instead because that's more accurate.

Note: Any issues assigned to the "Markup, CSS and JavaScript" components will still be broadcast on the HTML5 Twitter feed so that interested parties are aware and can participate.

andypost’s picture

suppose a lot of elements could be rendered with theme_html_tag() so removed also this reduce a theme registry

RobLoach’s picture

http://api.drupal.org/api/search/8/_link?page=1 ... But the book one is the most obvious.

sun’s picture

Status: Active » Needs review
Issue tags: +Theme Component Library
FileSize
1.68 KB

Like this?

Status: Needs review » Needs work

The last submitted patch, drupal8.theme-book-title.7.patch, failed testing.

sun’s picture

Component: markup » book.module
Status: Needs work » Needs review
FileSize
1.68 KB

doh, sorry.

Status: Needs review » Needs work

The last submitted patch, drupal8.theme-book-title.9.patch, failed testing.

andypost’s picture

Status: Needs work » Needs review
FileSize
2.46 KB

Suppose we need to change a theme_link() to safely check $options['html]

sun’s picture

+++ b/core/modules/book/book.module
@@ -322,7 +319,8 @@ function book_block_view($delta = '') {
       $tree = menu_tree_all_data($node->book['menu_name'], $node->book);
       // There should only be one element at the top level.
       $data = array_shift($tree);
...
+      $block['subject'] = theme('link__book_title', $data['link']);

It's possible that I'm mistaken here...

The link in $data contains the menu system information. I'm not sure whether that is compatible with theme_link() -- e.g., I think it contains a 'href' attribute (whereas 'path' is the original router path), and 'options' might not exist.

Let's make sure that we're working with the right data ;)

vlad.dancer’s picture

Status: Needs review » Needs work

We don't need to remove theme func., mentioned here How to convert theme functions to Twig templates

#12 sounds like very reasonable, we need add preprocess func and do 'href to path' staff there.

jenlampton’s picture

Status: Needs work » Needs review
Issue tags: +Twig
jenlampton’s picture

The function theme_book_title_link() exists in book module for the sole purpose of adding a class, book-title, to the link in the subject of a block.

function theme_book_title_link($variables) {
  $link = $variables['link'];
  $link['options']['attributes']['class'] = array('book-title');
  return l($link['title'], $link['href'], $link['options']);
}

As it turns out, this class is never used. Let's remove the class, and the function it rode in on! All we need in book.module is a call to l if we remove the useless class. Removing the need for this theme function at all.

jenlampton’s picture

Title: Remove theme_book_title_link() » Remove theme_book_title_link() use l() instead

updating title

sun’s picture

Status: Needs review » Reviewed & tested by the community

That makes sense in this case, since the link is actually output as the title of a block. The block provides sufficient context to theme the link in the block title already.

Fabianx’s picture

+1 for RTBC

catch’s picture

Title: Remove theme_book_title_link() use l() instead » Change notice: Remove theme_book_title_link() use l() instead
Priority: Normal » Critical
Status: Reviewed & tested by the community » Active

Yep, can't see a good reason for this either. Committed/pushed to 8.x. This probably needs a change notice, or adding to an existing one documenting theme functins

tim.plunkett’s picture

Title: Change notice: Remove theme_book_title_link() use l() instead » Remove theme_book_title_link() use l() instead
Priority: Critical » Normal
Status: Active » Fixed
jenlampton’s picture

woot!

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Updated issue summary. Add similar issue