The FIRST click you do on a page to an item that is collapsable. (only the first)

The class the tuns into:" dhtml-menu collapsed start-collapsed dhtml-folder expanded"
The collapse does not get removed. Bug.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

marcoka’s picture

Status: Active » Needs review
FileSize
4.67 KB
1.05 KB

attached patches should fix that.
http://drupal.org/node/1178226 This patch is already in the attached patches

General problem was the following:
Somehow core or taxonomy menu adds its own class "collapsed" "expanded" to menu items, so this results in double classes because dhtml menu classes are called collapsed and expanded too. i renamed them to be "collapsed-dhtml" and "expanded-dhtml"

My tests were successfull and it worked for me.

Please test.

vthirteen’s picture

against which files should these patches be applied?
[EDIT]
collapsed_expanded_PHP.patch against dhtml_menu.theme.inc
collapsed_expanded_JS.patch against dhtml_menu.js

is that right?

marcoka’s picture

hm crap netbeans did not add the files.....boah.
yes that is right i think.

vthirteen’s picture

OK, but now links just do not expand while in static navigation option 1 "No collapsing" is checked...

vthirteen’s picture

without the above patches, the DHTML menu expand as expected but it closes up again when the new page is loaded. not friendly.
after patching code, whatever option is set under "Other effects" the menus always stays collapsed and only first level parents are shown and clickable. not usable.

Peasoup’s picture

Patches did not work for me :/ No change at all.

michaelfavia’s picture

FileSize
5.86 KB

Here is the patch from above in a more standard unified git format. it seems to be functional but changes the css class to add dhtml.

felixSchl’s picture

The patches work for me, however there remains a problem. In the following case, the classes are not set correctly:

-Set Dhtml Module to remember opened menus
-Open a submenu item in a Dhtml menu
-Close the menu while still viewing the page
-Refresh

You will see the menu is set back to being expanded (which i think is cool), however, it is missing the dhtml-menu classes, hence we get the normal drupal arrow icon. It worked before the patch because dhtml menu would also use the expanded classes.

I think there might be away around it with using the active trail classes and such, but I though I just point it out here.
Cheers

felixSchl’s picture

Is anyone else experiencing the following issue: When clicking on a menu entry that has children, it won't get the "expanded-dhtml" class?

Now when I click the bullet link, it first adds the "expanded-dhtml" class and only on clicking AGAIN it will exchange it with the "collapsed-dhtml" class. This means I have to click twice to collapse it.

Edit:
I fixed it, the problem is that the JS script only saves elements with the class "expanded-dhtml" into the cookie array. By clicking a link without expanding it first, it won't be saved into the cookie array, hence not loaded when the page is being build. As far as I can see there are two solutions to this:
1) Fix the JS to take in the clicked element
2) Change the preprocess function to set the current menu item to be expanded

Since I have just enough knowledge about JS to understand what is going on, I fixed it in the preprocess function for me. Solution feels kind of crude, but well - it's working. Here's the code:

if( in_array($variables['element']['#attributes']['id'], $cookie) || in_array('expanded', $variables['element']['#attributes']['class'])) {
    $variables['element']['#attributes']['class'][] = 'expanded-dhtml';   
  }
michaelfavia’s picture

FileSize
5.65 KB

Fixed the patch above to properly namespace the classes and removed some debugging. Comment #89 feels like a new issue but i undertsand why you put it here with the patch. Lets see if we can get this commited to the dev branch and test against that.

michaelfavia’s picture

Title: expanden/collapsed » Expand and collapse broken on first attempt to change state
Assigned: Unassigned » michaelfavia

Renaming issue for better clarity

michaelfavia’s picture

FileSize
5.92 KB

Sorry for the bugspam but it looks like the currently active menu trail wasnt getting assigned the "dhtml-expanded" class because it was previousl maked as expanded by the menu module to begin with. Since we introduced our own handlers above (still not sure this is the best direction but it works for now) we need to set an initial state for expanded items ourselves.

michaelfavia’s picture

Ok Christoph was foolish enough to give me git access. In an effort not to make him regret it on the first commit to the dev branch id like to make sure we all think that not using the menu provided "expanded/collapsed" css classes makes sense because of their possible unintended interaction with other modules like menu.module and fancy_menus, etc.

If you can test the latest patch on a clean d7 branch and we get a consensus ill commit this bad boy.

@chritoph your opinion would be very appreciated here because this is changing css selectors that were probably used externally in theming. Going to test myself tonight and see what conclusions we come to.

michaelfavia’s picture

FileSize
5.47 KB

Remove static init for ajax prevention and deal with that in the other issue.

tahiticlic’s picture

The patch doesn't work on last dev version since on hiding, when "None" behavior is selected, the link is reached.

The definition of root and siblings should be outside the test in Drupal.dhtmlMenu.switchMenu function, leading to :

Drupal.dhtmlMenu.switchMenu = function(li, link, ul, open) {
  // No need for switching. Menu is already in desired state.
  if (open == li.hasClass('dhtml-expanded')) {
    return;
  }

  var effects = Drupal.settings.dhtmlMenu.effects;
  
  // If the relativity option is on, select only the siblings that have the same root
  if (effects.siblings == 'close-same-tree') {
    var root = li.parent();
  }
  else {
    var root = $('ul.menu');
  }
  var siblings = root.find('li.dhtml-expanded').not('.own-children-temp').not('#' + id);

  if (open) {
    Drupal.dhtmlMenu.animate(ul, 'show');
    li.removeClass('dhtml-collapsed').addClass('dhtml-expanded');

    // If the siblings effect is on, close all sibling menus.
    if (effects.siblings != 'none') {
      var id = li.attr('id');
      /* Siblings are all open menus that are neither parents nor children of this menu.
       * First, mark this item's children for exclusion.
       */
      li.find('li').addClass('own-children-temp');



      // If children should not get closed automatically...
      if (effects.children == 'none') {
        // Remove items that are currently hidden from view (do not close these).
    	  $('li.dhtml-collapsed li.dhtml-collapsed').addClass('sibling-children-temp');
        // Only close the top-most open sibling, not its children.
    	  siblings.find('li.dhtml-expanded').addClass('sibling-children-temp');
        siblings = $(siblings).not('.sibling-children-temp');
      }

      // The temp classes can now be removed.
      $('.own-children-temp, .sibling-children-temp')
        .removeClass('own-children-temp')
        .removeClass('sibling-children-temp');

      Drupal.dhtmlMenu.animate(siblings.find('ul:first'), 'hide');
      siblings.removeClass('dhtml-expanded').addClass('dhtml-collapsed');
    }
  }
  else {
    Drupal.dhtmlMenu.animate(ul, 'hide');
    siblings.removeClass('dhtml-expanded').addClass('dhtml-collapsed');

    // If children are closed automatically, find and close them now.
    if (effects.children == 'close-children') {
      // If a sub-menu closes in the forest and nobody sees it, is animation a waste of performance? Yes.
    	siblings.find('li.dhtml-expanded')
        .removeClass('dhtml-expanded').addClass('dhtml-collapsed')
        .find('ul:first').css('display', 'none');
    }
  }
}
Arvid Herfjord’s picture

Quick and dirty fix

Add the following to dhtml_menu.css:

li.dhtml-folder.collapsed.expanded {
  list-style-image: url("images/folder-open-white.png") !important;
}

That did it for me :-)

Rory’s picture

I'm using the DHTML Menu 'Static navigation' > 'None' option and I found a decent workaround solution.

Enable 'Show as expanded' for each parent menu item under the items' menu settings. This applies to any items in a single menu that contain other menu items, including children that are parents of other menu items.

AjitS’s picture

Status: Needs review » Reviewed & tested by the community

The solution provided by @Arvid worked for me!

Rory’s picture

Status: Reviewed & tested by the community » Active

The initial collapsed or expanded class assigned to the HTML by the module needs to be looked at, instead of changing the CSS to disguise the problem. Hiding the issue is not the same as fixing it, it's making things worse :)

Until someone is able to submit a proper patch, this issue should remain open. @Arvid has admitted it's a dirty solution. You shouldn't have an item that is both collapsed AND expanded at the same time.

atiba’s picture

#17 Thanks Rory, your suggestion worked for me. Simply because 'Show as expanded' will provide a different class different then .collapsed.

It's a nice workaround, but definitely isn't a fix. I'm surprised this issue is for about 3 years now and still isn't patched yet.

BWPanda’s picture

Component: CSS code » PHP Code
Assigned: michaelfavia » Unassigned
Issue summary: View changes
Status: Active » Needs review
FileSize
923 bytes

I'm also experiencing this issue (having both 'collapsed' and 'expanded' classes on list-items at the same time) and believe it is because there are two 'collapsed' classes on the same list-item; so when the code says to remove 'collapsed' and add 'expanded', it only removes one 'collapsed' and hence you get the problem.

This patch seems to fix the issue for me. Others should test to make sure it works for them too.

vuil’s picture

FileSize
923 bytes

Update the patch of #21 to the latest 7.x-1.x-dev branch.

  • ilchovuchkov committed 55ce511 on 7.x-1.x
    Issue #1178234 by by michaelfavia, marcoka, BWPanda, ilchovuchkov:...
vuil’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

vuil’s picture