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 635647 - StScrollView: "shadows" don't work on non-black backgrounds
StScrollView: "shadows" don't work on non-black backgrounds
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-11-23 21:54 UTC by Andreas Nilsson
Modified: 2010-12-04 11:17 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
current look (683.70 KB, image/png)
2010-11-23 21:54 UTC, Andreas Nilsson 		  Details
StScrollView: Remove shadow (1.16 KB, patch)
2010-11-30 19:27 UTC, drago01 		reviewed Details | Review
StScrollView: Remove default shadow style (1.15 KB, patch)
2010-11-30 21:00 UTC, drago01 		committed Details | Review

Description Andreas Nilsson 2010-11-23 21:54:09 UTC 
Created attachment 175136 [details]
current look

overview-relayout branch.
It would be neat if the dissolve effect in the app overview dissolved instead of going to black. That looks a bit odd right now with the hard edge.
Comment 1 Owen Taylor 2010-11-23 22:20:59 UTC 
We can easily do the fade-out by doing the rendering of the children to a fbo, but whether that will give decent performance impact is a bit unknown.
Comment 2 drago01 2010-11-30 19:27:15 UTC 
Created attachment 175572 [details] [review]
StScrollView: Remove shadow

It does look out of place for non black backgrounds, so it is better to
remove it until we have something which does not look to weird.
Comment 3 Florian Müllner 2010-11-30 19:42:24 UTC 
Review of attachment 175572 [details] [review]:

Hmmm, the commit message does not match the patch ;-)

If you want to leave the actual shadow code in place, instead of removing the CSS I'd change the constructor calls to not enable the shadows. And last and least a nitpick - "to weird" should be "too weird".
Comment 4 drago01 2010-11-30 19:56:29 UTC 
(In reply to comment #3)
> Review of attachment 175572 [details] [review]:
> 
> Hmmm, the commit message does not match the patch ;-)
> 
> If you want to leave the actual shadow code in place, instead of removing the
> CSS I'd change the constructor calls to not enable the shadows.

My idea was to leave the code in place and just not have a default one assigned.
So if one for whatever reason wants a shadow for another scrollview, it would work (by explicitly setting it). ex. altTab if it gets ported to StScrollView (assuming it doesn't suck like it did back when I added the scrolling to altTab).

> And last and
> least a nitpick - "to weird" should be "too weird".
...
Comment 5 Florian Müllner 2010-11-30 20:25:00 UTC 
(In reply to comment #4)
> My idea was to leave the code in place and just not have a default one
> assigned.

I don't have a strong opinion on that - the commit message should match what you are actually doing though. "StScrollView: Remove default shadow style" with the above explanation in the body works for me ...
Comment 6 drago01 2010-11-30 21:00:07 UTC 
Created attachment 175579 [details] [review]
StScrollView: Remove default shadow style

Remove the default shadow style but still allow StScrollView users to use a shadow by explicitly setting it.
Comment 7 drago01 2010-12-04 11:17:27 UTC 
Attachment 175579 [details] pushed as 670048e - StScrollView: Remove default shadow style