Hi,
As part of the collapsible container feature I required a need for configuring the default state also: Collapsed/normal for forums/Container and based on that initial page load should display the forums.
(http://drupal.org/node/291084#comment-4062936)
@Torky responded:
Configuring default state per forum/container complicates code in way I don't like and I am not eager to do it in current phase of development.
My idea to do this would be creating multiselect listbox on config page with list of all forums/containers and then in preprocess function add 'collapsed' class to selected container variable and adjusting template file to use it. I think javascript doesn't need to be altered in this case.

Based on Torky's suggestion I made changes to advanced_forum 6.x-2.x-dev (March 30th release). It works fine with the list of containers as Im using 'forum_containers' .
If required the same can be enhanced for forums but wondering how to go about.
Attached is the patch for this. Any feedback/review on this please let me know.

Thanks.

Comments

Anonymous’s picture

Status: Active » Reviewed & tested by the community

great work
applied and tested on my site

mcdruid’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new4.47 KB

Hi vidu_ram, sorry for the long delay in getting this looked at.

I've attached a patch of your changes which I fixed up to apply to the latest 2.x dev release.

However, as I fixed up your patch I noticed that you're doing the checking of containers against the advanced_forum_default_collapsed_list in the JS, rather than the preprocess function as suggested by troky.

Could you explain why you've done it this way (and your initial_load implementation) rather than through preprocess? I can think of some reasons, but would like to understand your thinking.

AFAICS this does work, and I'd like to get the functionality committed.

vidu_ram’s picture

Hi Mcdruid,
Thanks for the response and sorry from my end too for the little delay in responding.
The patch works pretty fine.
Reason for going in with changes in js is first time I want the listing (collapsed or non collapsed) based on the default configuration and later on wanted to remember the status of the list and list accordingly, this way the user is benefited as it lists the way the user was browsing the forum list.
Let me know your thoughts on the same.
Thanks
vidu_ram

michelle’s picture

Status: Needs work » Needs review

The question was answered. Setting back to needs review to move things along.

mcdruid’s picture

Patch rolled again to apply to latest dev - needs to be reviewed.

mcdruid’s picture

Status: Needs review » Fixed

Thanks for this vidu_ram, and sorry it has taken such a long long time to get in.

Committed to 6.x-2.x-dev

mcdruid’s picture

Just committed a small tweak to this, as it was causing in an error on the settings page:

warning: Invalid argument supplied for foreach() in [snip]/modules/advanced_forum/includes/settings.inc on line 129.

diff --git a/includes/settings.inc b/includes/settings.inc
index 2cfd5e4..8018459 100644
--- a/includes/settings.inc
+++ b/includes/settings.inc
@@ -124,7 +124,7 @@ function advanced_forum_settings_page() {
   );
 
   // For default collapsed state configuration
-  $collapsed_list_name = variable_get('forum_containers', '');
+  $collapsed_list_name = variable_get('forum_containers', array());
   $collapsed_list_description = array();
   foreach ($collapsed_list_name as $id) {
     $collapsed_list_description[$id] = taxonomy_get_term($id)->name;
vidu_ram’s picture

Thanks for the update.
Will the same be put forth in 7.x also?

troky’s picture

Version: 6.x-2.x-dev » 7.x-2.x-dev
Status: Fixed » Needs review
StatusFileSize
new5.75 KB

Patch adapted for 7.x... Please test.

vidu_ram’s picture

Thanks troky.
Tested it. It works fine.

Note: When applying patch using git apply af7_1121208_1.patch
got error as fatal: patch with only garbage at line 5
So tried applying patch with patch -p1 < af7_1121208_1.patch
went through fine.

troky’s picture

Status: Needs review » Fixed

Committed. Thanks for the patch.

Status: Fixed » Closed (fixed)

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

vidu_ram’s picture

Hi Troky,
When will this feature get added up in the release version?
Thanks
vidu_ram

troky’s picture

it is in dev already. 2.1 release soon.

vidu_ram’s picture

Thanks Troky. Saw it in dev version but wanted to know when it will be part of release version.