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 527532 - Use theme instead of custom drawing code to draw ETree
Use theme instead of custom drawing code to draw ETree
Status: RESOLVED OBSOLETE
Product: evolution
Classification: Applications
Component: general
2.26.x (obsolete)
Other All
: Normal normal
: ---
Assigned To: Evolution Shell Maintainers Team
Evolution QA team
evolution[cleanup] evolution[etable]
: 529206 530960 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2008-04-11 12:30 UTC by Johannes Berg
Modified: 2015-02-23 16:03 UTC
See Also:
GNOME target: ---
GNOME version: 2.25/2.26


Attachments
patch to use the theme instead of some custom drawing (6.48 KB, patch)
2008-04-13 13:21 UTC, Benjamin Berg
rejected Details | Review
etree style properties (7.42 KB, patch)
2008-05-05 13:43 UTC, Johan Euphrosine
reviewed Details | Review
e-tree and e-table style properties (8.62 KB, patch)
2008-05-06 12:37 UTC, Johan Euphrosine
reviewed Details | Review

Description Johannes Berg 2008-04-11 12:30:04 UTC
This new version of evolution uses custom treeview drawing code which is incompatible with my GTK theme and looks horrible. Please revert to drawing the treeview with the theme rather than custom code.


Distribution: Debian lenny/sid
Gnome Release: 2.22.1 2008-04-08 (Debian)
BugBuddy Version: 2.22.0
Comment 1 Benjamin Berg 2008-04-11 12:34:22 UTC
Well, not revert. You really should be using the theme to draw the ETree focused line background. 

As a starting point I'll point to http://live.gnome.org/GnomeArt/Tutorials/GtkThemes/GtkTreeView, but I am not sure how well it is described there (I wrote it, but hey). If you have any questions, feel free to ask me.

I'll take a quick look now. So maybe there'll be a patch already later today.
Comment 2 Benjamin Berg 2008-04-13 13:21:12 UTC
Created attachment 109171 [details] [review]
patch to use the theme instead of some custom drawing
Comment 3 André Klapper 2008-04-13 14:37:08 UTC
reviewers please ignore the mail/em-filter-i18n.h changes when reviewing/committing.
Comment 4 Srinivasa Ragavan 2008-04-14 04:13:46 UTC
Benjamin, Look better to me. Infact I wrote those to simulate and I didn't knew this. Thx.

Remove mail/em-filter-i18n.h changes while committing it to stable/trunk.

Also, you can revert my original changes completely. See

http://svn.gnome.org/viewvc/evolution?view=revision&revision=34855
http://svn.gnome.org/viewvc/evolution?view=revision&revision=34877
http://svn.gnome.org/viewvc/evolution?view=revision&revision=34914

Thanks and sorry to break it.

Comment 5 Johan Euphrosine 2008-04-21 15:25:24 UTC
*** Bug 529206 has been marked as a duplicate of this bug. ***
Comment 6 Benjamin Berg 2008-04-25 17:13:07 UTC
Sorry for not responding earlier.

Do you know whether DO_TOOLTIPS is ever enabled? I don't see any way it could be enabled, which means that quite some code can be deleted. (eg. with the patch the only place that uses eti_get_cell_background_color is in such an #ifdef'ed block)
Comment 7 Srinivasa Ragavan 2008-04-28 04:46:41 UTC
Benjamin, I really donno. For now, ignore it. We can clean it up later. Also I remember I noted some critial warnings on the console when I used your patch. Watch it out.
Comment 8 Benjamin Berg 2008-04-28 13:23:18 UTC
Yeah, like:
(evolution:30724): Gtk-WARNING **: gtkwidget.c:8486: widget class `ECanvas' has no property named `even-row-color'

Srinivasa, can't do much about the warnings :-/. They are comming from the buildin theme in GTK+, because it thinks it is drawing a GTK+ TreeView and then tries to lookup the "even_row_color" and "odd_row_color" style properties. This will fail, because we are drawing with an ECanvas widget, and not a GtkTreeView.
Comment 9 Johan Euphrosine 2008-04-28 13:51:09 UTC
Not sure, but does gtk-rc-get-style-by-paths could help, for having the Etree use the same style as GtkTreeView ?

http://library.gnome.org/devel/gtk/unstable/gtk-Resource-Files.html#gtk-rc-get-style-by-paths
Comment 10 Milan Crha 2008-04-28 14:07:40 UTC
I think you forgot to use alternating_row_colors property of the ETableItem, seems like with this patch is has no way to turn that off. The other thing, about properties, what about adding there these two new so one will be able to set it in rc file?
Comment 11 Benjamin Berg 2008-04-29 08:02:42 UTC
Oh, sorry, I had not realized the alternating_row_colors thing. All that needs to be done is to remove the "_ruled" from the detail string alternating colors is turned off.

There are different things that can be done to improve what is there with the patch:
 1. use gtk_rc_get_style_by_paths to get the "real" GtkTreeView style, in case some theme does a special case based on that. If done, this should also happen for eg. the header "buttons".
 2. Create a GtkTreeView widget, so that the style properties can be looked up from the GtkStyle. This means that the engine is able to use the "even-row-color" and "odd-row-color" style properties from the gtkrc. Evolution could also obey some other treeview style properties. (There are properties for paddings, and if even/odd row coloring is allowed for example.)

Both of these would be independent of each other. However I am not sure whether the benefits are worth it.
Comment 12 Johan Euphrosine 2008-05-05 13:43:17 UTC
Created attachment 110405 [details] [review]
etree style properties

Add GtkTreeView-like style properties to ETree to get rid of runtime warnings.
Comment 13 Matthew Barnes 2008-05-05 15:26:52 UTC
*** Bug 530960 has been marked as a duplicate of this bug. ***
Comment 14 Matthew Barnes 2008-05-05 16:42:21 UTC
Works great for me.  Very nice job!

I'll defer to Srini for final approval since this isn't my domain.
Comment 15 Matthew Barnes 2008-05-05 16:44:49 UTC
Also, would be nice if we could commit this to stable.
I wouldn't think fixing a UI bug would constitute a UI freeze break.
Comment 16 Benjamin Berg 2008-05-05 23:20:12 UTC
We might also want to add the style properties to ETable.
Comment 17 Johan Euphrosine 2008-05-06 10:55:46 UTC
Patch apply and run fine on:
http://svn.gnome.org/viewvc/evolution/branches/gnome-2-22/
Comment 18 Johan Euphrosine 2008-05-06 12:37:55 UTC
Created attachment 110456 [details] [review]
e-tree and e-table style properties

Add the following style properties to ETree and ETable:
::odd_row_color
::even_row_color
Comment 19 Srinivasa Ragavan 2008-05-13 11:57:53 UTC
<srag|meeting> proppy, Ill have a look at it
* You are now known as srag
<proppy> srag: thanks a lot, let me know which additionnal work is needed
<srag> proppy, the patch seems fine..
* mbarnes has quit (This place is dead anyway)
* vv|bbl has quit (Ping timeout: 600 seconds)
<srag> as I have commented above, you can restore the old code at e-cell-toggle etc and other places in  http://bugzilla.gnome.org/show_bug.cgi?id=527532#c4
<srag> proppy, pro'lly you can create a patch that reverts the old bits also and add it part of the patch
<srag> proppy, got it?
<proppy> srag: I'll take a look at it this afternoon
<srag> sure proppy 
<proppy> srag: thanks for the feedback
* srag updates the bug
Comment 20 Srinivasa Ragavan 2008-05-13 12:00:04 UTC
(evolution:10074): Gtk-WARNING **: gtkwidget.c:8544: widget class `ECanvas' has no property named `odd-row-color'

(evolution:10074): Gtk-WARNING **: gtkwidget.c:8544: widget class `ECanvas' has no property named `even-row-color'


I still get it in cal/addresbook/task/memo views.
Comment 21 Benjamin Berg 2008-08-08 21:22:43 UTC
Those widgets would also need to have the style properties registered. Not sure which those are, ECalendarTable, EMemoTable, ETask and EABView?

Though somehow I doubt any theme will set the style properties on all these widgets separately. But it will get rid of the warnings :-)
Comment 22 André Klapper 2009-01-12 19:49:26 UTC
Any news here?
Comment 23 Johannes Berg 2009-07-29 10:29:47 UTC
This bug persists in 2.27.4.

Additionally, in 2.27 each mail suddenly takes roughly 20% more space in the list than in 2.26, with the same font size. Even if it may look 'cleaner' because it means it's more spaced out, it's really not very usable to display much fewer emails in the list.
Comment 24 Matthew Barnes 2009-07-29 12:37:21 UTC
(In reply to comment #23)
> Additionally, in 2.27 each mail suddenly takes roughly 20% more space in the
> list than in 2.26, with the same font size. Even if it may look 'cleaner'
> because it means it's more spaced out, it's really not very usable to display
> much fewer emails in the list.

That was intentional.  See bug #565780. 

Comment 25 Johannes Berg 2009-07-29 12:38:54 UTC
(In reply to comment #24)
> (In reply to comment #23)
> > Additionally, in 2.27 each mail suddenly takes roughly 20% more space in the
> > list than in 2.26, with the same font size. Even if it may look 'cleaner'
> > because it means it's more spaced out, it's really not very usable to display
> > much fewer emails in the list.
> 
> That was intentional.  See bug #565780. 

Except it wasn't done right and isn't following the theme, see bug #590127.
Comment 26 André Klapper 2012-06-18 09:31:20 UTC
I wonder if this is still an issue - some fixes by cosimoc for this got committed for 3.4.
Comment 27 André Klapper 2015-02-15 16:44:27 UTC
Is this still a problem in 3.6 or later?
Comment 28 Pacho Ramos 2015-02-15 22:02:47 UTC
(In reply to André Klapper from comment #27)
> Is this still a problem in 3.6 or later?

With 3.12 I see this -> bug 737300

But I don't know if this could be related with that :/
Comment 29 Johannes Berg 2015-02-23 16:03:54 UTC
You're kidding, right? This bug is close to 7 years old, and evolution has gone to more UI changes than I can count, including moving from GTK2 to GTK3. Whatever theme I used at the time probably doesn't even exist any more.