Re: [patch] nautilus window border icon

[Dave Camp <dave ximian com>]

Looks good, please commit with ChangeLog.

-dave

On Mon, 2002-11-04 at 16:01, Jan Arne Petersen wrote:
> On Sun, 2002-11-03 at 20:45, Dave Camp wrote:
> 
> Hi,
> 
> updated patch and a nautilus-desktop.png icon (the gnome-show-desktop
> icon) is attached.
> 
> > On Sat, 2002-11-02 at 16:57, Jan Arne Petersen wrote:
> > 
> > > -static void
> > > -update_title (NautilusWindow *window)
> > > +void
> > > +nautilus_window_update_title_and_icon (NautilusWindow *window)
> > 
> > It doesn't seem like there's any reason to tie together the title and
> > icon setting into one function.  You could probably get away with just
> > adding a nautilus_window_update_icon function and leaving the title
> > handling alone (maybe putting the special case in compute_title()) 
> 
> done.
> 
> > >  {
> > >          char *title;
> > >          char *window_title;
> > >          GList *sidebar_panels;
> > >          GList *node;
> > > +	char *path;
> > > +	GdkPixbuf *pixbuf;
> > >  
> > > -        title = compute_title (window);
> > > +	/* Desktop window special title and icon */
> > > +	if (NAUTILUS_IS_DESKTOP_WINDOW (window)) {
> > > +		path = nautilus_pixmap_file ("nautilus-desktop.png");
> > > +
> > > +		if (path != NULL) {
> > > +			pixbuf = gdk_pixbuf_new_from_file (path, NULL);
> > > +			gtk_window_set_icon (GTK_WINDOW (window), pixbuf);
> > >  
> > > +			g_free (path);
> > > +			g_object_unref (pixbuf);
> > > +		}
> > > +
> > > +		gtk_window_set_title (GTK_WINDOW (window), _("Desktop"));
> > > +		return;
> > > +	}
> > > +	
> > > +        /* Update window border icon */
> > > +	pixbuf = nautilus_icon_factory_get_pixbuf_for_file (window->details->viewed_file,
> > > +							    "accept",
> > > +							    NAUTILUS_ICON_SIZE_STANDARD);
> > 
> > You probably shouldn't use the "accept" modifier here.  "accept" doesn't
> > necessarily mean "open", there's at least one theme that just changes
> > the color of the icon.
> 
> done.
> 
> > > +	/* Register IconFactory callback to update the window border icon
> > > +	 * when the icon-theme is changed.
> > > +	 */
> > > +	g_signal_connect_object (nautilus_icon_factory_get (), "icons_changed",
> > > +				 G_CALLBACK (nautilus_window_update_title_and_icon),
> > > +				 window, G_CONNECT_SWAPPED);
> > 
> > Your callback should match the signature of the function that's going to
> > be called by the signal handler.  It's probably best to not use SWAPPED
> > and just add a helper function.
> 
> done.
> 
> Jan Arne Petersen
-- 
Dave Camp <dave ximian com>