GNOME Bugzilla – Bug 461447
Add option to select tag icon from file
Last modified: 2007-10-09 13:06:10 UTC
New tags can be set to have a stock icon, or a scaled down photo from the catelogue. I propose a third option which lets the user select an image from his files. Such images could be custom-made icons or downloaded images that have no place in the photo catelogue.
tagged as gnome-love, easy to implement
Created attachment 96039 [details] Set a default icon when you create a new tag In this patch, when you create a new tag set a default icon. If the new tag is a child tag, take the icon from parent tag
Miguel, this patch is off topic :-) And BTW I don't think it was needed. Some people prefer NOT to have icons associated to tags.
Created attachment 96046 [details] [review] Proposed solution - WIP - TagCommands (small patch) This is a working solution to the problem. I must test it more however and internationalise it. Please do not yet review.
Created attachment 96047 [details] [review] Proposed solution - WIP - f-spot.glade (not cleaned up) This is the accompanying glade file to my previous patch. I have yet to pick through it and tidy it up. Please do not yet review.
Created attachment 96049 [details] [review] Cleaned up the previous patch. Ready for review. This is a cleaned up version of the previous patch I supplied and is ready for review. It also requires the associated glade file to be patched in.
Created attachment 96050 [details] [review] Cleaned up the previous patch. Ready for review. This is a cleaned up version of the previous patch I supplied and is ready for review. It also requires the associated TagCommands file to be patched in.
Working for me, great. Just my 2 personal notes: 1. I'd remove the hints you added over the file chooser and the standard stock icons. The hint showing in the upper part is visible only if there are no pictures attached to the tag, so it doesn't take more space. I think the help is a better place for those infos. 2. The texts at 1. made the window taller, and it isn't fitting in my monitor (1280x800, the highest resolution of my laptop), thus I cannot see the end of the stock icons and the ok/cancel buttons.
(In reply to comment #8) > Working for me, great. > > Just my 2 personal notes: > 1. I'd remove the hints you added over the file chooser and the standard stock > icons. The hint showing in the upper part is visible only if there are no > pictures attached to the tag, so it doesn't take more space. I think the help > is a better place for those infos. > 2. The texts at 1. made the window taller, and it isn't fitting in my monitor > (1280x800, the highest resolution of my laptop), thus I cannot see the end of > the stock icons and the ok/cancel buttons. > Sure thing. Will make those changes.
Created attachment 96109 [details] [review] Cleaned up the previous patch (for TagCommands.cs). I have removed the aforementioned text in this TagCommands.cs patch. You will still need the glade patch that is already attached to this bug. Ready for review.
Ok, fine to me. Seems to work great!
It looks fine (but not tested yet). Some comments though: - you set LocalOnly to false for the FileChooser but then you try to access the LocalPath of the Uri. It may fail if a non-local uri was selected. Instead, I recommend to use a code like this to load a pixbuf from an image. This way, you'll have raw support for free ;) Pixbuf pixbuf using (ImageFile img = ImageFile.Create(uri_to_open)) { pixbuf = img.Load (); } - check the identation of the code (nitpicking) - genrate your patches with "svn diff > mypatch". So you can have only one patch, even for multiple files
Created attachment 96321 [details] [review] Combined Glade plus TagCommands.cs with implemented recommendations I have updated the code to implement the suggestions. Some comments a.) I have implemented 2 using statements as otherwise external_image was not null on leaving method - and using is more efficient than Dispose(). Also I imagine that the nature of PixbufUtils.TagIconFromPixbuf () means that it is safe to dispose of the external_image as it is a transformed version that will be assigned to PreviewPixbuf anyway. Please advise if incorrect. b.) Hope the 'FSpot.ImageFile.Create(new Uri(external_photo_chooser.Uri))' is the correct way to form a network safe Uri - couldn't find much doco on how to correctly implement this c.) Only one patch file needed now d.) I've tried to follow the recommended indentation - let me know if there is still any probs. Other than that, all the best and happy hacking! Dec
committed in r3416