Administration Menu Elements

Welcome,

I just had a glance at the code and tested the functionality and the module seems to be working fine. Would get back to you with a detailed review of the code soon. For the time being some minor changes that have to be made.

Please create a detailed project page for the module. Please check here to create a good project/module description page.

There is a minor change in implementation of hook_menu (Line #21). All the admintrative functions should go into a file which is named as .admin.inc according to convention, so the code from line#29 to line #37 would change to

$items['admin/config/administration/admin_menu/elements'] = array(
  'title' => 'Elements',
  'description' => 'Choose the elements that you want to have in Administration Menu:',
  'page callback' => 'drupal_get_form',
  'page arguments' => array('admin_menu_elements_settings'),
  'access arguments' => array('administer site configuration'),
  'type' => MENU_LOCAL_TASK,
  'file' => 'admin_menu_elements.admin.inc',
);

So the function admin_menu_elements_settings would be placed in admin_menu_elements.admin.inc file instead of admin_menu_elements.inc file.

I get your project promoted as full project fast, it would be great in case you could get review bonus so that reviewers can come back to your application sooner.

This module works as expected... although I'm not really sure why it's useful. Technically the code is straight forward, uses Drupal standards, and passes strict Coder Review.

The only change that makes me think this is still "needs review" is a simple text change on the admin form description for "home icon": It mentions that this is usually a "house icon" but the Administration Menu renders the home icon as the site favicon. The house icon you describe appears when you are using from Drupal's core Toolbar module, so that is a bit misleading.

I'd also suggest providing more details in the project page of why anyone would ever want to hide any of the things they can hide in the admin menu... but that's a minor suggestion.

Hi,

I get this error, when I configure the module
Fatal error: require_once(): Failed opening required '/var/www/d7/sites/all/modules/custom/administration_menu_elements/admin_menu_elements.inc' (include_path='.:/usr/share/php:/usr/share/pear') in /var/www/d7/includes/menu.inc on line 514

This looks good to me! The new functionality you've added (left/right, sorting) definitely makes this Module make sense. Passes CodeSniffer, but there's one minor error when using Coder on minor setting:

Line 194: missing space after comma
    'header' => array(t('Elements'),t('Weight'),t('Position')),

I did not experience any of the errors reported in #5, which sounds like an issue with the user's local setup.

This looks like a feature that could go directly into admin_menu. Module duplication and fragmentation is a huge problem on drupal.org. We prefer collaboration over competition, so please open an issue in the admin_menu issue queue to discuss this and what you would have to add. You should also get in contact with the maintainer(s) to work together or to determine that this should be in fact a separate project.

After that If this should still be a separate project please set this issue back to "needs review".

Thanks for mentioning this issue, @klausi!

I agree that it is a bit confusing why this was started as a separate module -- there's even an issue for that in admin_menu's queue:
#293768: Allow to enable/disable menu additions

What's the benefit of having this in a separate module? (vs. having the functionality built-in?)

AFAIK, various users repetitively asked for being able to disable/remove some of the widgets in the menu in the past already, so there is definitely room for including this in admin_menu itself. I won't have time to work on this myself though.

This is definitely not meant as a "fight" - a separate module would work, too, of course. It's merely that, if you look at this code, then you see that it mainly consists of drupal_alter()s that are injecting or rather undoing the default behavior of a module. By implementing the functionality directly into the actual code that generates the output, it not only gets a lot cleaner, but it will also result in a possibly significant performance impact, depending on the widgets being disabled (since they are not undone after the fact, but instead, not built in the first place). In the end, that's a typical thing that should be controlled by configuration. :)

I guess that's all I can provide from the admin_menu perspective. The issue #293768: Allow to enable/disable menu additions already exists. Some implementation details would have to be figured out, but I'm happy to discuss and provide pointers. I think I like the tabledrag UI you've built there. Though one thing I wonder about would be whether the visibility of elements shouldn't be controllable per user role; e.g., so that you're able to expose the search widget to novice site admins/moderators, show/hide the shortcut bar for certain roles, etc. But yeah, could also be a later step. Overall, I like the idea a lot.

Yeah. Drupal's success is primarily based on the immense amount of collaboration between contributors. There's really no reason to be scared by that, but I will certainly agree that it is a slightly different environment to operate in, when comparing it to the "scratch your own itch" philosophy. When you're developing in own project or sandbox, then you do not necessarily have to care for anyone else's feedback and you can just do whatever you want to do. At the same time, working in a corner on your own also means that the overall resulting quality of the product will inherently not be as good as it could be, since no or almost no peer-reviews have been involved. Both sides have their strengths and weaknesses. However, from a higher-level perspective, it is guaranteed that working and collaborating with peers will make you a much better developer in the long run. In fact, that's a process which never ends. The more you work with peers, the larger become the projects that you attack. And the larger and the more complex the projects you attack are getting, the more intensive you need to work with peers. :) I will definitely agree and say that it takes time to get used to it, but at least from my perspective, I can guarantee you that it's worth it. In the end, it's that spirit which makes the Drupal community so awesome :)

When attacking to include the functionality directly within admin_menu, then I don't think we'd want to do that as a sub-module. Instead, the functionality would be integrated in the main admin_menu module. The conditions that decide whether to output something or not would be baked directly into the generating functions.

Oh. As mentioned earlier, I won't have time to work on this myself.

The way to approach this would be to extract the individual parts from admin_menu_elements module and (truly) integrate them into the corresponding code of admin_menu. This would be done as a patch.

AFAICS,

- the code in admin_menu_elements.admin.inc would be moved into admin_menu.inc.

- the main code in admin_menu_elements_admin_menu_output_alter() would be redone and applied to the respective output generating functions for the individual widgets throughout admin_menu instead. (This part might require some more thoughts, since admin_menu is actually prepared to allow for additional widgets/plugins being added by other modules, but doesn't expose a formal "registry" yet. Without a registry, we'd only ever know about admin_menu's own/built-in widgets and could only adjust those - so if another module adds something else, then that would probably appear at a seemingly random position... Adding a widget/plugin registry doesn't sound too hard to do though.)

- the OG group switcher widget would be contributed to OG instead. (admin_menu only contains Devel support built-in)

Does that help? :)

The way it usually works is to submit patches against a module, which will be committed by the project's maintainers (there may be multiple). It is possible to be even granted write access to a project, but that's a decision of the project maintainers, and most often depends on the overall quality of the patches as well as the collaboration and communication that happened between you and them — that is, because project maintainers most often need and want a certain level of trust, so as to be sure that a new co-maintainer also fulfills other project maintainer duties, such as double-checking that a change does not break the module functionality for existing users after updating to a new version, verifying that proposed changes are in line with the overall vision/direction of the project, and so on. A very sparse and ineloquent description for this can be found on Best practices for co-maintaining projects handbook page (which, alas, is maintained by me ;)). The mileage of individual project maintainers may vary, but in general, most existing projects on drupal.org will happily welcome new co-maintainers (to distribute the workload and improve the project's quality). :)

So yeah, for now, you'd submit patches to #293768: Allow to enable/disable menu additions. And if it turns out that we're collaborating like superstars and are getting awesome things done, then there'll certainly be room for getting elevated to a co-maintainer. ;)

Lastly, please note however, that the final decision here is certainly not mine (since I obviously have a conflict of interest). If others think that this can be a separate project on d.o, then I'll happily concede. ;)

Oh, the line you quoted was about this very project application here. ;)

Regarding #293768: Allow to enable/disable menu additions, the decision is essentially made already (otherwise, that issue would be closed as "won't fix" already) ;) That said, let me cross-reference this issue over there, so others might be able to help with merging/incorporating the code.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.