You've already mentioned this in your TODO list, but this is to remind you ;) i'll also add a patch for this in this issue today.

CommentFileSizeAuthor
#7 195247_themable_5.x_7.patch6.67 KBdeviantintegral
#7 195247_themable_6.x_7.patch7.04 KBdeviantintegral
#4 OperaScreenSnapz001.png26.3 KBdeviantintegral
#1 themable.patch5 KBcasey

Comments

casey’s picture

casey commented
Status: Active » Needs review
StatusFileSize
new themable.patch5 KB

patch contains the following changes:

  • Moved HTML-generation to a theme_hook called theme_tableofcontents_toc($toc, $options)
  • Moved drupal_add_js and drupal_add_css() also to theme_tableofcontents_toc(), so these are themable also
  • Removed toggle-link from HTML; only usefull when js is enabled
  • Changed tableofcontents.js so it adds toggle-link automaticly

I also would remove tableofcontents.css (I don't think it is necessary), but I didn't include this in the patch.

casey’s picture

casey commented

hold on... another one is coming in a few hours (first some sleep :p)

casey’s picture

casey commented

make it a few days... no time right now

deviantintegral’s picture

deviantintegral commented
StatusFileSize
new OperaScreenSnapz001.png26.3 KB

Thanks for the patch! A few suggestions:

  • I like what you've done for the JS. I think having a seperate 'hide' link was better though. What would be really neat would be if it toggled between 'hide' and 'show', and then was hidden completely when JS is disabled.
  • Something in the styling is causing list numbers to render outside of the
    (see screenshot). This might just be something in my site, can you check on your end too? It might not be obvious if you don't have a background colour set.
  • We should keep the CSS included, as it means that 'by default' the table will look decent. It also gives an example for developers to work off of.

    • --Andrew

casey’s picture

casey commented

I am working on another patch (including fixes for multiple-toc's-issue and prevent-appearing-in-teaser-issue), but next week I have some preliminary examinations for my study. It's pretty much done but I haven't thoroughly tested it yet.

casey’s picture

casey commented

see http://drupal.org/node/200404

deviantintegral’s picture

deviantintegral commented
StatusFileSize
new 195247_themable_6.x_7.patch7.04 KB
new 195247_themable_5.x_7.patch6.67 KB

I've created a new patch which implements proper theme functions. I'd appreciate some more testing before I commit it. The Drupal 5 version is only different in that it doesn't translate "hide" as that feature requires Drupal 6.

--Andrew

deviantintegral’s picture

deviantintegral commented
Status: Needs review » Fixed

I've been running this for a few days without any issues. I've committed both patches. If any specific issues come up, please create new issues describing them.

Thanks!
--Andrew

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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