Merge Administration Menu Dropdown module

The 1.5 release fixes the incompatibility issue. As far as merging goes, I wouldn't be opposed to it; it would make my tasks easier :) You would have to discuss it with maintainers of the Admin Menu module. One issue I think would come up is compatibility; position:fixed would break compatibility IE 6 (though I'm not sure if Admin Menu officially supports IE 6- I however, don't). There's also some issues with FireFox 2 (see issue queue), however I'm inclined to believe they are issues with FF2's rendering of CSS being that they seem to be non-existent in FF 3 beta 3.

I have not talked to them about a merge. Right now, my only concerns with maintenance of Admin Menu Dropdown is that it is working with the latest 5.x release (and perhaps a 6.x branch when that happens). If you want such a merge to happen, you will have to take initiative on it. I do not know the Admin Menu maintainers plans & goals so the issues I've mentioned above could be a showstopper for Admin Menu.

A merge would be possible if AMD would implement conditional checks/workarounds for the already mentioned bugs in some browsers/platforms. For example, there are plenty of (JS-based) workarounds for position: fixed xbrowser compatibility, and I could even imagine that there is already a jQuery (slider?) plugin for this purpose.

On another thought, I would really like to see this module (AMD) evolve in terms of enhancing DAM's usability. It seems like there are already some users that really like the shortcut keys. For example, I've moved this issue into AMD's queue, which reduces DAM to the icon in the upper-left corner instead of hiding it completely. This specific idea could be enhanced so that the menu slides in on hovering the icon. Such type of things should happen in AMD. Be experimental, allow all kind of different usability enhancements. Whenever needed, I'm totally open to implement some hooks in DAM to allow even crazier things in AMD, or even just to make AMD's PHP/JS code smaller. Just let me know via DAM's issue queue.

Hi Sun,

Can you provide any more information on the position: fixed issue? Like a way of reproducing the issue or one of the workarounds? I've looked into a few css alternatives and a few js alternatives but nothing comes close to achieving the goal as well as the standard position: fixed.

Speaking as an outsider—but one who really (really!) likes both modules—it does seem to me this is a sensible suggestion... Admin Menu Dropdown is providing functionality that could sensibly be embedded within Admin Menu, rather than offering something entirely separate. I'd have thought it'd be useful to expose all users of Admin Menu to those options.

If this is going to happen, it'd make sense at this point to do this against 6.x-2.x-dev so I have updated the version on this issue.

Hi dbabbage,

I too agree that this would be sensible, and would love to do it, however it's not my decision, it's up to Sun. Last time I mentioned it to him he seemed interested, however there are no firm plans to do anything at the moment.

I am still open to discuss a module merger further.

Please note that position: fixed can be enabled via the regular settings page of admin menu now. (see #310423: Add optional position fixed configuration setting)

Also, #331982: Add support for keyboard navigation adds a nice keyboard navigation feature to admin menu. Just showing/hiding the menu never convinced me personally, but I love the idea of navigating through the menu using the keyboard. So maybe you could team up with the other developer of that issue, squeeze your menu toggle shortcuts somewhere in the patch and we're one step closer? ;)

That said, are there any other features in this module?

@Sun

I did notice the position: fixed change; in my opinion that was one of the most important features of Admin Menu Dropdown.
As for the keyboard navigation, I guess the reason I hadn't bothered looking into that thus far is that it seems to be completely outside of the scope of what Admin Menu Dropdown is, which is simply a toggleable visibility addon to Admin Menu.

I would be more than happy to start looking into writing a patch to add some Javascript extras (Admin Menu Dropdown + Keyboard Navigation) directly into Admin Menu to negate the need of a secondary module, but if you can layout some constraints of how you would like it done (ie, hooks for user addons, written as a sub-module, etc) it would be extremely helpful.

That's an interesting question, yes. Currently, more and more JavaScript-based features are added to admin menu, #220100: Add support to detect hover intent (no dependencies added) probably being the next one.

The fear I have is that not everyone wants to use those features. While hoverintent may be an exception to this, I think that further features should go into new JavaScript files that ship with admin menu, but are only loaded when the corresponding feature has been enabled in the admin menu's settings.

However, those additional features should follow a certain structure, and most importantly, they probably need to be executed in a certain stage. So I'm thinking about JavaScript "hooks", which allow add-on scripts and other modules to process the menu in certain stages. Note: These are completely wild and "unleashed" thoughts to have a starting point:

Similar to Drupal.behaviors, we define one or more stacks of functions to execute: Drupal.adminMenu.behaviors. Each registered function is executed once the DOM is loaded.

Hm. Turned out to be easier than I thought.

Attached patch should implement the core functions that will allow us to enhance administration menu's features via separate JavaScript files. As of now, there is only one @todo in the patch, which needs to be solved.

Also, moving this issue into admin menu's queue to (hopefully) gather more attention.

I actually started working on an option, which is a simple hook system similar to what I started working on in admin_menu_dropdown. The main difference is it would allow for two types of 'tweaks', visibility tweaks and stand-alone tweaks.

However, I probably should have waited so that this exact issue wouldn't occur, but I hoped that maybe you where situated in a timezone that was in sleep mode so that I could get some stuff done.

I'll put my stuff on hold for the minute and have a look at your patch and see if I can figure it out.

Edit: Ok, doesn't look like this should effect any of my work, will continue and post a patch once I've got something usable.

@sun

This patch incorporates your previous patch, and adds some minor hooks that allow for addon 'tweaks'.
So far there are two tweaks implemented, stanislav.mueller Keyboard navigation (a stand-alone tweak) and Admin Menu Dropdowns original key combo behavior (a visibility tweak).

The patch isn't 100% complete, I still want to cleanup the settings page and implement the Icon Hotspot visbility tweak, but I wanted to make sure I was going down an acceptable path.

Let me know what your opinion is and I will continue from there.

Patch attached.

Updated the patch with the Icon Hotspot visibility tweak and a cleaner settings page.

And one more for luck: Improved usability of visibility tweak settings form.

Uh oh. I did not review the entire patch, but: Eval is evil. Avoid using eval() at all costs.

However, the primary reason why I did not review it completely was that I did not really understand the purpose and what you are doing in this patch. Also, what is the difference between "standalone" and "visibility" tweaks? And why do we need JS(ON) callbacks for them? Are those executed on every page request? If they are, this needs to be rewritten.

If you can, please provide a more detailed description of your patch, similar to the one in #10.

Also, for simplicity, please remove the changes for keyboard navigation from this patch. Those should be dealt with in #331982: Add support for keyboard navigation only.

@Sun

I will have a look at an alternative to using eval, but it is there to call the user defined javascript function. I might be able to do something similar to the way you did with your JS changes.

The reason I defined two different type of tweaks is that every visibility tweak is essentially doing the same thing but in a different manner, therefore you never want to visibility tweaks operating at the same time. Visibility tweaks also have access to some predefined JS API functions that provide some of the more important functions, primarily setting the visibility status as a $_SESSION variable via the said JSON callback.
Standalone tweaks can do whatever they wish and therefore should not be restricted to only one running at anytime.

As said above, the JSON callback is to toggle the $_SESSION variable and as such is only called initially to fetch the visibility status, and then anytime the visibility status is toggled. The user who requested this feature initially suggested cookies to achieve the end but I felt a $_SESSION variable was a better fit. The point of this functionality is so that if a user toggles the state to hidden it will be hidden on the next page they load, and vice versa.
If you have a valid alternative to this I will be happy to re-write it.

As for the Keyboard navigation, I will remove that ASAP. The reasons I put it in was as suggested by you above, to show how it would integrate with my changes/hooks and to show an example of how a standalone tweak would be implemented.

Will re-write the patch within the next hour or so.

Cheers,
Deciphered.

Is it possible that you mean "admin_menu related tweaks" with "visibility tweaks", and "unrelated/common administration tweaks" with "standalone tweaks"? Sorry if I was not clear in my last reply - I actually do not understand the entire approach/concept and flow behind your patch. This does not mean it's bad or wrong, it's just that I would appreciate some "theory" we can discuss in parallel to code. And, I'm asking on purpose, especially because of #345984: Client-side caching of administration menu, which will need similar settings stored per user and/or session.

@sun

"visibility tweak" - A tweak/add-on designed only with the intent to toggle on/off the visibility of the Administration Menu.
"standalone tweak" - A tweak/add-on designed with any other purpose than the above.

While these may not be the best categories for tweak types, given that I am coming from AMD it is where my mind is at this point.

As for theory, the way I see it is as such:

- As I said earlier, visibility tweaks, tweaks that only deal with the visibility of the Administration Menu, can not run in parallel, therefore they need a definition so that a select box of choices can be provided.

- Once a visibility tweak has been loaded it will be activated via 'Drupal.adminMenu.visibility', a javascript function that works with your new modular system. The tweak itself can not be a direct part of your modular system as it wouldn't allow for multiple visibility tweaks to be defined in one .js file.

- 'Drupal.adminMenu.visibility' will run the 'init()' function of the defined visibility tweak via eval() to run it's initial behavior (binding events, etc), and then checks the default/set status of the Admin Menu visibility and sets it as such.

- The defined visibility tweaks must define the events to show and hide the menu, and point them to the helper functions of 'Drupal.adminMenu_visibility', which will set the visibility status in the $_SESSION data, set the 'Drupal.settings.admin_menu.tweak_visibility.hidden' variable and then pass back to the visibility tweak to actually process the hide/show function as defined.

- Standalone tweaks have the ability to set settings on the Admin Menu settings form, but this is entirely optional as they are completely standalone and realistically could be defined without using any of Admin Menu's hooks.

I hope I'm shining some light on my reasoning and not just confusing the matter more.

Updated patch with the following changes:
- Keyboard navigation standalone tweak has been removed.
- Visibility tweaks no longer initiated via eval() call.
- Visibility tweaks now have to be defined as 'Drupal.adminMenu_visibility.tweak.name'.
- Improved documentation

There is still an eval() call in the 'Key combo' visibility tweak, but that is a necessary evil so that users can define a custom combo.
The JSON callback is unchanged as I don't see a cookie achieving the same end and can not see an other viable alternatives.

Look forward to your feedback.

.tweak seems like a rather opaque namespace... Given adminMenu_visibility, and a unique name appended to this, is "tweak" actually necessary?

Updated patch, fixing a bug from bad testing practices and improving the namespace.

@dbabbage
I had just written a fix for a bug related to the namespace and was in the process of submitting it.
While writing the comment I was also in the process of answer your question which an admission of my dislike for it myself and a justification of it's necessity when it twigged that not only was it not necessary, it was also introducing more complexity and issues than I had thought it was solving, so thank you :)

Instead of .inline-element, you probably want to use Drupal core's default class .container-inline.

Now I'm a bit stuck - how many visibility tweaks are there? Am I blind or is there just one?
Depending on the answer, we probably want to shift the "sub-behaviors" in Drupal.adminMenu into Drupal.adminMenu.behaviors and thereby make room for a new namespace in Drupal.adminMenu.visibility. In general however, having multiple handlers for controlling the visibility of admin menu feels a bit overengineered.

Drupal.adminMenu.visbility_settings (note, also a typo in here) should live in a completely separate JavaScript file, as this only needs to be loaded on admin menu's settings page.

Sorry for those partial reviews. I still do not have a clear idea of the intention and purpose of this patch. Based on the recent patches, I think this needs to be simplified. Also, I'm considering to create and commit the changes in #10 in a separate issue (but we should know whether those behaviors should live in Drupal.adminMenu or Drupal.adminMenu.behaviors upfront).

@sun

Will change the class, only reason it was .inline-element is that's what it is in AMD as was defined by the previous maintainer.

There are two visibility tweaks currently defined: Key combo and Icon hotspot.
If you look at the Administration menu settings page you can choose between them from the Visibility tweak dropdown. If you haven't got that far, you should be able to see evidence of them in the admin_menu_admin_menu_tweak() (implementation of hook_admin_menu_tweak) or in visibility.js as Drupal.adminMenu_visibility.key_comobo and Drupal.adminMenu_visibility.icon_hotspot.

As for changing the namespace, that's probably not a bad idea, I specifically had to go outside of the namespace so that the visibility tweaks wouldn't automatically get run.

No issue with your reviews, it's your module and I can understand the need to control what goes in. I just want to get the great functionality of AMD in so that people, myself included, no longer need to install both modules to get the combined functionality.
The only way I can think of simplifying things would be to remove the modular nature, but I would be reluctant to do so. While it may be unlikely that anyone will want to add another visibility tweak, or a standalone tweak, Drupal is all about providing that ability.

Hi Deciphered,

I've fixed issues you noticed.
We have got new key/combo this is: SHIFT+SPACE (The same combo as browser's scroll to top)

There are some very minor changes in the admin_menu.inc, needed to get right keyboard order during the keyboard navigation.

This patch goes on top of the current admin_menu dev release: 6.x-1.x-dev and is located here: http://drupal.org/node/331982#comment-1183188

Best,
Stanislav

Moved my patch from #10 into a separate issue for now: #349505: Performance: Cache menu tree, since we need this registry for admin menu behaviors for other stuff now.

If anything is wrong with that approach or registry structure, which might be incompatible with further features of Administration menu dropdown, please follow-up (quickly) over there.

Oops, wrong issue. Sorry, #373372: Turn procedural JavaScript into Administration menu behaviors it is.

Let's try this again:
Patch has been re-written from scratch, and functionality has been stripped down as much as I will willingly sacrifice.

What does this patch do?
This patch adds an item to the 'Advanced settings' menu to enable and define a hotkey that can be used to toggle the Administration Menu visibility.

What are the benefits of hiding the menu?
Hiding the menu has many benefits, including:
- Stops menus popping up when the user scrolls to the top of the browser.
- Removes an unnecessary element from the screen when showcasing your design.

What was not included from Admin Menu Dropdown?
As only the bare minimum functionality was included, quite a lot of features where stripped, including:
- Maintaining the previous visibility state when browsing through your site.
- The ability to disable/toggle the fixed position so users can scroll down for longer menus.
- Alternate and extensible visibility behaviors.

If I have not provided enough information, please let me know ASAP so that I we can finalize this issue, it really is a pain for myself, and presumably others, to have to install a second module for such a simple extra feature.

Cheers,
Deciphered.

What was not included from Admin Menu Dropdown?
As only the bare minimum functionality was included, quite a lot of features where stripped, including:
- Maintaining the previous visibility state when browsing through your site.

Removing this is a huge regression IMHO. It should certainly be kept in any merge.

@dbabbage

I also agree, out of the three items mentioned I believe it is the most important to be in there. But my first priority is to get anything in there, then once the core functionality is in there it can be expanded upon with further patches.

The simpler the patch, the more likely it will actually get committed.

Cheers,
Deciphered.

The simpler the patch, the more likely it will actually get committed.

Uhm. No. The more rock-solid, future-proof, fleshed-out, and awesome the patch, the more likely it will get committed.

+  $form['tweaks']['admin_menu_tweak_visibility'] = array(
+    '#type' => 'checkbox',
+    '#title' => t('Toggle menu visibility with hotkey:'),
+    '#default_value' => variable_get('admin_menu_tweak_visibility', 0),
+    '#prefix' => '<div class="container-inline form-item">',
+  );
+  $form['tweaks']['admin_menu_tweak_visibility_modifier'] = array(
+    '#type' => 'select',
+    '#options' => array(
+      'ctrl + alt' => 'ctrl + alt',
+      'ctrl + shift' => 'ctrl + shift',
+      'ctrl + alt + shift' => 'ctrl + alt + shift'
+    ),
+    '#default_value' => variable_get('admin_menu_tweak_visibility_modifier', 'ctrl + alt'),
+  );
+  $form['tweaks']['admin_menu_tweak_visibility_key'] = array(
+    '#type' => 'select',
+    '#options' => _admin_menu_theme_settings_keys(),
+    '#default_value' => variable_get('admin_menu_tweak_visibility_key', ''),
+    '#description' => '<br />' . t('If enabled, the administration menu visibility can be toggled on and off using the defined hotkey.'),
+    '#prefix' => ' + ',
+    '#suffix' => '</div>',
+  );

We want to use #prefix and #suffix on a new $form['tweaks']['toggle'] form array level. We don't need a separate checkbox, when the second select list would contain a <None> value (the default). The second select should probably use #field_prefix instead of #prefix. Please avoid manual styling (<br />) at all costs (i.e. just remove it here). The option labels of the first select should be wrapped by t() - I'm not entirely sure whether those keys are called the same world-wide.

 /**
+ * Return list of available options for visibility hotkey.
+ */
+function _admin_menu_theme_settings_keys() {
+  $keys = array('' => '');
+
+  $i = 65;
+  while ($i <= 90) {
+    $keys[$i] = chr($i);
+    $i++;
+  }
+
+  return $keys;
+}

I do not understand why we need a separate function here? Try this:

'#options' => array_merge(array('' => t('<None>')), drupal_map_assoc(range(65, 90), 'chr')),

Additionally, I would like to know what the range of characters 65-90 actually translates into (in a comment).

+Drupal.admin.behaviors.visibility = function (context, $adminMenu) {
+  if (Drupal.settings.admin_menu.tweak_visibility) {
+    $(document).keydown(function(e) {

Should use context instead of document here.

+      // Workaround for altKey bug in jQuery >= 1.2.5
+      evt = evt.originalEvent ? evt.originalEvent : evt;

I'm perfectly ok with adding a dependency on jquery_update for 5.x-3.x. Doesn't matter at all for 6.x+, since it ships with 1.2.6 already.

+      hotkey = eval(Drupal.settings.admin_menu.tweak_visibility_hotkey.replace(/\+/g, '&&').replace(/([a-z]+)/g, "evt.$1Key").replace(/([0-9]+)/g, "evt.keyCode == $1"));

Eval is evil. I don't understand why we can't pass the right settings in the first place, so this code can be simplified?

+    $settings['tweak_visibility'] = $setting;
+    $settings['tweak_visibility_hotkey'] = variable_get('admin_menu_tweak_visibility_modifier', 'ctrl + alt') .
+      (variable_get('admin_menu_tweak_visibility_key', '') ? " + ". variable_get('admin_menu_tweak_visibility_key', '') : '');

This should simply end up in Drupal.settings.admin_menu.hotkey. Only 1 JS setting is required to determine whether we want hotkey support.

We don't need a separate checkbox, when the second select list would contain a value (the default).

The <None> value implies that you don't want an extra key in the hotkey, ie. you just want Ctrl+Alt as the hotkey oppose to Ctrl+Alt+A.

I'm perfectly ok with adding a dependency on jquery_update for 5.x-3.x. Doesn't matter at all for 6.x+, since it ships with 1.2.6 already.

The comment in the code says >= 1.2.5, as in higher or equal than 1.2.5. The issue was introduced in 1.2.5 and last time I checked it was still in 1.2.6, therefore it would be 6.x-3.x that required jQuery Update not 5.x-3.x-dev.
I will re-confirm that this issue is still present, but I am fairly confident it is still present.

Eval is evil. I don't understand why we can't pass the right settings in the first place, so this code can be simplified?

It's not the settings that is the issue, it is processing the settings as a javascript function instead of a variable. I will look into this to confirm if there is another way, but I can't think of one of the top of my head.

Everything else you say is valid and I will have a new patch up as soon as possible.

As for my comment on the simplicity of the patch, I believe it is still valid, if I overloaded the patch with all the current functionality of Administration Menu Dropdown, it would take longer to review and it would likely have more issues.
The simpler the patch, the less there is to review, the more likely it will become "rock-solid, future-proof, fleshed-out, and awesome".

Cheers,
Deciphered.

The second select should probably use #field_prefix instead of #prefix.

#field_prefix is, according to the API, only available for textfields:
http://api.drupal.org/api/file/developer/topics/forms_api_reference.html...

In correlation with #331982: Add support for keyboard navigation, I also wonder why we don't rely on jQuery to do the ground-work? See http://jshotkeys.googlepages.com/test-static-01.html, resp. jQuery plugin project page http://plugins.jquery.com/project/hotkeys

oh my. Sorry about #field_prefix. Sounds like another nice core patch then ;)

Please avoid manual styling (<br />) at all costs (i.e. just remove it here)

The <br /> was used due to the inline-container wrapper around the hotkey configuration, without it the description starts on the same line as the select fields. I'm open to suggestions, I would be happy to put it into $form['tweaks']['toggle'] other than the fact that as a markup form type it doesn't support the #description attribute and would require another div wrapper to style it correctly.

As for for #35, depending on a jQuery plug-in would appear to over complicate things as we can't package 3rd party code.
IMHO requiring people to download 3rd party code to get access to all functionality limits your user base.

#36 sounds good to me.

New patch, made as many of the changes requested by Sun in #32 as viable.

Changes not made:
- Checkbox left in as per #33.
- #prefix not changed to #field_prefix as per #34.
- jQuery workaround left in as per #33 - more information available at: http://dev.jquery.com/ticket/2947
- Eval left in as per #33 - will change if a viable solution is determined.
- <br /> left in as per #37.

Cheers,
Deciphered.

Thanks!

+    '#prefix' => '<div class="container-inline form-item">',

I don't think that " form-item" is necessary in here.

+      'ctrl + alt + shift' => t('ctrl + alt + shift')

Missing trailing comma.

+    // Note: 'range(65, 90)' is used to generate characters 'A' through 'Z'.

Please cut this down to: // 65-90 maps to A-Z (also without "Note:" - comments always note something or should not be there at all ;).

+      evt = (e) ? e : window.event;
+      hotkey = eval(Drupal.settings.admin_menu.hotkey.replace(/\+/g, '&&').replace(/([a-z]+)/g, "evt.$1Key").replace(/([0-9]+)/g, "evt.keyCode == $1"));

Two variables defined in the global scope - please always use var.

+      // Workaround for altKey bug in jQuery >= 1.2.5

If this is a known bug and we just wait for a fix, then why not directly this?

+      // Workaround for altKey bug in jQuery >= 1.2.5 < 1.3.2
+      // @see http://dev.jquery.com/ticket/2947

.

+    $settings['hotkey'] = variable_get('admin_menu_tweak_visibility_modifier', 'ctrl + alt') .
+      (variable_get('admin_menu_tweak_visibility_key', '') ? " + ". variable_get('admin_menu_tweak_visibility_key', '') : '');

If admin_menu_tweak_visibility_key is optional, then we can do this a bit cleaner:

+    if ($hotkey = variable_get('admin_menu_tweak_visibility_key', FALSE)) {
+      $settings['hotkey'] = variable_get('admin_menu_tweak_visibility_modifier', 'ctrl + alt') . ' + ' . $hotkey;
+    }

...and additionally make sure the 'hotkey' property is properly initialized in Drupal.behaviors.adminMenu().

I don't think that " form-item" is necessary in here.

I did this because I noticed that container-inline removed the css margin, while it's not an issue with no other items below it at the moment, if there was later there would be no spacing between them.

(also without "Note:" - comments always note something or should not be there at all ;).

Based this on the first comment I found above that area of code. Normally I wouldn't use 'Note:', but was trying to keep things consistent.
Will change.

If admin_menu_tweak_visibility_key is optional, then we can do this a bit cleaner:

$settings['hotkey'] still needs to be defined if there is no 'key', only a modifier, so it would actually turn:

+    $settings['hotkey'] = variable_get('admin_menu_tweak_visibility_modifier', 'ctrl + alt') .
+      (variable_get('admin_menu_tweak_visibility_key', '') ? " + ". variable_get('admin_menu_tweak_visibility_key', '') : '');

into:

+    $settings['hotkey'] = variable_get('admin_menu_tweak_visibility_modifier', 'ctrl + alt');
+    if ($hotkey = variable_get('admin_menu_tweak_visibility_key', FALSE)) {
+      $settings['hotkey'] .= ' + ' . $hotkey;
+    }

While it's cleaner, it's also two lines longer.
If you think it's necessary I will certainly change it.

Making other changes now.

Cheers,
Deciphered.

New patch.

Only change I did not make, but will pending confirmation, is the 'form-item' class issue.

And again:

This patch should make you happy Sun, no more evil eval()!!!
Read through the source code of the jQuery plug-in you provided earlier, and while it didn't directly solve the issue, it helped me re-arrange my thinking and work backwards to get the perfect solution.

Enjoy :)

Edit: There is a bug in this code :(
Working on a new patch now.

New Patch, bug fix for #42.

Looks like my brain is due for a reboot, real patch attached this time...

+      'ctrl+alt' => t('Ctrl + Alt'),
...
+    '#default_value' => variable_get('admin_menu_tweak_visibility_modifier', 'ctrl + alt'),

With or without spaces?

Hi Sun,

That's what happens when you change something at the last minute. My bad.
Fixed it, updated the javascript a bit and added uninstall code.

Cheers,
Deciphered.

Re-rolled patch for latest updates to DRUPAL-6--3.

Edit: I don't believe the patch actually needed to be re-rolled as it appears it would have still applied cleanly, just an act of habit.

Added patches for HEAD and DRUPAL-5--3.
Patch from #47 is still relevant for DRUPAL-6--3.

Time for an updated patch, only for DRUPAL-6--3 at this stage.

Changes in this patch:
- Added position toggle hotkey - Users can toggle the 'fixed' positioning so they are able to view menu items that are below the screens threshold.
- Added persistent states via cookies - Position and Visibility states are carried across when browsing through your Drupal site.

Looking forward to feedback.

Cheers,
Deciphered.

Due to the drastic changes in Admin Menu 6.x-3.x-dev, a new patch will be available as soon as I have time.

Cheers,
Deciphered.

Subscribe.

sun: any chance of this for the 6.x-3.0 release? (assuming #550254: Menu links are sometimes not properly re-parented finally gets committed)

is it going to be merged into administration menu ?

No.

i bumped into this and similar threads repeatedly, with no working patch for the present dev. version of admin_module to be found. since i'm working on a site with a bunch of sub-themes that differ slightly from each other i got desperate for the ability to hide the admin_menu.

took one of the previous patches from this thread and tried to apply it manually to admin_menu 6.x-3.x-dev. after a while i figured out that things are differently named now; mainly $($adminMenu), used to toggle the admin menu in the old patch, needs to be changed to ('$div#admin-menu'), and voila, my admin_menu hides again.

THIS IS NOT A PROPER DRUPAL PATCH. i don't have time right now to get into the finer issues involved. i'm posting this here in case somebody else wants to hide their admin_menu badly enough to live with a hack for a while, until this functionality gets incorporated into the admin_menu, or whatever.

the attached patch works for me, using admin_menu 6.x-3.x-dev, ver. 1.5.2.1.2.2, date 2009/07/29 15:50:51.

it there's a problem, it should be easy enough to apply the patch manually if you really want the functionality. (or perhaps a better drupal warior than i can make a real patch from this?)

-----------------------
replacing a couple edits where i claimed this patch didn't work: it actually does. what i did was extract an unpatched version of the module under /sites/all/modules somewhere to create the patch, and it seems different browsers picked different versions of the same module...whatever. i'm too tired now; need some rest.

The current 6.x-3.x-dev integrates the settings page with Admin Menu's; but it could still be added as a sub module or something similar. Not many people use AMH, but this is probably because no one knows about it. I think that putting AMH into Admin Menu would be awesome, but I don't know how to do patches. From what I can tell from the patch above, it just adds code that AMH has in hook_form_FORM_ID_alter. A link to AMD from the Admin Menu module page would probably do the trick, and be a lot easier for everyone.
BTW, I am a new maintainer for Admin Menu Hider.
(Attached image shows the fieldset.)

I am sorry for no reply until now.

There are many issues regarding this module admin_menu which is a bit difficult for us to follow up since some of the issues might be already outdated, or is already fixed by the module or any other modules or itself core which means that the problem might no longer need to be fixed.

We can see that the issue has been created for a few years ago, I hope it is okay for you that I am postponing the issue, and give you around two weeks. If you still face the problem, could you tell us the step by step when until you get the error message or what is frustrated you, and a list of modules you are using related to admin_menu and a screenshot that might help us? So it makes us easier to reproduce your issue.

However, after two weeks with no feedback - we will close this issue. So in case, you noticed it after the issue is closed, do not hesitate to reopen it like and fill information which is mentioned above.

So before giving us a feedback, do you mind to test it again with our latest 7.x-3.x-dev?

Thank you for understanding! :)

This issue has been automatically marked as closed because it has not had recent activity after the last post.

However, if you or someone is still facing the same issue as described to the issue, could you please to re-open the issue by changing the status of the issue, and add an explanation with more details which can help us to reproduce your situation.

Again, thank you for your contributions! :)