GNOME Bugzilla – Bug 348822
gconf-editor should use icons provided by themes
Last modified: 2008-11-12 21:08:13 UTC
gconf-editor currently does not blend in correctly with the rest of GNOME, because it uses hard-linked icons (I think). Instead, it should use stock icons. That way, we could have a tangoified gconf-editor, similar to some attempts to do the same (http://www.gnome-look.org/content/show.php?content=43225) but in an automatical manner. Is there any reason preventing this? Thanks!
Well, last time I checked it we were missing some icons on themes for the entries panel.
Created attachment 85167 [details] [review] Use themed folder icons With this patch, gconf-editor uses themed folder icons in the left pane. Please review; I'll take care of removing the custom folder icons when I commit.
Hmmm, for some reason gconf-editor refuses to use the standard GTK theme anymore (I use Clearlooks, but gconf-editor doesn't pick it up). Has it anything to do with initialization or something?
Disregard comment #3, it had to do with my locale settings. The patch works fine.
1. better use GTK_ICON_SIZE_MENU then hardcode size (16) 2. icons are not updated on theme change About 2: open gconf-editor, open Theme tool, change icon theme from GNOME to Crux, gconf-editor will use "GNOME" until rerun (but it could depend on my jhbuild sandbox)
*** Bug 386514 has been marked as a duplicate of this bug. ***
Luca is right about icons not changing, this is due to the app not listening to changes of the icon theme. From the API docs: "Note that you probably want to listen for icon theme changes and update the icon. This is usually done by connecting to the GtkWidget::style-set signal." Wouter, are you willing to update the patch?
(In reply to comment #7) > Wouter, are you willing to update the patch? My time is extremely limited, and so is my C knowledge. So, if someone else could do it, that would be great.
Same for me... well, I did a very small test app for this ("hello world" that displays an icon) and strangely enough, it works there without any further signals...
*** Bug 494348 has been marked as a duplicate of this bug. ***
No volunteers? ;)
Created attachment 106365 [details] [review] Updated Patch This patch will force gce to also use the schemas-types from the icon themes. Currently size is hardcoded to 24. MENU and LARGE_TOOLBAR are too small, it looks weird, using those sizes. icon names: folder folder-open entry-blank entry-bool entry-list entry-number entry-schema entry-string I also can't develop C, so the monitoring feature is still missing. Perhaps I'll have a look at filerollers sourcecode and try to adopt the code ?
The entry-* are not/won't be part of G-I-T, so they need to be installed privately. Do we have tango icons for those?
No. I they are shipped by gce, I just took the old names. If you have better Ideas, eg: text-x-plain instead of entry-blank, then I'll update the patch.
Created attachment 106424 [details] entry-* icons Ok. I just put some basic entry-* icons based on the git together.
Christopher, those icons are too big (48x48, we need 16x16). Also, I think the metaphors are not that great. Here is what dobey on IRC suggested: - bool icon: a toggle switch - string: plain text icon - int: a 4 or something - float: 3.0 or something - binary: 10/11 We can easily get those for 2.24. Sadly, we missed the UI freeze for 2.22 :(
Sebastian Kraft has created Tango Icons for gce. The folder need to be droped and the icons have to be renamed. Have a look, here: http://www.gnome-look.org/content/show.php/tango-gconf?content=43225 float and int are currently the same: entry-number. I'll update the patch for size 16x16. Do you have suggestions for better filenames?
I don't really like those icons... they are not really tangoish IMHO and for example the checked box looks strange for boolean. As for the names, let's just stick with the words used by gconf itself: type-boolean type-integer type-float type-string type-list
Umm. What is PAIR? there's a: case GCONF_VALUE_PAIR: /* FIXME: We need an icon for pairs */ icon = blank_pixbuf; So that I can create an Icon for it.
Christopher, could you update your patch to use the "custom themeable icons" as explained here: http://live.gnome.org/ThemableAppSpecificIcons ? The key type icons are useful only in gconf-editor, so we can't put them in gnome-icon-theme (moreover g-i-t maintainers will not accept any new icon that isn't in fd.o icon namins spec standard) or install in "gnome" icon theme. So gconf-editor will have to install them in $prefix/share/gconf-editor/icons/hicolor directory and add this directory to icon theme search path. This is explained in previously linked page on wiki. About names, the last proposal ("type-<type>") seems good to me. If we want to be more specific, we could add a "gconf-" prefix, but IMHO is not needed. PS targeting to gnome 2.24.x due to ui freeze PPS a pair key is like a list key that can store only two primitive values. The big difference is that list keys values must be of the same type, pair keys values can be different. I'm not sure pair keys are currently used....
Created attachment 106440 [details] type-* icons Updated Icons. SVG and 16x16 PNG
Created attachment 106441 [details] [review] Broken? Patch Can anyone test this patch? 'cause then I apply it, the icons are set to type-*, but the old entry-* are still loaded. Can anyone reproduce and fix that? >> Christopher, could you update your patch to use the "custom themeable icons" as explained here:? No. Like mentioned before I can't develop in C. The icon name changes are trivial, but this is more complex.
Christopher: The 16x16 versions lacks some pixel-love. Currently the look a bit blurry. You need to make the lines align to the 16x16 grid so it looks sharp.
(In reply to comment #23) > Christopher: The 16x16 versions lacks some pixel-love. Currently the look a bit > blurry. You need to make the lines align to the 16x16 grid so it looks sharp. > That's because they are just scaled-down 48x48 svgs. One size doesn't fit all ;)
just fyi, I was once told on #tango that you can actually make "pixel perfect" SVGs, by having an alternate svg image whose area is at 16x16, and then you scale and adjust its contents so that it is pixel perfect. I've used this once or twice, I think it indeed works (and allows you to keep your workflow in inkscape).
Created attachment 106558 [details] Another incomplete patch Well... another incomplete patch. It converts all of gconf-editor to use icon names, well, all but the tree and list view :( I know what has to be done and I managed to get the bookmark listview to work this way without a problem. But my knowledge about cell renderers is too limited to also fix the main window. Basicly, you have to use "icon-name" instead of "pixmap", but the way the views are written makes this a pain. I hope someone else will be able to finish this. Anyway, also included is a new icon installation Makefile and support for loading icons from a private hicolor directory.
Created attachment 106823 [details] tangofied icons for gconf-editor
Created attachment 109042 [details] [review] Working Patch Working Patch. Known Issues: #1 Somehow it's impossible to split INT and FLOAT (that's why INT's code is commented), if we split them only the icons for schema list and string show up, bool, pair, float, int and blank won't ... #2 We currently don't listen for a changed icon theme
Created attachment 109043 [details] Renamed Icons Icons posted by Michael Monreal, renamed to match the patch.
Created attachment 109044 [details] Renamed Icons and updated Makefile.am Again Micheals Icons and his Makefile.am. Changes: - Renamed Icons to match the patch - Install Icons into $(datadir)/icons/hicolor/16x16/mimetypes not in $(pkgdatadir)/icons/hicolor/16x16/status - Don't install Bookmark Icons, already part of g-i-t
Note that those are not mime types (well, not really status but whatever). Also, git does not (and will probably never have) an up-to-date (tango) bookmark icon, because it is not part of the naming spec. Only bookmark-new.
>> Note that those are not mime types (well, not really status but whatever). Yes, but since the icons show the type of the value mimetypes is the best solution in my eyes. >> Also, git does not (and will probably never have) an up-to-date (tango) >> bookmark icon, because it is not part of the naming spec. Only bookmark-new. It does: /usr/share/icons/gnome/16x16/actions/bookmark-new.png /usr/share/icons/gnome/16x16/actions/bookmark_add.png /usr/share/icons/gnome/16x16/actions/bookmarks_list_add.png /usr/share/icons/gnome/16x16/actions/stock_add-bookmark.png /usr/share/icons/gnome/16x16/actions/stock_help-add-bookmark.png /usr/share/icons/gnome/16x16/filesystems/gnome-fs-bookmark-missing.png /usr/share/icons/gnome/16x16/filesystems/gnome-fs-bookmark.png /usr/share/icons/gnome/16x16/stock/generic/stock_help-add-bookmark.png /usr/share/icons/gnome/16x16/stock/object/stock_bookmark.png /usr/share/icons/gnome/16x16/stock/object/stock_delete-bookmark.png /usr/share/icons/gnome/16x16/stock/object/stock_edit-bookmark.png
We're planning on killing off the stuff in /stock, so if gconf can ship with a bookmark icon itself it would be great!
Created attachment 109058 [details] [review] Updated Patch OK. I didn't know that. Here's an updated patch, to use gnome-fs-bookmark and bookmark_add instead (shipped in git/actions and git/places) <- I guess those won't be removed
There are too many unneeded changes in your patch. Particularly a lot of whitespaces changes, where the code doesn't change, but the indentation does. Those should be removed, and the previous indentation should be kept. The gnome-fs-bookmark icon is not staying. It is in filesystems/ not places/, and the filesystems/ context is slated to go away along with stock. Also there is no such icon as "bookmark_add". There is "bookmark-new" in actions, though it should not be used for both Add and Edit actions. For Edit, one should probably just not use an icon. Likewise, a menu full of repetitive icons can become confusing, so perhaps the usage of a "bookmark" icon should be removed as well, and only bookmark-new be used. Also your patch is not generated properly. If you want to use the "diff" command to manually generate the patch, please copy the old source to filename.old, and use "diff -up filename.old filename" to get a diff of the file. Your latest patch will fail to apply because ./gconf-tree-model.c and ./src/stock_new do not already exist for everyone. The preferred way of course, is to edit the files in place, in your svn checkout of the source, and use "svn diff" to generate the patch instead.
Created attachment 109061 [details] [review] Updated Patch OK. - Removed some useless code - use document-new and bookmark-new btw: bookmark_add is a link to bookmark-new
Created attachment 109062 [details] [review] Updated Patch Found another useless hunk
(In reply to comment #36) > btw: bookmark_add is a link to bookmark-new Probably for compatibility with KDE. These symlinks do not exist in the development release, and are only in the stable release for compatibility. If it's not a regular file ending in .png or .svg, it's not an icon name, and even then, a lot of them shouldn't be used, like those in stock/ as Andreas mentioned. See the Icon Naming Specification for more details on proper names.
ring ring ring nobody at home? (who could have a look at the patch from comment #37)
The icons are not updated when the theme is changed ("style-set" signal). And gtk_icon_theme_get_for_screen should be used to get the icon theme for the current screen (icons need to be updated on the "screen-changed" signal). See for example http://svn.gnome.org/viewvc/gtk%2B/trunk/gtk/gtkimage.c In gconf_list_model_get_value it should be enough to use one g_value_init call for all different cases.
Created attachment 111500 [details] [review] remove useless init calls >> The icons are not updated when the theme is changed ("style-set" signal). And gtk_icon_theme_get_for_screen should be used to get the icon theme for the current screen (icons need to be updated on the "screen-changed" signal). See for example http://svn.gnome.org/viewvc/gtk%2B/trunk/gtk/gtkimage.c Sorry, but my C knowledge is not very big, an example like http://live.gnome.org/ThemableAppSpecificIcons for the ThemeAppSpec would be nice. >> In gconf_list_model_get_value it should be enough to use one g_value_init call for all different cases. Done
Instead of loading these icons and storing them in statics, you should simply make the cell renderer use the icon names directly, ie by setting the icon-name property instead of the pixbuf property.
For the bookmark entry, the recent fdo's icon spec has 'user-bookmark'. Though there is no icon designed yet, but I believe it would suit the bookmark entry of gce. Ring ring, status of the patch?
>> Ring ring, status of the patch? Only re-loading icons on icon-theme-change will gce is running is currently missing, but my GTK/C skills don't allow me to add it (I just got no clue). The rest should be OK.
(In reply to comment #44) > Only re-loading icons on icon-theme-change will gce is running is currently > missing, but my GTK/C skills don't allow me to add it (I just got no clue). Comment 42 provides the solution to that.
Fixed in svn trunk. _Except_ that I re-used the old icons, since the icons provided here didn't have a licence attached (e.g. attachment 109044 [details]) or seem to be CC licensed. We need GPL 2+ or LGPL 2+ ones for gconf-editor. If the creator of those icons could clarify that, I'll gladly check them in to svn.
I don't think the icons Christopher Bratusek posted are CC. Christopher, can you confirm?
Those Icons are just renamed versions of the ones you have posted (I did that to match my patch), so you have to confirm :)
Ah, no, Sebastian Kraft seems to be the author of most of them :) Is he still CC'd here? We could however use the user-bookmarks icon (new in gnome-icon-theme 2.23.x) here instead of the old stock_bookmark and perhaps ship the bookmark-edit I packaged above (which is gpl2). Each one just requites a one-line change in src/gconf-stock-icons.h.
Sebastians are CC'd. I also have an account on gnome-look.org, so I'll ask him for a license change to GPL2 or LGPL2.
Oh while reading the comments on gnome-look.org for his icons, I just found out, that they are part of tango. So he can't re-license them. I guess we have to make new defaults, or leave the old ones. ...
What a mess :) I was talking about http://bugzilla.gnome.org/attachment.cgi?id=106823&action=view which only includes icons which are either original or based on GNOME-Icon-Theme. (the theme on gnome-look looks a bit odd, the tango icon theme part is only the folders, though)
None of those icons are in tango-icon-theme, so I have no idea what that comment might be talking about. The paper sheet in Tango is significantly different, with the 3-hole punch look on one side, and without a flap on the corner. The ONLY thing in those icons that I see which might be taken from tango-icon-theme is the "text" bit on the "string" type icon. Either way, it would be extremely trivial to take the paper sheet from gnome-icon-theme and make icons in the Tango style, if the licensing really is an issue (which it really isn't). (In reply to comment #51) > Oh while reading the comments on gnome-look.org for his icons, I just found > out, that they are part of tango. So he can't re-license them. I guess we have > to make new defaults, or leave the old ones. ... >
Ah yes. The theme on gnome-look.org (not the attachment here), is licensed CC-by-SA because it includes the Tango (and Tangerine) folder icons. He had originally licensed the images as GPL anyway.
Created attachment 116911 [details] Proposal So here's another proposal. Using the included (renamed to replace the icons in trunk 1:1) icons by needcoffee which are very similar to the current pack, only Tango. The included patch... a) removed the stock_edit-bookmark usage. The icon is not useful and will make theming harder (bookmark-new being a fdo spec icon). Also, see nautilus' bookmark menu, it does not use any icon for the same thing, too. b) Change stock_bookmark to folder icon. Why? Well, this also matches nautilus: The bookmarks are all folders actually and we show the folder icon in the tree view, so why not here as well? c) Use window-new for the first new window action and don't use any icon for the other two (3 times the same icon looks bad imho)
cpe commited a patch, based on mine to gconf-editor 2.23x. closed
Trunk is still using the old icons though... not fixed imho, still no tango love :(
2008-08-18 Christian Persch <chpe@gnome.org> * data/icons/Makefile.am: * data/icons/*.png: * src/Makefile.am: * src/gconf-tree-model.[ch]: * src/gconf-list-model.[ch]: * src/gconf-stock-icons.[ch]: * src/gconf-editor-window.c: Use themed icons. Bug #348822; partially based on patches by Christopher Bratusek. This is the commit. and it is working. It's just like that, that there are no new icons. But this is not the issue of this bugzilla entry. If you want to replace the default Icons, you should open a new bug. Ahh ... yes: Here's a screenshot of gconf-editor whith my OxygenRefit2 Iconset (so that you can see, that it is fixed): http://www.nanolx.org/other/gce.png