Use imagecache for thumbnails

attiks - March 14, 2009 - 18:35
Project:ImageField
Version:6.x-3.0-alpha4
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:needs work
Description

Included is a patch that allows you to use imagecache for the thumbnail previews.

AttachmentSize
imagecache_preview.patch9.98 KB

#1

attiks - March 14, 2009 - 19:35
Status:active» needs review

This patch only changes the thumbnails you see on node/x/edit.

Imagecache settings are only available if imagecache is installed, without imagecache all remains the same.

Can someone please do a quick code review on the patch we can make this RTBC.

#2

attiks - March 14, 2009 - 18:52

New patch attached, only text changes

AttachmentSize
402014_imagecache_preview.patch 9.73 KB

#3

quicksketch - March 14, 2009 - 22:28
Status:needs review» needs work

Thanks for taking the effort to put this together! I'd like to see this available in ImageField, however I'm not entirely sure this is really the approach we'd like to take with ImageField. Could you explain why you'd like to use ImageCache (nearly making it a dependency for ImageField) rather than having ImageField be self-sufficient? #305621: Imagefield configurable upload thumbnail size has a lot of interesting discussion on the topic.

Besides that, some comments on the patch:
- This needs to be updated for HEAD, as the code has changed drastically.
- We don't need to pass in "useimagecachepreset" and "imagecachepreset" into theme functions. If "imagecachepreset" is NULL, then that means we're not using a preset.
- Variable names should not be all crammed together. To shorten it up a bit, could we use "use_preset" and "preset_name"?
- If we decide to make an imagecache-only solution, the code for hook_file_insert() and hook_file_delete() should be removed. And we'd need to write an update function that removes all the thumbnails.
- Rather than wrapping form elements in if (module_exists('imagecache')) {, a better approach is to use #access => module_exists('imagecache'). Values can be maintained even when ImageCache is disabled.

But before getting into rerolling, more explanation of your motives would be helpful. I personally don't like the way that ImageField puts a .thumb. file right in the directory with the original image, but my solution to this was to move all the thumbnails to a separate directory, similar to ImageCache. This would have the advantage of still not requiring ImageCache (which has the additional burden of requiring Clean URLs and ImageAPI).

#4

attiks - March 14, 2009 - 23:08

Thanks for the review, the patch was a quick and dirty job so I'll make a new one against head.

Motivation is three-fold:
1/ get rid of the thumb.jpg files to get rid of the clutter with WYSIWYG editors.
2/ all thumbs are generated as jpg files, even if you upload a (transparent) png.
3/ be able to give more accurate previews, suppose you display images in black and white on your site, wouldn't it be nice to have the thumbnails on the edit form in black and white as well.

The idea is to make the imagecache part optional so that imagefield can work without imagecache. I'm also in favor of a separate direcotry for all thumbnails generated by vanilla imagefield (see http://drupal.org/node/305621#comment-1068302).

#5

quicksketch - March 14, 2009 - 23:40

2/ all thumbs are generated as jpg files, even if you upload a (transparent) png.

At least this one is already fixed. I changed this in imagefield_update_6004().

The idea is to make the imagecache part optional so that imagefield can work without imagecache.

Sounds good, but let's make sure that thumbnails still work without ImageCache. Putting all the thumbnail images into a separate directory would fix your first concern. If you're up for writing that fix that would be fantastic, but we should put it in a separate issue.

#6

quicksketch - March 15, 2009 - 05:38

1/ get rid of the thumb.jpg files to get rid of the clutter with WYSIWYG editors.

I made some more serious changes to the functionality of ImageField thumbnails in #307503: Move *.thumb.jpg files to a separate directory. Thumbnails are now stored separately, not all mixed in.

I definitely still think this is important to get in though, please post an updated patch when you get the chance.

#7

attiks - March 15, 2009 - 19:09

Included a patch against head, following the remarks in #3

One problem remains: If you select preview on the node/x/edit page you'll get the classic thumbnail, so either I missed a function or the ['data'] part doesn't get filled in.

AttachmentSize
402014_head_imagecache_preview.patch 5.76 KB

#8

attiks - March 15, 2009 - 19:15

Some random thought:

If we provide an /admin/imagefield/settings page we can:
- make 'use imagecache for thumbnails' a global option and remove it from the widget
- add a default imagecache preset to use for all thumbnails
- add options for the custom image thumbnail size #307503: Move *.thumb.jpg files to a separate directory
- allow people to select the directory for the thumbnails #307503: Move *.thumb.jpg files to a separate directory

#9

arielon - August 22, 2009 - 07:09

This sounds good. Better than the current way of changing the '100x100' constraint by editing a variable on the settings.php file...

Is this going to 6.x-3.1?

 
 

Drupal is a registered trademark of Dries Buytaert.