After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 359640 - Support for icon themes
Support for icon themes
Status: RESOLVED FIXED
Product: glade
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: 3.4.0
Assigned To: Glade 3 Maintainers
Glade 3 Maintainers
: 400423 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2006-10-04 16:37 UTC by Tristan Van Berkom
Modified: 2007-08-14 19:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot - named icon browser (33.05 KB, image/png)
2006-12-14 22:54 UTC, Vincent Geddes
  Details
screenshot - named icon browser (42.12 KB, image/png)
2006-12-16 23:18 UTC, Vincent Geddes
  Details
demo (10.04 KB, application/x-gzip)
2006-12-16 23:49 UTC, Vincent Geddes
  Details
new screenshot (42.93 KB, image/png)
2006-12-18 17:35 UTC, Vincent Geddes
  Details
Latest code (94.01 KB, application/x-gzip)
2007-01-16 00:17 UTC, Vincent Geddes
  Details
Icon Chooser (93.80 KB, application/x-gzip)
2007-04-03 14:44 UTC, Vincent Geddes
  Details
Named Icon Chooser (68.23 KB, patch)
2007-07-06 23:35 UTC, Vincent Geddes
none Details | Review
Named Icon Chooser (68.29 KB, patch)
2007-07-06 23:39 UTC, Vincent Geddes
none Details | Review

Description Tristan Van Berkom 2006-10-04 16:37:39 UTC
After discussion with mclasen and dobey on irc, the desired
policy for icon themes is as such:

   - icon themes should all implement the list of icon names
     in the spec (http://freedesktop.org/wiki/Standards_2ficon_2dtheme_2dspec)
   - applications should use icons from the icon theme or icons that
     the application itself is adding/providing as a custom icon
   - applications should not use weird icon names from themes that may
     or may not be installed

Glade should be adapted first to implement a dropdown userfriendly
list of icons from the spec, and allow the user to type in by hand
any custom icon name that they want to use - in which case we can
display "gtk-broken-image" for that graphic in the UI and hopefully
find a way to make it clear to the user that using a custom icon name
means that they must ensure that that icon is installed on the target
host.
Comment 1 Vincent Geddes 2006-12-14 22:54:15 UTC
Created attachment 78397 [details]
screenshot - named icon browser
Comment 2 Vincent Geddes 2006-12-16 23:18:34 UTC
Created attachment 78480 [details]
screenshot - named icon browser
Comment 3 Vincent Geddes 2006-12-16 23:49:40 UTC
Created attachment 78483 [details]
demo
Comment 4 Vincent Geddes 2006-12-18 17:35:27 UTC
Created attachment 78580 [details]
new screenshot

added a GtkEntry
Comment 5 Vincent Geddes 2007-01-16 00:17:24 UTC
Created attachment 80348 [details]
Latest code

Here's the latest code. Feature complete except for stock item browsing.

Todo: 

 * Figure out how to support stock items cleanly.
 * refine/refactor API, reduce number of source files.
Comment 6 Jean-François Fortin Tam 2007-01-25 02:47:21 UTC
*** Bug 400423 has been marked as a duplicate of this bug. ***
Comment 7 Vincent Geddes 2007-04-03 14:44:39 UTC
Created attachment 85767 [details]
Icon Chooser
Comment 8 Vincent Geddes 2007-04-03 23:04:53 UTC
I have now reduced the number of source files from 8 to 6. This is now a total reduction from an original 14 files (Wow!). 

Your concerns about the number of source files are entirely valid, 14 is waay too much for glade to handle, so the reduction to 6 files is a great improvement.


Some File Descriptions:

glade-named-icon-chooser.[ch]
-----------------------------

Fundamental interface for icon chooser implementations. Allows us to implement many different kinds of icon chooser widgets without having to duplicate API unnecessarily. Essentially provides a consistent interface for icon-choosing widgets.

For example, the GtkFileChooser interface provides the fundamental API for GtkFileChooserDialog, GtkFileChooserWidget, and GtkFileChooserButton.
 
These files also contain the GladeNamedIconChooserEmbed interface, which provides a communication path between a chooser widget and the toplevel window which embeds it. This communication path is essential for correct operation. It is possible for me to remove this interface and put the methods and signals on
GladeNamedIconChooserWidget instead. I might actually do that.

glade-named-icon-chooser-dialog.[ch]:
-------------------------------------

A dialog for selecting icons. Implements the GladeNamedIconChooser interface for exposing API, and delegates functionality to a GladeNamedIconChooserWidget child. This dialog can be used anywhere in glade and is not tied to the notion of a GladeEditorProperty.

I would really prefer not to have glade-named-icon-chooser-dialog.c merged with glade-editor-property.c. I can't see how doing that would be better than the way it is now - The very idea scares me. The whole point of object-orientation (for me) is to compartmentalise and reuse code so that it is easier to maintain.

glade-named-icon-chooser-widget.[ch]
------------------------------------

The implementation. Also implements the GladeNamedIconChooser and GladeNamedIconChooserEmbed interfaces.


Please can we compromise and move on now, seriously? I have culled the number of source files to 6, and requiring any more culling is just going to make me not care anymore.
Comment 9 Tristan Van Berkom 2007-04-03 23:50:46 UTC
(In reply to comment #8)
[...] 
> Please can we compromise and move on now, seriously? I have culled the number
> of source files to 6, and requiring any more culling is just going to make me
> not care anymore.
> 

No, we dont need all this code, sorry not going to happen, period.

I explained myself on irc and I wont spend an hour of my time expaining
it to you here - simply put, there is no need for an iconchoosing
widget, dialog and button so no need for an interface, theres no
need to create a custom dialog in glade when theres a perfectly good
GtkDialog in gtk+ that we can pack an icon choosing widget into.

I'm sorry that you dont agree and that you aperently dont care anymore,
this not caring also comes across as a pressure tactic - I dont want
to snap as a response but its the truth - I wont sacrifice simplicity
in glade and add your (in my opinion) overly convoluted code to glade,
its just out of context here.
Comment 10 Vincent Geddes 2007-04-04 00:46:51 UTC
> No, we dont need all this code, sorry not going to happen, period.
> 

Ok, I understand. No hard feelings. If you or someone else can come along and forge my existing code into something suitable for glade, then that would be appreciated. 

> I'm sorry that you dont agree and that you aperently dont care anymore,
> this not caring also comes across as a pressure tactic - I dont want
> to snap as a response but its the truth - I wont sacrifice simplicity
> in glade and add your (in my opinion) overly convoluted code to glade,
> its just out of context here.
> 

It's not a pressure tactic, I really just don't care anymore. I spent months improving the design and I am not going to spend another month undoing that good design. At least I am happy that I gained valuable experience while creating the icon chooser.
Comment 11 Vincent Geddes 2007-04-12 01:43:28 UTC
OK, well it's been about a week and I have cooled down a bit. I still heartily disagree, but that's not the issue now. Abandoning the code because of personal differences would be a great folly. So let's try to salveage something.

The most important thing for me is to have encapsulation of code and behaviour wherever possible. For me that also includes putting chunks of code into separate files where appropriate. So to that end, how about providing a dialog only - no interfaces or widgets?

With such a design we would only have 2 files:
glade-named-icon-chooser-dialog.[ch]

The stock item chooser would be similar:
glade-stock-item-chooser-dialog.[ch]
Comment 12 Tristan Van Berkom 2007-04-12 02:54:58 UTC
That would be great - it would even be better if glade_icon_chooser_new () 
came with a GLADE_STOCK vs GLADE_ICON_THEME argument if its at all possible
to share all that treeview code and only populate it with different data
depending... but at this point I think its better to take baby steps, lets
start with the icon theme dialog and work from there.

Now, not in the hope of starting circular argument, just to try to express
the way I see it, you dont have to agree with me just try to understand;
glade is an application and not a platform library - so the code in glade
is always "applied" - all the extra interfaces and stuff that the previous patch
adds would have only one single application: the dialog. So from my point
of view - all that extra code is like what we reffer to in the industry as 
dead code; support for alot of use cases that just will never happen
in this application. I never wanted to imply that your design was bad,
only out of context and overdesigned for the straightforward use case
that its needed for.
Comment 13 Vincent Geddes 2007-07-06 23:35:15 UTC
Created attachment 91329 [details] [review]
Named Icon Chooser

Note that in the glade_editor_property_new() factory function, I had to insert a special case in order to assign the icon-chooser EProp to G_TYPE_STRING properties which have the id "icon-name". This works fine for GTK+ widgets, but it still seems a bit hacky and unsafe.

Maybe we need some GladePropertyClass API to handle these special cases in a clean way. Then GladeEditor can get the required EProp GType from the GladePropertyClass instead of using glade_editor_property_new().
Comment 14 Vincent Geddes 2007-07-06 23:39:42 UTC
Created attachment 91331 [details] [review]
Named Icon Chooser
Comment 15 Tristan Van Berkom 2007-08-14 19:11:05 UTC
the code is in trunk, closing this one.

currently some properties have been marked to use
the themed icon chooser but this might break the UI freeze,
so if we dont get authorization to release with the
new dialog then we'll still keep the code in, but
just not mark any properties to use it (doubt that
will be the case but just marking here in case of
confusion).