I'd think that the 'class' of the image should be 'captioned' not 'caption'.
That then describes a property of the image, and no longer interferes with actual css that might describe the 'caption' itself.
Even in your code I see you've had to fudge it with

div.image-caption-container
- img.caption
- div.image-caption

I'd think

div.image-caption-container
- img.captioned
- div.caption

may be tidier.
TRULY minor suggestion, but my image is not itself a "caption". And I have a .caption style already :-)

Comments

davidwhthomas’s picture

davidwhthomas commented

You can edit the image_caption.js file to change the classname.

change the jquery selector from:

$("img.caption")

to 

$("img.captioned")

as a workaround.

I thought about using a config variable for the classname and tried that but had some issues with adding the inline js.
so i used the external js file instead.

dman’s picture

dman commented

Oh, I found how to switch it alright.
This was just a slight semantic suggestion to make the label appropriate to what it was labelled. Certainly doesn't need to be configured, but could be made grammatically correct internally.

My image is not itself a caption.
The text below it is a caption.

I guess I was feeling pedantic that night :-) ... and sick of juggling badly-named wrapper divs.

davidwhthomas’s picture

davidwhthomas commented

'caption' is an imperative form of the verb 'to caption'
the way I saw it was that it would be an instruction to 'caption me' for jquery, but I'd rather not argue over semantics.
Once I figure out CVS, I may submit a release with the classname as a config setting to appease everyones tastes.

dman’s picture

dman commented
Status: Active » Closed (won't fix)

True enough. I guess I'm used to looking at classes as descriptive adjectives, not instructional imperatives. Basically a stylistic choice.
I didn't mean to argue, it was just top-of-the-head user feedback on what looked like a newish module.
I do think that bothering with a setting for it would be overkill, so forget it :)

fletchgqc’s picture

fletchgqc commented

I agree with dman. Post #3 also makes sense, but owing to the fact that caption is also used in other contexts, I think "captioned" should be used. As pointed out, it would have to be a config option though because otherwise you are going to break all the sites that upgrade!

I'm going to use "captioned" and hack the code...

fletchgqc’s picture

fletchgqc commented

Wait I had a better idea. The class should be "auto-caption". Then everyone is happy and it makes sense.