GNOME Bugzilla – Bug 405505
Tabs menu should have favicons instead of radiobuttons
Last modified: 2010-02-19 07:05:03 UTC
I think a favicon to quickly locate the tab where you'd like to go is quite more useful than a radiobutton telling where you are, which should be pretty obvious. Will attach a patch soonish :)
Created attachment 153782 [details] [review] Patch to show favicons instead of radio buttons in the tabs menu Hey Xan & epiphany developers, I picked up this one as it seemed abandoned and looked like a useful way for me to get started on epiphany's codebase. I'm attaching a patch that replaces the radio buttons by favicons in the tabs menu. I reckon I did it the clean way, but of course any comments and suggestions to improve it are welcome.
Review of attachment 153782 [details] [review]: Hey Olivier, thanks for the patch. Right now we are in UI freeze though so we can't commit this, let's see what others think about enabling this in the next version. Seems like a good idea though :). ::: src/ephy-tabs-menu.c @@ -299,1 @@ - LOG ("active tab is embed %p", embed); Nit pick: I think you should keep this LOG()
Thanks for the fast review Diego. The removed LOG() was in a callback (sync_active_tab) that the patch removes altogether. Do you reckon I should re-add the callback just for the sake of logging? Wouldn't it be more sensible to add such a LOG() in e.g. ephy_window_set_active_tab (ephy-window.c)?
I vote aye!
I suggest we apply for a freeze break for this. The impact is minimal, and there's a nice usability gain.
(In reply to comment #5) > I suggest we apply for a freeze break for this. The impact is minimal, and > there's a nice usability gain. So yeah, this patch looks good to me and puts me to shame for not doing it before (so much for 'will attach a patch soonish'), so let's ask for a freeze break and commit it! I'll do that now.
OK, I got the two approvals and landed this as 2df12ad588. Thanks for the patch!