Problem/Motivation

When a block is set to be collapsed by default, it is not collapsed on a page if the content of the block has a link to the page you are viewing.

Eg. A block with the user menu is created and set to collapsed by default. On the front page the block works as expected, however on the user page (there is a link to the user page in the user menu and therefore in the block) the block is expanded by default and the menu link is styled as an active link.

Proposed resolution

This may be a feature on purpose in which case it would be nice to have an option to disable this.

Otherwise, we need to find a way to stop this from happening.

Remaining tasks

  • TBC
CommentFileSizeAuthor
#6 2447245-3.patch3.46 KBsonvir249
#2 blocks_are_expanded-2447245-2.patch862 bytesyanniboi
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yanniboi’s picture

yanniboi CreditAttribution: yanniboi commented
Related issues: +#1877972: Collapsiblocks not staying collapsed, +#2336347: Block with a menu is always expanded

I believe that these issues are probably related to the one I have described.

yanniboi’s picture

yanniboi CreditAttribution: yanniboi at CTI Digital commented
Status: Active » Needs review
FileSize
blocks_are_expanded-2447245-2.patch862 bytes

So looking at the code in collapsiblock.js it looks like this is desired behaviour. But I definately do not want that to be forced. The attached patch removes it, however I think an option on the block configuration page might be nice allowing users to choose whether the block is expanded on active pages.

MariskaTh’s picture

MariskaTh CreditAttribution: MariskaTh commented

Hello,

Thank you for this module. It really helped me out. I'm not sure my problem is related to the current issue, but just in case I post this in here.

I have also set a menu in a block and set that to 'Collapsed by default'. The block seems to work fine. The only problem is that it doesn't collapse after a page refresh. So the block is collapsed, I expand it and click on a menu item. The website goes to the other page and then the block is still expanded. I can collapse it by clicking it, but I would expect that a page refresh would result in that the blocks that are set to 'Collapsed by default' would automatically collapse again after a page refresh.

Am I right or is what I see the expected behaviour?

Thanks,
Mariska.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
CreditAttribution: darvanen as a volunteer commented

Initial discussion of this issue and potential remediation is in #1953062: Collapsed by default not compatible with sortable table.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
CreditAttribution: darvanen as a volunteer commented
Status: Needs review » Needs work

The patch in #2 is not backwards-compatible, it removes something that may be seen as a desired feature by some.

sonvir249’s picture

sonvir249 CreditAttribution: sonvir249 as a volunteer and at QED42 commented
Status: Needs work » Needs review
FileSize
2447245-3.patch3.46 KB

Hi @Darvanen,

  1. I appreciate you suggestions, I have added changes as suggested.
  2. $(this).find('a.active').size() search in whole page that is why we can not add as a per-block setting.
  3. We can make another issue for Drupal coding standards after resolving the issues. First we should focus on functional issues.
darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
CreditAttribution: darvanen as a volunteer commented
Status: Needs review » Needs work
  1. Thanks that looks like it'll work
  2. +++ b/collapsiblock.js
@@ -79,8 +80,8 @@
+          if (stat ==  4 || (cookieData[id] == 0 || (stat == 3 && cookieData[id] == undefined)) && (!$(this).find('a.active').size() || activePages === 1)) {

    $(this) refers to the block defined in line 16: 

    $(block + ':not(.collapsiblock-processed)', context).addClass('collapsiblock-processed').each(function () {

    .find() only searches descendants (children).

    So it can be done as a per-block setting. 

  3. +++ b/collapsiblock.module
@@ -12,7 +12,6 @@ function collapsiblock_init() {
-    $collapsiblock_path = drupal_get_path('module', 'collapsiblock');

    This is not a functional change, it's just removing an unused variable, which is a good thing to do, but that's what I was suggesting we should leave out of this.

This is coming along nicely, great work :)

It's probably a good idea to practice giving your patches the recommended name (put a short description in front of the issue code and comment number).

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
CreditAttribution: darvanen as a volunteer commented

Oh and don't forget to add the new setting to the uninstall hook.

  • Darvanen committed 8261490 on 7.x-1.x authored by sonvir249 
    Issue #2447245 by yanniboi, sonvir249: Blocks are expanded with active...
darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
CreditAttribution: darvanen as a volunteer commented
Status: Needs work » Fixed

Thanks @sonvir249 and @yanniboi.

I added an uninstall hook and tried to make the JS a bit more readable, but you are the credits for this commit.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
CreditAttribution: darvanen as a volunteer commented

Oh and I did test this with menu blocks, the language switcher block from core, and sortable tables - all worked.

  • Darvanen committed 8261490 on 7.x-2.x authored by sonvir249 
    Issue #2447245 by yanniboi, sonvir249: Blocks are expanded with active...

Status: Fixed » Closed (fixed)

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