CVS edit link for jdschroeder

I have created a module for use on my own projects that I believe would be useful to others in the community, so I would like to share it as part of the contrib repository.

My module creates a theme-able, unobtrusive, collapsible floating menu for content editors and managers. There are several excellent modules providing administrator menus (most notably admin_menu) , but we have had trouble adapting those for use by non-administrative users on large scale sites.

This module features:
- the ability to select any existing menu from within your Drupal installation
- simple CSS themability, with three themes included
- jQuery UI draggable floating "window" that doesn't require inclusion in a certain region of a theme (window is collapsible to save space and maintains persistent positioning through page loads)
- seamlessly compatible with dhtml_menu for an even smoother experience

Beyond just wanting to share this module, I would like to be able to get more eyes on it to suggest improvements and further development potential.

Comments

jdschroeder’s picture

Status: Postponed (maintainer needs more info) » Needs review
StatusFileSize
new14.56 KB
avpaderno’s picture

Issue tags: +Module review
jdschroeder’s picture

StatusFileSize
new9.15 KB

Uploading a minor update.

- removes LICENSE.txt
- adds a z-index property in the themes to prevent the menu from being hidden by other positioned elements

matt2000’s picture

Status: Needs review » Needs work

Have you discussed packaging this with DHTML menu module with it's maintainers?

I think HTML inside of a t() function is discouraged -- see floating_manager_menu_settings() line 181.

Could you use system_settings_form() to save you the trouble of floating_manager_menu_settings_form_submit() ?

floating_manager_menu_theme_list() sounds dangerously prone to namespace collision. naming your menu skinning system 'themes' might not be a good idea in the first place....?

You shouldn't list menu as a dependency, since it is required.

jdschroeder’s picture

Status: Needs work » Needs review
StatusFileSize
new9.01 KB

Have you discussed packaging this with DHTML menu module with it's maintainers?

Personally, I'm not convinced that the modules are really that closely related that they warrant that; however, on your suggestion, I have contacted the developer of that module to seek his opinion.

I think HTML inside of a t() function is discouraged -- see floating_manager_menu_settings() line 181.

Of course. I've fixed this.

Could you use system_settings_form() to save you the trouble of floating_manager_menu_settings_form_submit() ?

I could, and I now have.

floating_manager_menu_theme_list() sounds dangerously prone to namespace collision. naming your menu skinning system 'themes' might not be a good idea in the first place....?

This is something that had crossed my mind, and I have now changed the terminology to skins rather than themes.

You shouldn't list menu as a dependency, since it is required.

I've removed that dependency.

Updated code attached.

matt2000’s picture

Status: Needs review » Reviewed & tested by the community

Cool. I like it.

One trivial note: you still use the term 'theme' in the constant FLOATING_MANAGER_MENU_THEME .

But no reason to delay CVS access on that point.

jdschroeder’s picture

StatusFileSize
new9.06 KB

Thanks Matt. Not sure how I missed that constant, but I've got it fixed now, too.

avpaderno’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)
Issue tags: -Module review

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

avpaderno’s picture

Component: Miscellaneous » new project application
Assigned: Unassigned » avpaderno
Issue summary: View changes
Status: Closed (fixed) » Fixed

Status: Fixed » Closed (fixed)

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