Modify

Opened 9 months ago

Last modified 9 months ago

#11627 new defect

CollapsiblePlugin does not work from report

Reported by: kenclary@… Owned by: codingking
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 (13)

comment:1 Changed 9 months ago by rjollos

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 9 months 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 9 months ago by kenclary@…

The prospective version works for my immediate needs.

comment:4 Changed 9 months ago by rjollos

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 9 months 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 follow-up: Changed 9 months 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 9 months ago by rjollos

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 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 1, edited 9 months ago by rjollos (previous) (next) (diff)

comment:8 follow-up: Changed 9 months 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 9 months ago by rjollos

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 9 months ago by rjollos (previous) (diff)

comment:10 follow-up: Changed 9 months 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 9 months ago by rjollos

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 9 months ago by rjollos (previous) (diff)

comment:12 Changed 9 months 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 9 months 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 9 months ago by rjollos (previous) (diff)

Add Comment

Modify Ticket

Action
as new The owner will remain codingking.
Author


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

 
Note: See TracTickets for help on using tickets.