I wanted a UI to attach blocks to my menu, but I'm handling dropdowns myself. Needed away to disable the more link. Patch attached.

CommentFileSizeAuthor
#3 2094457-menu-attach-block-no-drop.patch4.38 KBfearlsgroove
menu-attach-block-no-js.patch4.49 KBfearlsgroove

Comments

cameron tod’s picture

cameron tod commented

Thanks for the patch! I will let the testbot run and then review it later today.

cameron tod’s picture

cameron tod commented
Status: Needs review » Needs work

Just a quick review while I have 5 minutes...

  1. +++ b/menu_attach_block.info
@@ -7,8 +7,6 @@ dependencies[] = block
-scripts[] = menu_attach_block.js
-

    We don't want to remove this entirely. We just need an option to disable it. 

  2. +++ b/menu_attach_block.module
@@ -43,8 +43,8 @@ function menu_attach_block_theme() {
- * @todo Implement a theme function to do the render.
- *
+ * @todo Implement a theme function to do the render
+. *

    Not sure why the punctuation was lost here :) 

  3. +++ b/menu_attach_block.module
@@ -67,18 +67,21 @@ function menu_attach_block_link(&$variables) {
+        $link .= PHP_EOL . l(t('More'), '', $l_options);
+        drupal_add_js(drupal_get_path('module', 'menu_attach_block') . '/menu_attach_block.js');

    OK, I see you adding the JS back in here. It's probably better to add it on a per page basis anyway, although I might move it to #attached. 

  4. +++ b/menu_attach_block.module
@@ -223,13 +228,17 @@ function menu_attach_block_form_menu_edit_item_alter(&$form, &$form_state) {
+  // $form['menu_attach_block']['no_drop'] = array(
+  //   '#type' => 'checkbox',
+  //   '#title' => t('No drop link'),
+  //   '#default_value' => isset($form['options']['#value']['menu_attach_block']['no_drop']) ? $form['options']['#value']['menu_attach_block']['no_drop'] : FALSE,
+  // );

    I assume this is meant to be removed?

fearlsgroove’s picture

fearlsgroove commented
Status: Needs work » Needs review
StatusFileSize
new 2094457-menu-attach-block-no-drop.patch4.38 KB

Sorry .. terrible patch. Here's another try against -dev, which includes the ajax stuff. This does a couple things:

  • Adds a "None" option to orientation to prevent any class from being added
  • Adds a "Don't render javascript drop link" option. This is only available if "use ajax" is unchecked, since the ajax implementation seems to require the drop link.
  • Loads the JS with drupal_add_js to prevent universal inclusion.

Note that I couldn't get the ajax feature to work with or without this patch applied. Not sure what's going on there -- the attach function never seems to fire, even though the javascript is included?

designguru’s picture

designguru commented

Hi all,

This seems like an essential feature of this module - I'd love for users to simply click on the menu item title to open up the attached block, rather than the little carrot icon - can someone please write a patch which works for this?

I just tried the patch above and didn't see this functionality working - Ideally instead of writing a 'More' link which gets hidden with included css, the actual menu item name should be printed in the drop link (without printing the original title not linked to anything.)

q./

fearlsgroove’s picture

fearlsgroove commented

This patch is about removing the requirement that the built-in drop link be included. So if I'm handling dropdown menus myself already via css, superfish, nice menus, whatever, I don't want the cruft included at all. I want to be able to support providing a submenu attached directly to a top level menu without including an extra child menu item, but I still want to show the actual menu link associated with that item. I do NOT want the block to serve as the menu item itself -- that's already supported by using as the path for the menu item link. Nor do I want the drop link to be rendered as the block link -- the idea here is we're using our own css and javascript to handle that.

So if I create a top level menu item with a normal path as the link, attach a block and choose "No drop link", the outputted markup would look more or less like
UL.menu
--LI
----A <-- menu link
----div <-- block contents

cameron tod’s picture

cameron tod commented
Status: Needs review » Fixed
Issue tags: +Needs tests

Committed with attribution. Thanks!

Regarding the Javascript issue, I'm not having any issues here. Can you test again and post a new issue if you're still having trouble?

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