Attached is a patch that fixes strangeness in the way that gallery descriptions get displayed.

Currently the only time a gallery description is displayed is on a gallery listing page. In other words, the description of a gallery never shows up on an individual gallery page.

This patch will show the description of a gallery on the gallery listing page, as well as on an individual gallery page (actually making the descriptions useful).

andre

CommentFileSizeAuthor
#13 image_gallery_172620.patch2.78 KBdrewish
#10 image_gallery.module_3.patch2.68 KBandremolnar
#8 image_gallery.module_2.patch2.66 KBandremolnar
#5 image_gallery.module_1_0.patch2.51 KBandremolnar
image_gallery.module_1.patch1.78 KBandremolnar

Comments

Hetta’s picture

Hetta commented

I think this should be optional ... I like the current clean look'n'feel of galleries.

andremolnar’s picture

andremolnar commented

It would be 'optional':
To not display it you could
a) not providing a description of the gallery (none would show up)
b) use theming and a template to decide what shows and what doesn't.

Without this however, there simply isn't an option to have it show up even if you wanted it to.

If however there people want an administrative option (turn it on or off) then I suppose I could roll that patch too. What say yee all?

andre

vm’s picture

vm commented

I think it would add a nice feature to the module for those who wish to have gallery descriptions available to them. With the fact that you are also adding an admin on/off switch, it seems to me to cover boths types of users. Those who want and those who don't.

drewish’s picture

drewish commented
Status: Needs review » Needs work

the current behavior is by design so any changes should preserve it. make it an optional setting and i'll look at it.

andremolnar’s picture

andremolnar commented
StatusFileSize
new image_gallery.module_1_0.patch2.51 KB

Done and done - now an administrative option.

andremolnar’s picture

andremolnar commented
Status: Needs work » Needs review

just setting status
andre

drewish’s picture

drewish commented
Status: Needs review » Needs work

Seems pretty simple, few small things though.

I wouldn't bother changing this since they're equivalent, and IMHO, the former is clearer:
- if (count($galleries)) {
+ if (count($galleries) > 1) {

The comments should be on the line above the code they're referring to and written as proper sentences with correct capitalization and punctuation. I know that there's plenty of of places in the module that this isn't followed but I'm working on improving it and would rather not take any steps backwards.

andremolnar’s picture

andremolnar commented
StatusFileSize
new image_gallery.module_2.patch2.66 KB

Yes this is super simple. Tidied up the comments.

Re: count
I am checking to see that there is actually more than 1, because the case I am interested in is where there is ONLY 1. count($foo) > 1 is not the same as count($foo)

$foo = array('one item');

print count($foo); // displays 1

$foo = array('first item', 'second item');

print count($foo); // displays 2
drewish’s picture

drewish commented

you're correct count() > 1 isn't equivalent to count()... but in your last patch you're treating count() == 0 and count() == 1 the same way...

andremolnar’s picture

andremolnar commented
Status: Needs work » Needs review
StatusFileSize
new image_gallery.module_3.patch2.68 KB

So true - we still need a non zero count. Fixed.

andre

drewish’s picture

drewish commented

marked http://drupal.org/node/173594 as a duplicate

Hetta’s picture

Hetta commented
Status: Needs review » Reviewed & tested by the community

Works a treat on 5.x-2.x-dev.

drewish’s picture

drewish commented
Version: 5.x-1.5 » 6.x-1.x-dev
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new image_gallery_172620.patch2.78 KB

Probably should do this for 6.x-1.x and 5.x-2.x... Here's a re-roll for 6.x. I'd like to get joachim's feed back on this though.

andremolnar’s picture

andremolnar commented

Is there any fundamental opposition to this functionality?

Should we not close out the 5.x issue first and/or open 6.x as a new issue? I would hate to see 5.x solution lost in the shuffle.

Hetta’s picture

Hetta commented

Marked http://drupal.org/node/220895 as duplicate.

Hetta’s picture

Hetta commented
Status: Needs review » Reviewed & tested by the community

The patch in #13 doesn't break things, but it won't show gallery descriptions at the top of a gallery page.

joachim’s picture

joachim commented

*notices his name belatedly*

Would it be possible to put this into the views support, as the page view header?

Hetta’s picture

Hetta commented

OK, both #13 patch and #10 patch do what they should:
1) add an option to admin/settings/image_gallery
2) if said option is clicked, display image gallery descriptions.

(my bad: I didn't click on the [x] in #16)

andremolnar’s picture

andremolnar commented

Just stopping by to check on the status of this - its been RTBC for 5 months.

Can someone commit this please. If not for the community then do it for me so I can close a ticket at work. :)

andre

drewish’s picture

drewish commented
Status: Reviewed & tested by the community » Needs work

sorry for the delay... just tested this out and:

if (count($galleries) && variable_get('image_gallery_display_description',0) == 1) {
   $content .= '<div class="description">'. check_markup($galleries->description) ."</div>\n";
}

seems problematic. because $galleries is an array but we're using the -> object notation.

Hetta’s picture

Hetta commented

marked http://drupal.org/node/272725 as duplicate.

joachim’s picture

joachim commented
Status: Needs work » Closed (duplicate)

I want to move galleries over to being made with views. This will give much more control over how they are generated and themed.
See http://drupal.org/node/405456 for a patch. Help testing this would be welcome if you are able.