Modify

Opened 10 years ago

Last modified 5 months ago

#11627 assigned defect

CollapsiblePlugin does not work from report

Reported by: kenclary@… Owned by: Dirk Stöcker
Priority: normal Component: CollapsiblePlugin
Severity: normal Keywords:
Cc: Trac Release: 1.0

Description

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.)

Attachments (0)

Change History (20)

comment:1 Changed 10 years ago by 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.

comment:2 Changed 10 years ago by 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

comment:3 Changed 10 years ago by kenclary@…

The prospective version works for my immediate needs.

comment:4 Changed 10 years ago by 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.

comment:5 Changed 10 years ago by 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.)

comment:6 Changed 10 years ago by 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.

comment:7 in reply to:  6 Changed 10 years ago by 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 anchors to each link no1, no2, ..., so it wasn't designed to be called more than once on a page. I tried a few different ways to work around this, 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).

Version 0, edited 10 years ago by Ryan J Ollos (next)

comment:8 Changed 10 years ago by 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);
 });

comment:9 in reply to:  8 Changed 10 years ago by 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.

Last edited 10 years ago by Ryan J Ollos (previous) (diff)

comment:10 Changed 10 years ago by 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.

comment:11 in reply to:  10 Changed 10 years ago by 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.

Last edited 10 years ago by Ryan J Ollos (previous) (diff)

comment:12 Changed 10 years ago by 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).

comment:13 Changed 10 years ago by 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

     
    2929    def expand_macro(self, formatter, name, content):
    3030
    3131        # Make sure we don't call enableFolding more than once on a page.
    32         folding_chrome_path = 'common/js/folding.js'
    33         if folding_chrome_path not in formatter.req.chrome['scriptset']:
    34             add_script(formatter.req, folding_chrome_path)
    35             add_script(formatter.req, 'collapsible/collapsible.js')
     32        #folding_chrome_path = 'common/js/folding.js'
     33        #if folding_chrome_path not in formatter.req.chrome['scriptset']:
     34        #    add_script(formatter.req, folding_chrome_path)
     35        add_script(formatter.req, 'collapsible/collapsible.js')
    3636
    37         return '<div class="collapsed"><h3 class="foldable">%s</h3><div>' % content
     37        return '<div class="collapsed"><h3 class="foldable collapsibleplugin">%s</h3><div>' % content
    3838
    3939    def get_htdocs_dirs(self):
    4040        from pkg_resources import resource_filename
  • traccollapsible/htdocs/collapsible.js

     
     1(function($){
     2
     3  $.fn.enableCollapsiblePlugin = function() {
     4    return this.each(function() {
     5      var trigger = $(this).children("a").eq(0);
     6      var contents;
     7      if (trigger.length == 0) {
     8        contents = $(this).html();
     9        $(this).text("");
     10      } else {
     11        contents = trigger.html();
     12        trigger.remove();
     13      }
     14      trigger = $("<a href='javaScript:void(0);'></a>");
     15      trigger.html(contents);
     16      $(this).append(trigger);
     17
     18      trigger.unbind("click").click(function() {
     19        var div = $(this.parentNode.parentNode).toggleClass("collapsed");
     20        return !div.hasClass("collapsed");
     21      });
     22      trigger.parents().eq(1).addClass("collapsed");
     23    });
     24  }
     25
     26})(jQuery);
     27
    128jQuery(document).ready(function($) {
    2     $('div h3.foldable').enableFolding(true, false);
     29    $('div h3.collapsibleplugin').enableCollapsiblePlugin();
    330});
Last edited 10 years ago by Ryan J Ollos (previous) (diff)

comment:14 Changed 5 months ago by Ryan J Ollos

Owner: changed from codingking to Dirk Stöcker
Status: newassigned

comment:15 Changed 5 months ago by 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?

comment:16 in reply to:  15 Changed 5 months ago by 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?

comment:17 Changed 5 months ago by kenclary@…

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

comment:18 Changed 5 months ago by 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.

comment:19 in reply to:  18 Changed 5 months ago by 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.

comment:20 Changed 5 months ago by 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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as assigned The owner will remain Dirk Stöcker.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.