Re: [PATCHES] Fullscreen toolbar button / SMB hide arrows

Sorry for not answering before, but i didn't read the email until today.

El dom, 22 de 06 de 2003 a las 22:26, Crispin Flowerday escribió:
> > This patch looks ok to me. Only two minor problems:
> >         - Some comments are wrong (one says TbiZoom, and should say
> >         TbiFullscreen, and anothar says that the class is abstract
> >         thought it obviously isn't).
> >         - I would change gul_tb_button_use_toggle_button (widget) to
> >         gul_tb_button_set_use_toggle_button (widget, gboolean) to keep
> >         it consistent with the rest of functions.
> 
> Changes made, (those same incorrect comments are in most of the other
> toolbar button source files).
> 

Yes, my fault.

> 
> > > 2) Hide SMB arrow - fixed it so it actually acts on the correct gconf
> > > key
> > 
> > Do we need this preference? I don't care if it is included (the patch
> > looks good).
> 
> It would be great if it could be committed, these arrows have been once
> of my main problems with galeon 2 (they just take too much space on the
> toolbar). It is non-intrusive in terms of the UI and the code changes
> are quite simple.
> 

Ok, no problem for me.

> > > 4) middle+click on smb icons opens in new tab (taking into account any
> > > text in the entry box) (this patch requires the ctrl+enter one as well)
> > >
> > > I changed the way the final patch is done. I have left the button-down
> > > signal handler for middle click on the base bookmarks-tb-widget class
> > > and just specified a new one which gets called earlier in the
> > > smart-site-tb-widget class.
> > > 
> > 
> > I'm not sure that relying in the order that signals handler get called
> > is a good idea. Are we sure that gtk+ guarantees that signals connected
> > later get called earlier always?
> 
> In fact GTK does guarantee that signals are called in the order
> connected, see :
> 
> http://developer.gnome.org/doc/API/2.0/gtk/gtk-Signals.html#gtk-signal-connect
> 

I thought you were connecting them in the opposite order... 

Anyway, I don't like it very much (it looks easy to get wrong). I would
prefer that you added a method to the parent class to connect (or
better, disconnect) the default signal handler and that the subclass
called that method if it wanted to override the behavior.

Thanks again for your work!

-- 
Ricardo Fernández Pascual
[email protected]
Murcia. España.




-------------------------------------------------------
This SF.Net email is sponsored by: INetU
Attention Web Developers & Consultants: Become An INetU Hosting Partner.
Refer Dedicated Servers. We Manage Them. You Get 10% Monthly Commission!
INetU Dedicated Managed Hosting http://www.inetu.net/partner/index.php