I've made a small change to the upload module to make it possible to show a link under the teaser text on the homepage similar to the one shown by the comments module to indicate that a node has attachments.

I find this feature particularly useful if you create an article that is so small that all text does fit in the teaser on the homepage. In that case there will be no 'read more' link under the article, if you've added an attachment a user scanning the homepage will not notice it's existence.

This feature is configurable by the admin - default value is off, but maybe it should be on.

I hope someone who has CVS access will commit this patch - or notify me if it should be changed in some way to get it accepted.

(Sidenote: This is my first Drupal modification and I don't do a lot of php programming. This is just a simple bit of cut-and-paste programming so please tell me if it can be improved)

CommentFileSizeAuthor
#9 upload_attachment_node_link_0.patch1.49 KBmatt westgate
#8 upload_attachment_node_link.patch1.35 KBmatt westgate
upload.module.diff1.53 KBsimon_g

Comments

Bèr Kessels’s picture

Bèr Kessels commented

two minor things:
adding more configuration options is considered bad: unless absolutely nessesary
you use a tab instead of two spaces before $links[] = drupal coding style only allows spaces.

furthermore +1 for this functionality

simon_g’s picture

simon_g commented

thanks for the quick response!

Do you (or anyone else) need a new diff with spaces instead of tabs?
I just noticed the diff should have been against the CVS HEAD - I checked it and the line numbers are identical, so it won't be really needed for that.

About the number of configuration options: The upload module currently has only one 'global' configuration option (the max size) so I think a second setting will not hurt a lot. I agree that too many options is bad, maybe it's just that I was hesitant about anyone else wanting this functionality.

I'll create a patch without the configure options if needed, but how do I know what's the most favored version? (Or is one +1 enough for all?) [I've read a bit in the "Contributers guide" but I'm not really sure about the process.

Bèr Kessels’s picture

Bèr Kessels commented

I am not in charge, at all, of any patches that might go to core.

I merely gave my ideas, so that if a core cvs maintainer passes by he will notice that the patch was reviewed already, and then might easier decide to commit it.

Bèr

Steven’s picture

Steven commented

Wouldn't it be better to have a little attachment/paperclip icon instead? The link is more so informative rather than for clicking.

simon_g’s picture

simon_g commented

I like the idea - but at what location should that icon be shown? At the same place the text will display now?

And I think the link certainly is for clicking if the text is short and therefore there is no "read more" link. I know you can click the title (at least in the bluemarine theme you can) but that doesn't really feel intuitive to me. (Probably having something to do with moving your mouse up from where you were reading instead of down in the 'normal' reading direction)

And I don't see a lot of images being used in the rest of Drupal - how would something like that interact with the templating part? Are there examples of modules using images in the user-interface part?

Simon

boris mann’s picture

boris mann commented

This should be added as a regular link, ideally something like "download attachment (filename.xxx)". BUT, I agree that at the theme level you would want to easily style ANY of those links as an icon.

But...we currently can't order or edit or replace-with-icons any of those node links. So add the link as plain text, then lets get to work on some theming improvements.

simon_g’s picture

simon_g commented

I wouldn't want to have links to all the attachments on the homepage, if you have more than one attachment that would clutter the homepage too much I'd think -- My patch is only meant to make the user aware of the existence of the attachtment(s) so they will know there is more to see if they click the article.

There is already a nice table of attachments in the page of the article itself, if you really want all the info on the frontpage you could just as well include that table in the teaser text.

matt westgate’s picture

matt westgate commented
StatusFileSize
new upload_attachment_node_link.patch1.35 KB

+1

This feature makes it possible to know what nodes have attachments when multiple nodes are shown on a page in 'teaser mode'. This is a great usability feature especially when one is making reference to an attached file in node->teaser that currently is only visible at the node/nid/view level.

This new patch applies to CVS HEAD and removes the setting of making this feature optional. I don't think that's necessary given that it's a usability issue and not a feature that changes the way the module behaves and treats files.

matt westgate’s picture

matt westgate commented
StatusFileSize
new upload_attachment_node_link_0.patch1.49 KB

Slight revision to my last patch. Only show the attachments link when an attached file has the 'list' option checked. It doesn't make sense to say that attachments exist if none are listed.

killes@www.drop.org’s picture

killes@www.drop.org commented

Sounds usefull. And still applies.

Steven’s picture

Steven commented

Committed to HEAD.

I changed two things:
- Added an anchor to the attachments link and table to scroll down to the attachments list automatically.
- Added check for "view uploaded files" permission.

Anonymous’s picture

(not verified) commented