By xano on

There is this menu_local_tasks function, but that only puts a nice unordered list around the tabs. The actual list items (which I would like to style) are generated through a general list theme function. I want to provide each tab with a unique icon, so every tab needs a unique ID. How would I do this without brute force (using regular expressions)?

⋅ Categories: Drupal 5.x

Comments

monkeybeach’s picture

monkeybeach commented

At least, judging by the lack of replies on every post I've found regarding this. ;-)

I can't understand why these are so difficult to theme nor why there they don't have CSS hooks by default given the rest of Drupal is pretty easy to modify.

Perhaps this should be a core change?

monkeybeach’s picture

monkeybeach commented

I finally made some headway on this after running across some code quite at random

<ul class="tabs">
<?php if (user_access('administer nodes')): ?>
<li id="edit-button"><a href="<?php global $base_url; print $base_url;?>/node/<?php print $node->nid ?>/edit" title="<?php print t('Edit') ?>">Edit</a></li>
<?php endif; ?>
<?php if (user_access('administer nodes')): ?>
<li id="view-button"><a href="<?php print '' . url("node/$node->nid") . ''; ?>">View</a></li>
<?php endif; ?>	
</ul>

As it stands you should be aware that this works purely for the View and Edit buttons. If other button states are given (such as 'clone' and 'export' buttons shown by $tabs on views) you won't see them.

Also the because a view doesn't have a nid as such the edit and view buttons href's break entirely when looking at a view.

Still its start :-)

prattboy’s picture

prattboy commented

Do you have an example of the markup that is being generated by the general list theme function?

monkeybeach’s picture

monkeybeach commented

Have a look over here: http://drupal.org/node/157876

The issue is the links for Edit / View aren't easily themeable without rewriting something like 5 or 6 functions (so I'm told).

Seems overkill just to put two CSS classes in :-)

Gold medal to you if you can think of a simple clean way to do this cos its been doing my head in for days!

prattboy’s picture

prattboy commented

If Javascript is an option, my Javascript guru recommends the following Jquery code. You can insert Jquery via Drupal, but I usually do an external JS file that I reference in my template:

$("ul.tabs li a").each(function(idx, item){
    if($(item).html() == "Edit"){
        $(item).before("<img src='editIcon.png'/>");
    }
    if($(item).html() == "View"){
        $(item).before("<img src='view.png'/>");
    }
});

I haven't tested it, so there's the possibility the syntax might be off. You'll also have to make sure this runs after the page has loaded.

Basically, here's what happening: the Javascript looks for every link that is contained in an li in a ul with a class of "tabs". It reads the text that is in the a-href, and then if it matches one of the strings, it loads the right image before the a-href. So, if the text is equal to "Edit" it inserts the image editIcon.png before the a-href

Having to resort to a Javascript fix, of course, is less than ideal, but it prevents you from hacking the core.

monkeybeach’s picture

monkeybeach commented

Yeah JavaScript isn't the best solution but it appears to be the only one so far!

I'll keep looking for a more robust solution, but I'll implement the JS as a quick fix and test and post back with results and any changes that might have been necessary.

Thanks for your help :)

dvessel’s picture

dvessel commented

This'll do it. Theme override for menu_local_task. Converts the title of links into classes. menu_item_link() has the option to return as array (second parameter). The default is to render the link. Here it's done up as an array, then the title is cleansed and placed in as a class.

It would be better to use the path information instead ('href' key) but this is easier.

function phptemplate_menu_local_task($mid, $active, $primary) {
  // Get menu link as array.
  $menu_item = menu_item_link($mid, FALSE);
  // characters to clean for classes. Add as many as needed. regex may be overkill.
  $clean = array(' ', '_');
  // keep lowercase. create classes from title.
  $class = strtolower(str_replace($clean, '-', $menu_item['title']));
  // get active state.
  $class .= $active ? ' active' : '';
  
  return "<li class=\"$class\">". l($menu_item['title'], $menu_item['href'], $menu_item['attributes']) ."</li>\n";
}

Using ID's is a bad approach and not necessary in this case. Hook in your styles through the classes. Use descendant selectors to be safe. i.e. ".tabs .edit { ... }".

monkeybeach’s picture

monkeybeach commented

Much better than the hack I was coming up with. This is great, thanks v much :-)

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

Great idea! I would recommend to only allow word characters (with [a-z], so no special characters like ä, " etc.) for class names though. In English those characters probably aren't an issue, but in other languages they are. A regex is indeed a bit of a heavy load for such a simple task, but it would be too bad if there would be titles that make the HTML 'crash'. The second thing I'd like to say is that ID's are the proper attribute to use here, because you're trying to identify unique tabs.

function phptemplate_menu_local_task($mid, $active, $primary) {
  // Get menu link as array.
  $menu_item = menu_item_link($mid, FALSE);
  // Keep lowercase. Create IDs from title.
  $id = strtolower(preg_replace('#(^[a-z][0-9]-_)#', '', $menu_item['title']));
  // Get active state.
  $class = $active ? 'active' : '';
 
  return '<li id="'.$class.'"($class ? $class : '')>'. l($menu_item['title'], $menu_item['href'], $menu_item['attributes']) .'</li>'."\n";
}

I've added a little regex to create the IDs. All characters except non-special letters, numbers, dashes and underscores are being stripped. Classes are now only used to distinct active tabs from non-active ones.

dvessel’s picture

dvessel commented

Okay, the reason I said using the 'href' key would be harder was because some of the paths omit the last part. Example: "example.com/node/1" instead of "example.com/node/1/view". But I just realized that you can get the raw menu information by using menu_get_item(). So here an improved version. No need to clean up the class.

function phptemplate_menu_local_task($mid, $active, $primary) {
  $raw_menu = menu_get_item($mid);
  // Convert path into array then pop off the last element into $class.
  $class = array_pop(explode('/', $raw_menu['path']));
  $class .= $active ? ' active' : '';
  return "<li class=\"$class\">". menu_item_link($mid) ."</li>\n"; 
}

All we needed to do was to convert the path information into an array and pull the last element. So, for a link like "example.com/node/1" would produce a class of "view". Very consistent.

And the reason why I said using ID's is a bad approach is because there's the possibility to have duplicates. What if a primary local task shared the title or even the path in this case with the secondary. You'd have duplicates. And who knows when you'll run into a page with a similar ID from the local tasks.

It's better to use classes with descendant selectors. That way you can target exactly what you need with a minimal chance that you'd run into conflicts.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

Okay, you're right about the IDs, although I don't agree with you on the way you make the classes. What if a developer has two tabs which point to /node/[nid]/view and to /custompath/view? Wouldn't it be possible to use the tab ID? Perhaps not as semantic as we'd like, but the chance of conflicts would be zero.

dvessel’s picture

dvessel commented

What if a developer has two tabs which point to /node/[nid]/view and to /custompath/view?

If your talking about those paths being on the same page in the same set of primary local tasks, then I think it's highly unlikely. Definitely wouldn't be the case in core but if a contrib module was overloaded with tabs it would be possible but who's going to style every iteration of a tab? :)

If the same class for the tab needs to be styled differently for other pages then you can set a body class with something like this..

Inside template.php:

function _phptemplate_variables($hook, $vars) {
  if ($hook == 'page') {
    // Get a very specific location of the site.
    $i = 0;
    $vars['id_attr'] = 'loc';
    while ($arg = arg($i++)) {
      $vars['id_attr'] .= '-'. $arg;
    }
    // Gets the general location of the site.
    $vars['classes'] = arg(0);
  }
  return $vars;
}

inside page.tpl.php:

<body id="<?php print $id_attr; ?>" class="<?php print $classes; ?>">

Then use those descendant selectors. Drill down from the body id or class and into the tabs.

The tab ID or menu id would mean styling for very specific cases. Bad idea since it can change moving across installations. Best approach is to be as general as possible then use descendant selectors to narrow into specific elements.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

but who's going to style every iteration of a tab? :)

I am. I'm using icons for the tabs instead of text. It makes them nice and small so I can stuff them away in some corner so they don't take up any unnecessary space ;-)

But I didn't mean the same classes for different tabs on different pages. I meant the same classes for different tabs on the same page. I know it's highly unlikely, but it's possible and because everybody's trying to make Drupal a sturdy and foolproof CMS, why shouldn't we? I think it might take the same amount of code to use the tab ID (if there is one) for the classes, which makes them unique.

prattboy’s picture

prattboy commented

In that case (and if I'm understanding correctly), you might want to give the containing element for the tab a unique id and keep the classes on the tab elements. That might me more semantically correct (and I think that's what dvessel is suggesting, too). Just because you are giving an element an ID doesn't mean that Drupal won't use that ID later on in the node. Technically it shouldn't happen, and that's where you're going to run into rendering problems in browsers.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

I was talking about tab ID's (I mean the one used by Drupal to distinct tabs) if they exist. These are unique and in combination with a prefix it provides a perfect value for the ID attribute of the tabs. If you use the right prefix (not one already used elsewhere on the page) this is the perfect solution and 100% foolproof.

prattboy’s picture

prattboy commented

If you're sure that the ID won't pop up later on, then go for it. There's nothing technically wrong with that. However, I'd still recommend putting an ID on a containing element and keeping classes on the tabs, just in case... so your menu would be called like this:

<div id="myUniqueMenuID">

/*Your code to create your menu with classes for each menu item*/

</div><!--myUniqueMenuID-->

Then you'd style each tab like this:

#myUniqueMenuID li.view {
/*My Styles For the View Tab*/
}

That way, if for some reason you wanted the menu you are creating to appear someplace else on the page without icons (like in the footer as text only links) you could reuse the code you are using to generate the menu and just change the ID of the containing element.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

I think that's up to the developer although it's a good idea if you're planning on using the same tabs on one page multiple times. My point was about the creation of the tabs themselves which is supposed to be done in the theme function. A container element should be put into place in a *.tpl.php file and not in this function.