CollapsiblePlugin does not work from report

[kenclary@…]

When writing a custom report, you can use the "description" column to display wikiformatted text (e.g. "description AS description"). This performs wiki formatting, including macro expansion.

However, this collapsible macro does not work in that context.

It would appear that the report page does not load folding.js, nor does it call $(".foldable").enableFolding(true, true). I suspect that is the source of the problem.

(Having collapsable sections in report result tables would be very nice.)

[Ryan J Ollos]

There's a proposed solution in my ​GitHub branch. You can grab it through:

svn co https://github.com/rjollos/trachacks/branches/traccollapsible/collapsibleplugin/0.12

Please let me know if it's working well for you. If it is working well, and codingking is okay with me pushing the changes, then I'll commit them to collapsibleplugin/0.12​.

Future improvements could include:

• Allowing autofold and snap to be specified as arguments.
• Make this work at heading levels other than h3.
• Making this function also as a WikiProcessor.

[codingking]

Hey, thanks for taking a look at this. I appreciate it. I should be able to test it out on Friday.

In the future if you want to collaborate on developing the plugin on Github, the "official" repo is here (I'm manoyes on Github):

​https://github.com/manoyes/collapsibleplugin

[kenclary@…]

The prospective version works for my immediate needs.

[Ryan J Ollos]

There's one small change we might want to make. I think we don't need the conditional enclosing add_script since the use of a set will prevent the script from being added to ant pages more than once.

[kenclary@…]

upon further testing...

The change appears to have fixed folding sections in reports, but caused them to freeze shut in wiki pages. (On both Chrome and Firefox, at least.)

[kenclary@…]

I suspect the new issue is caused by enableFolding being called on the same object more than once. Which is probably a bug with enableFolding, though I'm sure there's a workaround.

[Ryan J Ollos]

Replying to kenclary@…:

I suspect the new issue is caused by enableFolding being called on the same object more than once. Which is probably a bug with enableFolding, though I'm sure there's a workaround.

enableFolding adds an anchor to each link (no1, no2, ...) so it is clear that it wasn't designed to be called more than once on a page.

I tried a few different ways to work around the issue that enableFolding can't be called more than once on a page, eventually setting on ​this change.

If the other Trac developers agree, I might add the feature to Trac in a forthcoming release (albeit in a different form that CollapsiblePlugin, instead using WikiProcessor arguments).

[kenclary@…]

I was able to fix my issues with the below patch.

The particular problem I was having was that .enableFolding keeps repeatedly binding a click handler, so if called more than once, you have more than one function handling clicks. In this case, if you've called .enableFolding an even number of times, clicks appear to do nothing (by rapidly unfolding and refolding the section).

As best I can tell, it manages to not re-add the anchor each time. So, if one were looking to just patch enableFolding, having it be "trigger.unbind("click").click(...)" should solve the problem.

Index: traccollapsible/collapsible.py
===================================================================
--- traccollapsible/collapsible.py      (revision 109)
+++ traccollapsible/collapsible.py      (working copy)
@@ -33,7 +33,7 @@
             add_script(formatter.req, folding_chrome_path)
         add_script(formatter.req, 'collapsible/collapsible.js')
 
-        return '<div><h3 class="foldable">%s</h3><div>' % content
+        return '<div><h3 class="foldable collapsibleplugin">%s</h3><div>' % con
tent
 
     def get_htdocs_dirs(self):
         from pkg_resources import resource_filename
Index: traccollapsible/htdocs/collapsible.js
===================================================================
--- traccollapsible/htdocs/collapsible.js       (revision 109)
+++ traccollapsible/htdocs/collapsible.js       (working copy)
@@ -1,3 +1,18 @@
+(function($){
+
+  $.fn.unbindFolding = function() {
+    return this.each(function() {
+      // Use first child <a> as a trigger
+      var trigger = $(this).children("a").eq(0);
+      if (trigger.length != 0) {
+        trigger.unbind("click");
+      }
+    });
+  }
+
+})(jQuery);
+
 jQuery(document).ready(function($) {
-    $('div h3.foldable').enableFolding(true, false);
+    $('div h3.collapsibleplugin').unbindFolding();
+    $('div h3.collapsibleplugin').enableFolding(true, false);
 });

[Ryan J Ollos]

Replying to kenclary@…:

As best I can tell, it manages to not re-add the anchor each time.

The issue I saw was not with re-adding the anchor. The issue is when a second call to enableFolding adds a link element, it creates it with id=no1, restarting the numbering. Having duplicated id elements is violation of the HTML spec. This is in addition to the problem you noted about the folding behavior not functioning correctly. I mentioned the issue with the id attributes because it makes it clear that enableFolding was not designed to be called more than once on a page.

Anyway, your patch will mess-up the "snap-to" behavior on several pages. Whether you care about that or not I don't know.

[kenclary@…]

Anyway, your patch will mess-up the "snap-to" behavior on several pages. Whether you care about that or not I don't know.

As it turns out, on the pages where I'm using the plugin, there are no pre-existing uses of folding (that use snap-to, at least). In the short term, I have a working solution.

However, I'm still interested in seeing a cleaner implementation of the feature --- both to fix my bug and to allow for use-by-use control of the starting folded state and snap-to. From what I can tell, that requires either a significant rewrite of trac's built-in folding feature, or the plugin rolling its own folding code.

[Ryan J Ollos]

Replying to kenclary@…:

As it turns out, on the pages where I'm using the plugin, there are no pre-existing uses of folding (that use snap-to, at least). In the short term, I have a working solution.

I think the changes I posted earlier today avoid the bug.

However, I'm still interested in seeing a cleaner implementation of the feature --- both to fix my bug and to allow for use-by-use control of the starting folded state and snap-to. From what I can tell, that requires either a significant rewrite of trac's built-in folding feature, or the plugin rolling its own folding code.

Setting the folding state should be possible with selective application of the collapsed class. You can see how its done on the ticket page. I think the best course would be to just integrate the feature into Trac, so if you are interested in helping with that we can prepare the changes in ​trac:#11550.

[kenclary@…]

As it turns out, on the pages where I'm using the plugin, there are no pre-existing uses of folding (that use snap-to, at least). In the short term, I have a working solution.

I think the changes I posted earlier today avoid the bug.

I just tested with your earlier changes. I note 2 problems:

1. If I use the macro in a report view for a report that also has dynamic variables (which cause the report to show a foldable section for the dynamic variable setting), my report-generated sections don't fold. (They work correctly on reports without dynamic variables.)
2. On regular wiki pages, which call .enableFolding(true, true) on every .foldable at document ready, the plugin works, but always has snap. (Whereas the version from my patch added them without snap, which I've decided I like better.)

Setting the folding state should be possible with selective application of the collapsed class.

I believe you also need to bind a click handler, yes?

I think the best course would be to just integrate the feature into Trac, so if you are interested in helping with that we can prepare the changes in ​trac:#11550.

I am interested in helping (though I may continue to use my less-than-ideal patch in the meantime).

[kenclary@…]

Of note: I was able to do a cleaner local modification. It is still not perfect (it relies on any .enableFolding calls having happened before, not afterwards, but that seems to be the case for instances where trac itself is adding them).

traccollapsible/collapsible.py

@@ -29 +29 @@
     def expand_macro(self, formatter, name, content):
 
         # Make sure we don't call enableFolding more than once on a page.
-        folding_chrome_path = 'common/js/folding.js'
-        if folding_chrome_path not in formatter.req.chrome['scriptset']:
-            add_script(formatter.req, folding_chrome_path)
-            add_script(formatter.req, 'collapsible/collapsible.js')
+        #folding_chrome_path = 'common/js/folding.js'
+        #if folding_chrome_path not in formatter.req.chrome['scriptset']:
+        #    add_script(formatter.req, folding_chrome_path)
+        add_script(formatter.req, 'collapsible/collapsible.js')
 
-        return '<div class="collapsed"><h3 class="foldable">%s</h3><div>' % content
+        return '<div class="collapsed"><h3 class="foldable collapsibleplugin">%s</h3><div>' % content
 
     def get_htdocs_dirs(self):
         from pkg_resources import resource_filename

traccollapsible/htdocs/collapsible.js

@@ +1 @@
+(function($){
+
+  $.fn.enableCollapsiblePlugin = function() {
+    return this.each(function() {
+      var trigger = $(this).children("a").eq(0);
+      var contents;
+      if (trigger.length == 0) {
+        contents = $(this).html();
+        $(this).text("");
+      } else {
+        contents = trigger.html();
+        trigger.remove();
+      }
+      trigger = $("<a href='javaScript:void(0);'></a>");
+      trigger.html(contents);
+      $(this).append(trigger);
+
+      trigger.unbind("click").click(function() {
+        var div = $(this.parentNode.parentNode).toggleClass("collapsed");
+        return !div.hasClass("collapsed");
+      });
+      trigger.parents().eq(1).addClass("collapsed");
+    });
+  }
+
+})(jQuery);
+
 jQuery(document).ready(function($) {
-    $('div h3.foldable').enableFolding(true, false);
+    $('div h3.collapsibleplugin').enableCollapsiblePlugin();
 });

[Dirk Stöcker]

This is referencing a no longer existing GitHub repository.

Is this still an issue? If so, does someone have the complete solution?

[anonymous]

Replying to Dirk Stöcker:

This is referencing a no longer existing GitHub repository.

Is this still an issue? If so, does someone have the complete solution?

10 years later, I had forgotten all about this thread.

I believe the code, with my edits above, has been humming along, unchanged, all this time. I have no idea if CollapsiblePlugin or the main trac code has been updated to address this; I'm assuming neither has. Looking back through this thread, it would seem we couldn't agree on what the actual cause was.

Does anyone else use CollapsiblePlugin in reports?

[kenclary@…]

above comment is me; I even forgot how I (didn't) log in here...

[Dirk Stöcker]

Can you upload the version you're running? Then I can integrate the functionality. Also please provide a test case (i.e. what do I need to setup so I see if it works or not).

I took over maintainership now and updated the plugin for 1.6.

[kenclary@…]

Replying to Dirk Stöcker:

Can you upload the version you're running? Then I can integrate the functionality. Also please provide a test case (i.e. what do I need to setup so I see if it works or not).

I'm fairly sure that the patches in comment:13 are the sum total of local changes I ran with. (Getting into that system and checking would be at bit of a pain, now, unfortunately.)

For a test: well, have a page that uses [[CollapsibleStart]]...[[CollapsibleEnd]] a few times, and also implement them in a report (in a description AS description column). And see if both sets of foldable sections work as intended. As per the ticket description.

[Dirk Stöcker]

The patch in comment:13 is against the GitHub version, which is already enhanced. Could probably be reconstructed, but with complete source it would be easier.