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 405505 - Tabs menu should have favicons instead of radiobuttons
Tabs menu should have favicons instead of radiobuttons
Status: RESOLVED FIXED
Product: epiphany
Classification: Core
Component: Interface
git master
Other Linux
: Normal enhancement
: ---
Assigned To: Epiphany Maintainers
Epiphany Maintainers
Depends on:
Blocks:
 
 
Reported: 2007-02-07 20:22 UTC by Xan Lopez
Modified: 2010-02-19 07:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to show favicons instead of radio buttons in the tabs menu (4.50 KB, patch)
2010-02-14 19:13 UTC, Olivier Tilloy
none Details | Review

Description Xan Lopez 2007-02-07 20:22:50 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 :)
Comment 1 Olivier Tilloy 2010-02-14 19:13:06 UTC
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.
Comment 2 Diego Escalante Urrelo (not reading bugmail) 2010-02-14 19:52:45 UTC
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()
Comment 3 Olivier Tilloy 2010-02-14 22:47:53 UTC
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)?
Comment 4 Reinout van Schouwen 2010-02-15 09:17:50 UTC
I vote aye!
Comment 5 Gustavo Noronha (kov) 2010-02-18 19:06:14 UTC
I suggest we apply for a freeze break for this. The impact is minimal, and there's a nice usability gain.
Comment 6 Xan Lopez 2010-02-18 19:17:35 UTC
(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.
Comment 7 Xan Lopez 2010-02-19 07:05:03 UTC
OK, I got the two approvals and landed this as 2df12ad588. Thanks for the patch!