Created attachment 37699 [details]
a screenshot of the effect with a translucent panel

Created attachment 37700 [details] [review]
and of course, the (one-liner) patch itself

That's nice :-)
I don't think we'd want this for panels with no special background.

We could possibly add wnck_tasklist_set_shadow_type() (we already have
wnck_pager_set_shadow_type()).

Elijah, Mark, Havoc: what do you think?

It does help translucent panels look really cool, but I can only see it being
used in those cases.  In particular, I'm worried about the following things
(which affect both the translucent as well as opaque background cases):

1) The user loses the cue that there's something that can be activated (without
   the button drawing, it looks somewhat like a simple text area that at most
   could be used for cutting and pasting)
2) Assuming bug 160977 gets fixed (I'm still trying to see if I can hold out
   longer than Vincent or whether I'm actually going to have to work on that
   bug soon), it means that there won't be a clear separation between where one
   button ends and the other begins.  This reduces the area in which the user
   feels safe clicking.  Seems like a Fitts' Law issue, at least until the user
   learns that "everything at least 5 or so pixels to the left of any icon
   belongs to the taskbar item to the left of it".

These aren't major UI problems (e.g. the clock applet can be clicked on but most
users don't know it), but I'd still personally lean against it and say that we
should wait for gtk+ themes (and anything else necessary) to provide better
support for compositing.  I'm pretty sure there'll be several cases where that
is wanted once the compositing manager in Metacity is ready for wide release,
and it'd make sense to fix them all at once instead of piecemeal as this bug
wants to do.

Anyway, that's my $0.02 (though the screenshot does look really cool).  But
let's wait to hear back from Mark or Havoc...

Created attachment 37727 [details]
another screencap

here's another case that benifits from removing the frame for non-active
windows. (before and after shots)

My view is this would be fine if it ONLY happens when you enable crackrock
transparent mode for your panel. If it's some kind of separate setting, or if
you don't get the button bevels on a normal reasonable panel, then I don't think
this is a good idea.

.02

Havoc: that's exactly what I was thinking.

From the point of view of the applet, it can tell if the user has selected to
have a normal panel (system theme) or a solid colour or a pixmap.  It can not
tell what the selected level of alpha transparency is (it just gets sent to the
applet as a pixmap that has been pre-blended by the panel itself).

Would it be ok if I cook the patch to make it remove the bevels for "solid" and
"pixmap" while leaving them in place for system theme?  (motivation here is that
if the user wants a bright-red panel then they probably want the entire thing to
be bright red).

Ryan: the first thing to do is to make a patch that implements
wnck_tasklist_set_shadow_type() in libwnck. After that, a simple patch to use
this should be made for gnome-panel. There's already code to do this in
workspace-switcher.c that uses wnck_pager_set_shadow_type().

Nod.  I've started in this direction.

Assuming I don't get distracted I'll probably have something tonight or tomorrow.

Created attachment 37763 [details] [review]
the changes, as discussed

This patch adds the following method to wnck_tasklist:

void
wnck_tasklist_set_button_relief (WnckTasklist *tasklist, GtkReliefStyle relief)


The default state is that the button relief is GTK_RELIEF_NORMAL.

I'm fairly sure this works but I have a tendancy to cook bogus patches so
someone should probably give this a good look over before it's OK'd :)

Looks good to me.
Elijah, Havoc, Mark: does it look okay for you too?

The only problem I can find is spacing and location of parameters.  Instead of:

static void
wnck_tasklist_set_relief_callback (WnckWindow *win, WnckTask *task,
                                      WnckTasklist *tasklist)

it should be

static void
wnck_tasklist_set_relief_callback (WnckWindow *win, 
                                   WnckTask *task,
                                   WnckTasklist *tasklist)

to be consistent with the style used in the rest of the code.

Oops, I mean

static void
wnck_tasklist_set_relief_callback (WnckWindow   *win, 
                                   WnckTask     *task,
                                   WnckTasklist *tasklist)

I guess my mistake shows why the code isn't totally consistent with that giving
us a couple anomalies.  Still, it'd be nice to keep the number of anomalies to a
minimum...

With the modification, is this OK to commit?

Ok for me.

I believe this would break feature freeze, so we'd either have to commit after
branching or else get approval from the release team.

Considering that the patch in bug 168117 is for enabling this for solid color
backgrounds other than the default in addition to transparent windows (which is
unclear whether it would fall under "crackrock" or "normal and reasonable" as
far as Havoc's opinion is concerned), I'd personally prefer waiting until 2.11.
 Otherwise, we get no time for user feedback and getting to fix problems if it's
decided that this should only be done for transparent panels.  (If everyone else
decides to move forward I won't try to block anything; it's just my $0.02.)

I agree that this can be seen as a feature. Thinking about it, adding this a few
days before the hard code freeze might not be a good idea...

Ryan: you can ask the release team for approval, but I'd be surprised they'll
accept.

Elijah: if this patch gets the approval, one solution would be to only enable
this for pixmap backgrounds. Then we'll enable this for solid color backgrounds
in 2.11.

When this feature is accepted (and I hope it will, as it improves the panel look
a lot), it would make sense to apply the same logic to all "GtkButton" applets
(for instance, the "show desktop" applet)...

About Elijah's comments in comment #4 : 

1) The majority of "standard" applets are clickable without looking clickable
(e.g. the clock, weather, volume applets), so this change would actually make
the panel more consistent.

2) When you hover the tasklist buttons with your mouse, the button appears, just
like the clock applet. So there's no risk to click on the wrong spot.

I guess I'm trying to say I'd like to see that even with a regular panel
background ;-)

Can I commit these to HEAD?

Ryan: It's up to Vincent Untz; I'm going to let him handle this one, though I
may still throw out comments...  ;-)

Vincent Noel: Your arguments have a flaw, though--the patch doesn't remove the
button appearance from the active window, only the inactive ones.  Thus, there's
one that looks clickable and a bunch that don't.  If the relief were set to none
for all windows including the active one, then it would match better.  But then
we lose the indication of the active window (though, thinking about it, do we
really need it?)

I sort of like the idea of no relief for all windows.  If we did this it would
probably be nice to still have some sort of visual indicator such as bolding the
text.

Vincent (Untz), or Mark: pingety ping ;)  Since there's a related panel patch, I
want to hear from you before telling Ryan to commit.  Besides, I'd rather dump
any look-n-feel stuff onto you guys since you're responsible for the look-n-feel
of the panel anyway.  ;-)

I am going to mark the first of the two patches as obsolete, though, since only
the second is under consideration.

I don't think we should add API to allow the application that is using the
widget to set the relief for buttons. I vote for just making the default
GTK_RELIEF_NONE.

I've made a patch that just adds the one line call to libwnck/taskbar.c and
built packages of it to test with, and the people using it all love the lack of
button borders everywhere. Anything beyond that can be dealt with in the theme,
I think. Themes have the ability to do different things based on the widget's
tree structure, and I would much rather have my theme decide to be ugly or not,
rather than having the author of whatever app is using WnckTasklist decide
through API. :)

If we want to make this themeable then the approach is to gtk_widget_set_name()
on the widget allowing themes to special-case it.

I would be somewhat against no relief by default; it makes the buttons look less
pressable, it's basically eye candy over usability.

But it's not like I'll put up a fight either way.

The patch looks fine apart from the indentation the function declaration coding
style as Elijah said ...

(Just go with this and we can consider alternative ideas later)

Yeah, the code is fine, aside from the small style issues. However, do we want
to just add new API in this stage, when alternative ideas could mean that the
API is unnecessary?

It seems a bit foolish to add new API, if during the next release cycle we come
up with a better solution that deems that API deprecated. The patch seems like a
lot of code that could be done better without adding new API to me. And the
patch to the applet seems a bit extraneous with having different reliefs for
different states of tasks, especially when the applet also embeds the items in a
GtkHandleBox. I would agree that having RELIEF_NONE for the panel applet in its
current state would look weird. Especially given that the handle box widget
doesn't do PRELIGHT/ACTIVE widget states, and simply changes the X cursor after
you click on it, though it doesn't lend itself to telling the user that you can
click on it.

I would suggest holding off on committing a patch that adds API, until we do
consider the alternatives, so that we don't end up deprecating API within the
span of a single release cycle. Then again, the libwnck so version seems to keep
changing during the cycle, so I end up having to rebuild a lot of things anyway,
just to test some patches.

dobey: Sorry.  I didn't see your comment before I went ahead and commited this.

I do think this is a good idea for now, at least, to improve the situation for 2.12.

Closing.

Ehm: This bug is closed, but I don't consider the discussion to be over.
I do strongly agree with HP and Dobey even though it can be made to look good.
If this looks good in some themes, then the themes should just go ahead and change this. Having it dependend on whether or not there is a background image is plain weird.
Either leave it to the themes to not draw the NORMAL state of panel buttons or have a gconf/panel setting for this.

(In reply to comment #20)
> 1) The majority of "standard" applets are clickable without looking clickable
> (e.g. the clock, weather, volume applets), so this change would actually make
> the panel more consistent.

Actually I would like to see GTK_RELIEF_STYLE_NONE for all panel modes (i.e. including "use system colors" mode), since it:
 * looks better
 * is more consistent with e.g. the clock applet

Note that the "Applications, Locations, System" consists of clickable text without any visual hint for it as well, and I've never heard complaints like "oh, I didn't know I could actually click on the applications menu". To make it really consistent, the standard menu should consist of 3 buttons with GTK_RELIEF_STYLE_NONE as well, imho.

(Sorry for chiming sooooo late, but I was just wondering about this issue and found this bug report. Reopening this bug, but please don't flame me too hard. Ok? ;))

Closing this again -- I seriously doubt anybody still cares.