Somewhat amusing story - I just spent about an hour starting to write a module that would provide a nice listing of image galleries, with thumbnails and links for each gallery. Turns out, this feature already exists at path image.

I was not aware of this, as path image is a MENU_SUGGESTED_ITEM. Would it be possible to make this MENU_NORMAL_ITEM, or in some other way make the user aware of this page?

From my own experience, I can say that it is easy for users to become confused when this page is not highlighted. After installing Image Galleries, I had no idea how to use it. It was only by looking through my taxonomy that I realized the galleries existed at image/tid/#. And even then, I didn't know that a nice listing was provided at image.

Comments

joachim’s picture

Hmm, I'm glad you see the funny side, and only spent an hour!

You can get to path image from the breadcrumb when you're in a gallery, so I guess the path of discovery is image node -> gallery via taxo link -> main gallery via breadcrumb.

But I've often wondered by we make the menu MENU_SUGGESTED_ITEM -- some modules put things in the navigation menu (eg blog) some only suggest (forum, I think). Given image module was originally based on forum for a lot of its code, I guess that's where it came from.

So we could make it MENU_NORMAL_ITEM. But when we switch to galleries with views, I'd be against putting in a menu item (as menu items for a view often go a bit doolally), so a short-term change one way seems daft.

We could add a note to the online help and documentation pages?

Anonymous’s picture

Glad to see that you are receptive to community concerns. I don't think you would necessarily need to use MENU_NORMAL_ITEM. Any way of making the user better aware of the gallery page would suffice, I think. As I said, I can speak from experience that it's not perfectly clear to the user where image galleries reside.

There are, I'm sure, many different solutions to this. Just off the top of my head: documentation (as you mentioned), a link to the gallery page from admin/settings/image/image_gallery, a post-install drupal_set_message() indicating that the menu item is suggested, etc.

geerlingguy’s picture

The drupal_set_message() function is one of my favorite methods - every time I enable a module, I check all the messages to see if there are any important new pages to visit or permissions that need to be set.

dman’s picture

This is a general meta-issue, but all good suggestions.
I wrote up into the Module documentation guidelines how I really think that modules should be a lot noisier about the functions they provide :-)
Yes, a drupal_set_message() in the HOOK_install is still my favorite way of saying hello.

Module X is now installed. The settings can be found *here*, and the help page is *here*. You will see a new item on the content types edit page *over here* and there is a suggested menu item you can enable *over here* and a new block available *here*. Remeber to enable permissions *over here* if you wish to grant access to this feature!

I do this whenever I can, and occasionally contribute a line like that to modules I mess with.

joachim’s picture

Version: 6.x-1.0-alpha5 » 6.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new3.05 KB

Giving netbeans another try... if only it knew to ignore its own .cvsignore *sigh*

dman’s picture

Hooray for contributions of documentation!
Oh, um, for mysterious reasons, it's best to use st() not t() in install messages. Something to do with being able to bootstrap a module from an install profile before the translation table is available. I think.

joachim’s picture

Status: Needs review » Fixed

Thanks for the pointer! Hurrah for patch reviews :D
Fixed a spacing around . operator too.

#539388 by joachim: Fixed lack of documentation in Image gallery module.

sun’s picture

Status: Fixed » Needs work
+++ contributions/modules/image/contrib/image_gallery/image_gallery.install Locally Modified (Based On 1.10)
@@ -2,7 +2,8 @@
+  drupal_set_message(t('Image gallery has been installed. You may want to enable the <a href="@navigation-menu-url">Image galleries menu item</a>.', array('@navigation-menu-url' => url('admin/build/menu-customize/navigation'))));

As dman mentioned, t() should not be used directly in .install files.

$t = get_t();
...
$t('...');

@see http://api.drupal.org/api/function/get_t/6

I'm on crack. Are you, too?

joachim’s picture

Status: Needs work » Fixed
StatusFileSize
new1.03 KB

Committing the attached patch.

Anonymous’s picture

Oh, um, for mysterious reasons, it's best to use st() not t() in install messages. Something to do with being able to bootstrap a module from an install profile before the translation table is available. I think.

That may be true. I know that at the very least, st() must be used in actual .profile files for that reason. It makes sense that it should be used in .install files as well, but I'm not certain.

Status: Fixed » Closed (fixed)

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