Additional caption and byline fields

This is awesome! I have needed this on multiple sites.

Could you please re-roll a patch against the stable release? I had trouble applying the patch.

I patched the module by hand and the code works as advertised. Thanks again.

If you end up having problems let me know. I have a patch for this I did earlier against the stable release, but I didn't upload it since all new features are going into -dev

+1, this is a very useful addition. Both the byline and the caption are frequently required. I found this patch when I was thinking of writing one to do the same. I'll apply and report back.

I was about to write exactly this before I found this patch, which works perfectly for me. I need it for cases when I have a CCK type with an image field that may be used multiple times; I didn't want to have to create a text field which could also be used multiple times and then try to find a clean way of associating a caption with a given photo. The supplied patch didn't apply cleanly to the latest version of imagefield (5.x-1.x-dev, datestamp 1192795778) so I'm attaching an updated version, but I didn't add any features or find any bugs to fix. In my opinion this is ready to commit. Many thanks, heyrocker.

It is considered impolite to mark one's own patch as "ready to be committed", except if it's a trivial one-liner or that kind of patch. So any patch which might cause some controversy or affects the database scheme (both of which apply to this issue) should stay in "code needs review" until someone else pushes it to "ready to be committed". Also, submitting patches for imagefield 5.x-1.x are unlikely to be applied because feature development only happens on 5.x-2.x which corresponds to the DRUPAL-5--2 tag in CVS. If you'd like to see your patch upstream, it seems like a good idea to make sure it applies to this version.

As for the general approach of the patch, ...I don't know. I have seen a lot of "just one or two more data columns" issues by now (both in imagefield and in filefield), maybe there should be a more general approach like a serialized array that add-on modules can utilize. I doubt that adding fields for each possible use case has a bright future in the upstream version.

mvc did not write this patch. I did.

My original patch was against HEAD, mvc's is against 5.x-1.x. That seems a useful thing to me, given that someone above had already asked for it (although he shouldn't have changed the issue.) I'm resetting the version here.

As for the patch, I agree with your general point about "one more field" however these can be used or not as people wish by checking/unchecking in config, and as so many people have requested these specific features, they seem like good additions to me. As far as its upstream future, I emailed Darren about this approach before submitting the patch, and he said such a thing would be welcomed.

I welcome more review and comment on this patch! I'm glad people are finding it useful.

heyrocker, I didn't realize your patch applied to HEAD or 2.x, I just assumed that as it was first posted in August it was simply out of date. But of course I ought to have checked, so sorry for changing the version.

As for the "just one more field" issue, I would personally love to be able to repeat field groups in general, but of course that would be a lot of work. In the meantime, I've considered writing a CCK field type which implements a groups of text fields, so that it's possible to have a set of three or four fields that can repeat, but haven't so far because I've found other workarounds for my own projects. Until some more general approach is written, I hope this patch will be accepted (but I won't set it to 'ready to commit' again; I'll let the next reviewer do that).

I use this patch on two production sites.

The patch (#5) applies correctly on my setup using imagefield-5.x-1.x-dev.

Thanks, this has solved a big problem for me.

The patch works great for the 'Default' formatter. Thanks for the patch. The only caveat is that if imagecache or lightbox2 or both are used then the data disappears as the extra fields are supplied in the theme function called by hook_formatter and the other modules that offer image formatting to imagefield supply their on hook function. This means the theme function is never called and they have no knowledge of the extra fields.

As pointed out previously there might be a cleaner way to supply extra fields in a consistent manner so that the other formatters can can get the data in the _imagefield_file_load function and theme the 'img' tag only while letting imagefield theme the remaining data items. It seems that maybe the best approach would be to allow some hook overrides in imagefield allowing for the addition of data elements through standalone modules rather than patches. That way, all of the add on modules would be able to work without having a working knowledge of each add-on.

I am working on this approach now but didn't want to write duplicate code if this is currently being worked on.

Where does this end? Don't we need additional fields for all of the EXIF data? And perhaps a few other extra ones, right? My point in asking that question is because I think that if we take this feature to the logical conclusion (which is supporting all EXIF data) then we would need an additional 20 or so fields. I don't think that makes sense for the imagefield module to support those.

However, we have a workaround that works today - using imagefield with cck fields to provide storage for the additional metadata which you wish to store. IMO, that is the best way to solve this problem without adding additional metadata to the imagefield cck definition (and therefore this patch should be "won't fixed").

Also, the most recent patch removes the drupal_add_css which seems like a mistake or at least it's a fix for a problem that's not discussed anywhere in the issue. So, if this goes in that should be added back or explained.

Using additional CCK fields to store metadata works fine as long as you're only doing one image. However, if you're doing multiple images, then the interface for entering data quickly becomes unusable. We actually went this route initially, it didn't work out because we often have 8-12 photos per story and it becomes very difficult to figure out which caption goes with which credit goes with which alt tag etc. This is the whole reason I added this functionality to imagefield itself, especially since it already had two additional fields to start with.

I will agree with jpetso and dmaynor above that there may be a better way to approach this that doesn't involve hacking the module every time someone wants new data.

To propose an alternate workflow:

Create your story - use nodereference on your imagefield-nodes to associate the imagefield-nodes to the stories. Then you can use whatever metadata you need in the imagefield-nodes. This is more or less the approach that has been used successfully on some big media sites and I think it's a reasonable one. If you need the UI to be super-slick, it's possible to have a "create article, jquery lightbox/sidebar for creating associated image nodes, which are automatically associated to the story node.

I think this satisfies your needs, right?

Greggles,

While your approach technically works. From a UI point of view it is torture to create 8 nodes, then another node with 8 node references (assuming you want to build an 8 image node). Big media sites may not have a problem with making their users suffer, but my clients wouldn't stand for it. I've tried the "create article, jquery lightbox/sidebar" solution, but then the UI is different from all other node creation - and you're dealing with multiple additional modules to install and maintain vs. a simple patch to an existing module.

My preferred outcome would be to have the two additional fields - one a text field for title/credit and the other a text area for body/description.

Drupal nodes default with title and body - why can't images? I like elegant code and databases, but taking it to an extreme and not allowing for the fact many images have a credit/title and description/body does not make sense to me. Also, adding two more fields to a database table does not seem expensive to me in a age where 500Gb drives cost US$100.

I think you have a good point with the "EXIF data" use case - and would suggest a supplementary module that adds N additional fields to imagefield.module and stores this data is a separate table.

If you do the nodreference piece well it's no more work than creating the normal node.

I'm arguing against this because I see it as a deviation from the "simple but extensible" mantra that has served Drupal (and cck fields) so well.

FWIW, I also dislike the idea of serialized array field - storing serialized data in DB fields is often convenient but never good for performance or migrations. I wouldn't be opposed to that, though, since it provides people who want to shoot themselves in the foot a great way to do it without deviating from the "simple but extensible" idea.

I disagree that nodereference is no more work. If this was the case - every cck field would be a nodetype and to build more complex nodes you would "just" use nodereference.

If I read the patch correctly - there are no serialized arrays, but two new columns to store the new data. No foot shooting I can see, but I may be mistaken.

I agree that "simple but extensible" is an admirable goal. But, in this case I see multiple users who would benefit from an existing patch. And, none who would be harmed. If there is a demand for a solution that is more inline with "simple but extensible" - I look forward to seeing the patch.

From a UI point of view it is torture to create 8 nodes, then another node with 8 node references (assuming you want to build an 8 image node).

First, that's not what I suggested. I suggested that if you need 1 story node with 8 images and a byline on the images, use 1 story node and 8 image nodes and put textfields on the image nodes for the byline. So, total 9 nodes, not 17.

It's no more torture for the end user if you (the developer) have implemented it correctly. It is undoubtedly more work for the developer. There are many reasons why we don't implement all cck fields as nodereferences to each other, perhaps chief among them is the ease to the developer but certainly also the impact on the database model underneath plays a part. Extending arguments to the extreme case is often useful for finding whether an idea makes sense, but in this case I don't believe it makes sense (especially since your critique was based on a false understanding of my workaround).

When I discussed serialized arrays I was responding to jpetso's ideas in comment #6. Note his opposition to this patch is basically the same as mine.

People would be harmed by this. The harm here is that it starts the module down a path towards increasing complexity with little real benefit (since there is a workaround which achieves the same effect). If you think there is no harm in that, look at some of the giant monolithic contrib modules from the pre-cck era. e.g. location module. Their enormous size makes them harder to maintain and causes them to stagnate. If you look at what is happening to location now it is being split into smaller modules that do very specific things. Why should we start pushing imagefield in the opposite direction?

In my opinion, if you want a solution for this I suggest you work on making the "story node + 8 image nodes" recipe easier and slicker to implement/use. That is something which can be done today with a little glue code but which should ideally be possible without glue code.

Greggles,

I totally agree with you that the patch is not a perfect solution. But, I would prefer to have increased functionality today instead of waiting for someone to write a more abstracted generalized solution *sometime* in the future.

Additionally, I think the work flow for the workaround is not equivalent to the work flow of the patch.

Noderefernce solution:
click create content image -> upload image, title, description -> submit (x 8)
click create content slideshow -> add title, description, *remember titles of previous image* for nodereference (x 8) -> submit

Patch solution:
click create content slideshow -> add title, description, (upload image, title, description x8) -> submit

On the code side - would it make sense to add a hook in the imagefield.module to allow other modules to add metadata to the image?

I would like hear if other users of imagefield.module need this type of functionality.

Here is the ideal workflow for the nodereference solution, IMO:
1) Create slideshow
2) On submit page get presented with a sidebar that has a nodeform for creating the imagecontent, fill in and hit submit 8 times

Putting it in the sidebar you can have code that automatically knows "this node I'm looking at now is the one to put in the nodereference field." So, there is no "remembering titles of previous nodes" required. There's also no need to click create content image 8 times. Yes, this does require a bit more code and work but we should at least be comparing apples to apples on this.

Your description of the patch solution misses the part where submitting the image/title/description 8 times requires clicking a "more fields" button 8 times which is, IMO, equivalent to submitting a sidebar node 8 times. Right?

I am torn on this issue because certainly Greg is right about the bloat of modules. I have been following location as well and this point is not lost on me. However imagefield as it stands (again I'll point out it has additional fields with or without this patch) provides a very usable solution out-of-the-box for people. Image/File handling is one of the number one things you hear frustration with from new Drupal users. Not everybody has a developer sitting around to throw together glue code for them. None of the solutions presented above are as clean as what is here now, especially with the ajax uploader which is totally great.

I'd love to hear from some more voices on this.

dupe of http://drupal.org/node/151014 and http://drupal.org/node/213222.

I've applied several patches to several versions, and on all of them, whenever I go back to edit a node, I do not get the usual edit field, with the already uploaded images, I get a blank node form, however the already uploaded images and caption both remain, I just can't see them when I'm editing the node.

This is a feature request and duplicate. Leave this issue closed.