Merge lp:~gcollura/content-hub/fix-1384490 into lp:content-hub

[Giulio Collura]

Fix bug #1384490 by setting a different background color for ContentPeerPicker.

I changed the default background of the ContentPeerPicker to Theme.palette.normal.background. If the the app has a dark background, ContentPeerPicker will have a proper background color (it's actually purple, point me to the correct color to choose). Ideally, the developer should be able to choose which background the component should have, maybe we could expose a property like "backgroundColor:".
Let me know what you think about this idea.

Thanks in advance,
Giulio

[Michael Sheldon (michael-sheldon)]

I think it's best if we pick up the colour from the Theme palette as this is how the developer should be customising colour throughout the app, however I think we need to do some extra things to match the way the UITK uses these properties, I'll have a bit of look into this and get back to you on what I find.

Aside from that it looks like you've got a couple of typos for Qt.rgba listed as Qt.rbga (g and b switched around), so dark layouts still get the white background. However, with these corrected the apps box for light backgrounds becomes gray, so we might need to think about that a bit more.

review: Needs Fixing

[Giulio Collura (gcollura)]

Thanks for the review!

> I'll have a bit of look into this and get back to you on what I
> find.

Ok, thanks a lot :)

> Aside from that it looks like you've got a couple of typos for Qt.rgba listed
> as Qt.rbga (g and b switched around),

Ops, sorry for the typo. (fixed it)

> so dark layouts still get the white
> background.

The alpha layer is set to 0.1, so it's just a bit lighter, but still a dark grey (at least in my tests)

> However, with these corrected the apps box for light backgrounds
> becomes gray, so we might need to think about that a bit more.

I thought it would have been better to remove that background color of the Rectangle and leave everything of the same color, tell me what you think :)

Thanks again for the review

Giulio

[lp:~gcollura/content-hub/fix-1384490]

161. By Giulio Collura on 2014-11-04

fix rgba typo. make rectangle color transparent

[Michael Sheldon (michael-sheldon)]

At this stage it'd be best to keep the design the same for use cases that already work in apps (i.e. with light coloured backgrounds). We can use the same technique the UITK uses to determine whether something has a light background or not via ColorUtils, so if it has a light background set a white apps area, otherwise leave it transparent for darker backgrounds, for example:

color: ColorUtils.luminance(Theme.palette.normal.background) >= 0.85 ? "#FFFFFF" : Qt.rgba(0, 0, 0, 0)

Should do the trick.

I still need to figure out what the UITK does with the background colour though, as the colour that gets set as Theme.palette.normal.background is modified in some way from the colour the user actually sets as the MainView's backgroundColor. It might end up being that we'll have to expose explicitly backgroundColor properties on the Peer Picker, but I want to make sure we can't do it automatically first.

[lp:~gcollura/content-hub/fix-1384490]

162. By Giulio Collura on 2014-11-07

use ColorUtils.luminance to determine background color

[Giulio Collura (gcollura)]

I updated my branch using your suggestion.

[Michael Sheldon (michael-sheldon)]

Sorry! We've decided to take another approach to fixing this bug, since it isn't really possible for us to mimic the application's colours correctly without the developer having to explicitly set headerColor/backgroundColor/footerColor properties on the peer picker and then us copying gradient stops from the UITK (which could then easily get out of sync if this is changed in the UITK), we've decided to have the peer picker keep the same light appearance across all applications and just force the font colour to be correct. I'll see about preparing a branch with the new approach this week, since it might require a bit of fiddly work for the 1.0 implementation (since only UITK 1.1 allows explicitly setting the header colour).

[Michael Hall (mhall119)]

Not having a proper way to change the ContentPeerPicker colors makes it less ideal for apps that define their own colors. Which means those developers will either avoid using it and roll their own, or do some nasty hacks to force it to change color (I'm currently doing the later).

Unmerged revisions

162. By Giulio Collura on 2014-11-07

use ColorUtils.luminance to determine background color

161. By Giulio Collura on 2014-11-04

fix rgba typo. make rectangle color transparent

160. By Giulio Collura on 2014-10-30

Fix bug #1384490

=== modified file 'import/Ubuntu/Content/ContentPeerPicker10.qml'
--- import/Ubuntu/Content/ContentPeerPicker10.qml	2014-10-10 15:28:39 +0000
+++ import/Ubuntu/Content/ContentPeerPicker10.qml	2014-11-07 13:37:12 +0000
@@ -40,7 +40,7 @@
 
     Rectangle {
         anchors.fill: parent
-        color: Theme.palette.normal.overlay
+        color: Theme.palette.normal.background
     }
 
     Header {
@@ -160,7 +160,7 @@
 
     Rectangle {
         id: apps
-        color: "#FFFFFF"
+        color: ColorUtils.luminance(Theme.palette.normal.background) >= 0.85 ? "#FFFFFF" : Qt.rgba(0, 0, 0, 0)
         clip: true
         anchors {
             left: parent.left

=== modified file 'import/Ubuntu/Content/ContentPeerPicker11.qml'
--- import/Ubuntu/Content/ContentPeerPicker11.qml	2014-10-10 15:28:39 +0000
+++ import/Ubuntu/Content/ContentPeerPicker11.qml	2014-11-07 13:37:12 +0000
@@ -41,7 +41,7 @@
 
     Rectangle {
         anchors.fill: parent
-        color: Theme.palette.normal.overlay
+        color: Theme.palette.normal.background
     }
 
     Header {
@@ -177,7 +177,7 @@
 
     Rectangle {
         id: apps
-        color: "#FFFFFF"
+        color: ColorUtils.luminance(Theme.palette.normal.background) >= 0.85 ? "#FFFFFF" : Qt.rgba(0, 0, 0, 0)
         clip: true
         anchors {
             left: parent.left