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.
| Attachment | Size |
|---|---|
| imagecache_preview.patch | 9.98 KB |

#1
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
New patch attached, only text changes
#3
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
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
At least this one is already fixed. I changed this in imagefield_update_6004().
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
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
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.
#8
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
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?