The theme hooks (theme_tableofcontents_number, theme_tableofcontents_toc) do not currently adhere to the spirit of proper theme-able functions. They contain much business logic so I am wary of overriding either one in my theme for fear that future module updates will break. Please update these functions so that only HTML theming is performed here. It would also be nice to add a theme_tableofcontents_title() or something which allows one to theme the text after the number.

Comments

AlexisWilke’s picture

AlexisWilke commented

Hi tangent,

The theme_tableofcontents_number() function is very simple. It transforms $toc->counters[$toc->level_to] into a number. And if you want to include all the numbers at all levels, you can use the loop:

for ($idx = $toc->level_from; $idx <= $toc->level_to; ++$idx) {
  $result .= $toc->number_separator . _tableofcontents_convert_number($mode, $toc->counters[$idx]);
}

The rest of the logic is just if you want to allow the user flags to work. You can as well ignore them if you are programming your own theme for your customer or yourself. The mode and numbering settings are there to allow more defaults.

Actually, someone asked to get different numbers at different levels. Not a bad idea, just annoying to have to setup each level. So it could have been even more complicated. My idea was that the user could revamp the function to, for instance, call the _tableofcontents_convert_number() with a different mode based on the level.

As for the other function, theme_tableofcontents_toc, I have to agree that it is a complicated one and you probably want to support many of the features in that function...

Yet, all it does is generate HTML. I guess what we could do is have lower level theme functions that this very function calls, if that makes sense? It would make everything slower but more versatile.

Thank you.
Alexis Wilke

P.S. btw, it is not very likely that the D6 version will change that much more... maybe to have some problems fixed with time.

tangent’s picture

tangent commented

Ideally, theme_tableofcontents_number() and other theme functions would look something like the following.

function theme_tableofcontents_number($number) {
  if ($number) {
    return '<span class="toc-number">' . $number . '</span>';
  }
}

function theme_tableofcontents_text($text) {
  if ($text) {
    return '<span class="toc-text">'. $text .'</span>';
  }
}

Code that currently calls theme('tableofcontents_number') would need to call something like _tableofcontents_number() instead. This would allow themers to override these theme functions to use their own html for these elements.

AlexisWilke’s picture

AlexisWilke commented
Assigned: Unassigned » AlexisWilke
Status: Active » Fixed

Okay, I added two themes:

  • tableofcontents_toc_text
  • tableofcontents_number_text

Let me know whether that works for you or not. You'll need to reset the theme registry. You can do so with mini module or by running update.php or some other admin module.

Thank you.
Alexis Wilke

See:
http://drupalcode.org/project/tableofcontents.git/commit/b9b8d9b

Status: Fixed » Closed (fixed)

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