Collapse other fieldsets if a fieldset is clicked to open. [#1793096]

Hi guys,

Thanks a lot for the great work on this module, it has come in handy and useful in many situations.

I just wanted to follow up on a feature request or support issues that have probably come back in the tracker several times:

See ticket: #645762
http://drupal.org/node/645762

Pretty much, on many sites I found myself reproducing this fix several times (adding the classes, and theme/JS code), since in many cases the actual effect that would be expected would be:
Toggle open a fieldset, which would toggle close all other fieldsets in the same form/div from the same content section.

For example, with 3 fieldsets F1, F2, F3 and F1 is opened, if I clicked on F2, it would close F1.

But at some point, adding the classes and theme/js became rather repetitive and tedious (adding the classes to the content), so I've thought that perhaps coming up with a patch in the module could help with this feature/behavior).

I would like to know if you could potentially consider taking a quick look at the attached patch against 7.x-2.4 (File named: collapse_text-close_all_behavior.patch).
I've tried to keep the code to its minimal and add an unobtrusive option to the filter's form settings (by default is not active).

If this option is activated on the filter format settings form, then it will enable the behavior above.
I have already tested this feature together with other options and it seemed to work fine, but I would certainly be interested in getting more feedback and information on what could potentially be improved here.

I would greatly appreciate if you could take a bit of time to take a look at the patch and give me your feedback on this feature request.

Feel free to let me know if you would have any questions about the code or any other aspect of the patch, I would be glad to explain in more details.

Thanks in advance.

Comment	File	Size	Author
#5	collapse_text-collapse_all-behavior-1793096-5.patch	3.76 KB	DYdave
#5
#1	collapse_text-close_all_behavior-1793096-1.patch	2.39 KB	DYdave
#1
	collapse_text-close_all_behavior.patch	2.39 KB	DYdave

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

DYdave CreditAttribution: DYdave commented 24 September 2012 at 07:41

Status:

Active

» Needs review

File	Size
collapse_text-close_all_behavior-1793096-1.patch	2.39 KB

Quick clean up of the patch's coding standards (whitespaces and indent).
Changing status to ask for review.

Comment #2

jippie1948 CreditAttribution: jippie1948 commented 8 February 2015 at 09:55

Works perfect as such! :) Thanks.

Small question though. If I work with subfieldsets, opening one is closing the parent one. If that one could stay open when one of its children is opened, would be great!

Comment #3

thalemn CreditAttribution: thalemn commented 13 February 2015 at 23:21

I couldn't apply the patch with git, so did it manually and it works great. Thanks!

Comment #4

tommann CreditAttribution: tommann commented 16 April 2015 at 23:53

I manually applied the patch to 7.x-2.4 and get this error:
Notice: Undefined index: collapse_all in _collapse_text_filter_settings() (line 65 of /home1/bicorg/public_html/2015/sites/all/modules/collapse_text/collapse_text.module).

This is where I am trying to use it http://2015.bic.org/about/about-us#representatives

Any thoughts on a fix?

Thanks,
Tom

Comment #5

DYdave CreditAttribution: DYdave commented 29 April 2015 at 06:25

Related issues:

+#645762: Changing clicked behavior

File	Size
collapse_text-collapse_all-behavior-1793096-5.patch	3.76 KB

Hi guys,

Thanks a lot for getting back on this issue, testing the patch and reporting, it is definitely greatly appreciated.

Alright, let's get back to the problems encountered above, applying the patch or with error messages:

First of all, it is important to specify that currently the code of the 7.x-2.x (at 7da8520 development branch and its 7.x-2.x-dev) and the 7.x-2.4 (stable release) are the same to the only difference of the collapse_text.test which is only used for automated testing (not a source of any usage problems).

In other words, applying the patch to the stable, dev or GIT versions would lead to the exact same results.

I have tested applying the patch from #1 against the stable version (7.x-2.4), dev (7.x-2.x-dev), and the code from GIT (7.x-2.x branch) and didn't get any error applying the patch (I used TortoiseGIT).
In other words, the patch applied properly, without errors.

That's when I started testing and encountered the issue reported at #4:
When browsing to the filter/editor's configuration page, at admin/config/content/formats/full_html for example:

Notice: Undefined index: collapse_all in _collapse_text_filter_settings() (line 65 of /sites/all/modules/collapse_text/collapse_text.module).

Without saving the configuration (still with the "previous" full_html) profile configuration, I added a new content item with collapse text's markup in the body and got the following errors, when displaying node/1 (for example):

Notice: Undefined index: collapse_all in _collapse_text_process_child_item() (line 546 of /sites/all/modules/collapse_text/collapse_text.module).
Notice: Undefined index: collapse_all in _collapse_text_process_child_item() (line 546 of /sites/all/modules/collapse_text/collapse_text.module).

Then, I edited again the full_html configuration and checked the option: Collapse all other fieldsets on the page.
After saving all error messages detailed above disappeared.

Obviously it appears these error messages would be caused by a missing key/index in an array (I'd guess the input filter's settings), which would make sense, since the collapse_all index was added by the patch without the case where the key wouldn't exist.

To conclude this diagnosis, the problems reported above are actually rather minor....
1 - I was able to apply the patch quickly without any errors (7.x-2.x, 7.x-2.x-dev, 7.x-2.4).
2 - I was able to make the errors disappear after saving the configuration and the patch still worked as expected.

The problem comes from the function: collapse_text_filter_info, where the collapse_all property was not added to the filter's declaration array.

@tommann,

This is where I am trying to use it http://2015.bic.org/about/about-us#representatives

I checked the website you indicated but was unable to find any collapsible sections of text. I would be curious to see where Collapse Text is used on this website.
Could you please give us a little bit more details? (a screenshot would be great).

Next, getting back to #2:

If I work with subfieldsets, opening one is closing the parent one. If that one could stay open when one of its children is opened, would be great!

Indeed, I tested the feature with subfieldsets and encountered the same problem: when clicking on a subfieldset, the parent fieldset would be closed, as all other fieldsets, preventing users from looking at the content inside the clicked fieldset.
I guess, I didn't test or envision the case of nested fieldsets in the first version of the patch. This could probably be considered as a bug of this new feature.

So I went ahead and modified the collapse_text.js file to support subfieldsets.

Result:
Alright, please find attached to this comment the updated patch as an attempt to fix all the issues reported above:
File attached as: collapse_text-collapse_all-behavior-1793096-5.patch.

I would greatly appreciate to have your feedback on this updated patch.

Feel free to let me know if you would have any questions about the code, this comment, this ticket or any other aspects of the patch, I would be glad to explain in more details.

Thanks in advance to all for your reviews, testing, reporting and feedback.
Cheers!