Merge lp:~lukas-kde/unity8/fixPanelMaxState into lp:unity8

[Lukáš Tinkl]

Fix panel drop shadow and click-to-focus of maximized apps

Fix recent desktop stage regressions:

1. the panel drop shadow is always displayed (should only be visible when there's a maximized app in background)
2. with a max'd app in background, clicking the panel no longer brings that app into foreground

* Are there any related MPs required for this MP to build/function as expected? Please list.

No

* Did you perform an exploratory manual test run of your code change and any related functionality?

Yes

* Did you make sure that your branch does not contain spurious tags?

Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

Yes

* If you changed the UI, has there been a design review?

Yes (this is to align with the intended design)

[lp:~lukas-kde/unity8/fixPanelMaxState]

2109. By Lukáš Tinkl on 2016-01-07

add a regression test for the drop shadow stuff

[Lukáš Tinkl (lukas-kde)]

Added a regression test for the drop shadow stuff

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2108
http://jenkins.qa.ubuntu.com/job/unity8-ci/7027/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5960
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/442/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1732
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/435
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1627
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1627
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/434
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/433
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4607
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5971
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5971/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26473
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-xenial-touch/194/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/440
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/440/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26474

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7027/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2109
http://jenkins.qa.ubuntu.com/job/unity8-ci/7029/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5967
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/444/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1734
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/437
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1629
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1629
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/436
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/435
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4612
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5978
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5978/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26483
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-xenial-touch/196/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/442
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/442/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26482

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7029/rebuild

review: Needs Fixing (continuous-integration)

[Michael Zanetti (mzanetti)]

It fixes the interaction with the panel, so that's good. Two remarks:

====

+ property int foregroundMaximizedAppIdIndex: -1

I know this has been there before, but please rename it to foregroundMaximizedAppId, as it has nothing to do with an index.

====

it fixes the issue, but there is something wrong with the dropshadow still. If there is a maximized app, but not in foreground, the panel's dropshadow is visible. However, comparing that with unity7, the dropshadow seems to be *always* behind any window. It effectively only drops a shadow onto the wallpaper.

review: Needs Fixing

[Lukáš Tinkl (lukas-kde)]

> It fixes the interaction with the panel, so that's good. Two remarks:
>
> ====
>
> + property int foregroundMaximizedAppIdIndex: -1
>
> I know this has been there before, but please rename it to
> foregroundMaximizedAppId, as it has nothing to do with an index.

Shouldn't it rather be foregroundMaximizedAppIndex? It is an index in fact, it has nothing to do with "appId" in fact.

>
> it fixes the issue, but there is something wrong with the dropshadow still. If
> there is a maximized app, but not in foreground, the panel's dropshadow is
> visible. However, comparing that with unity7, the dropshadow seems to be
> *always* behind any window. It effectively only drops a shadow onto the
> wallpaper.

I believe that's the correct behavior, the drop shadow should be visible only if there's a maximized app in the background (https://docs.google.com/presentation/d/1O9B6d4gm3X_u7460o_iTYORTWJ7Td7aN7jLvo2O9A4w/edit?hl=en#slide=id.ge5d9cfc49_0_17)

[Michael Zanetti (mzanetti)]

On 08.01.2016 12:37, Lukáš Tinkl wrote:
>> It fixes the interaction with the panel, so that's good. Two remarks:
>>
>> ====
>>
>> + property int foregroundMaximizedAppIdIndex: -1
>>
>> I know this has been there before, but please rename it to
>> foregroundMaximizedAppId, as it has nothing to do with an index.
>
> Shouldn't it rather be foregroundMaximizedAppIndex? It is an index in fact, it has nothing to do with "appId" in fact.

ok, if it is an index, fine. but the current mix of IdIndex is broken in
any case :)

>
>>
>> it fixes the issue, but there is something wrong with the dropshadow still. If
>> there is a maximized app, but not in foreground, the panel's dropshadow is
>> visible. However, comparing that with unity7, the dropshadow seems to be
>> *always* behind any window. It effectively only drops a shadow onto the
>> wallpaper.
>
> I believe that's the correct behavior, the drop shadow should be visible only if there's a maximized app in the background (https://docs.google.com/presentation/d/1O9B6d4gm3X_u7460o_iTYORTWJ7Td7aN7jLvo2O9A4w/edit?hl=en#slide=id.ge5d9cfc49_0_17)
>

ok then. wfm, just wanted to be sure this is what's wanted.

[lp:~lukas-kde/unity8/fixPanelMaxState]

2110. By Lukáš Tinkl on 2016-01-08

rename internal variable to foregroundMaximizedAppIndex

[Lukáš Tinkl (lukas-kde)]

>
>
> On 08.01.2016 12:37, Lukáš Tinkl wrote:
> >> It fixes the interaction with the panel, so that's good. Two remarks:
> >>
> >> ====
> >>
> >> + property int foregroundMaximizedAppIdIndex: -1
> >>
> >> I know this has been there before, but please rename it to
> >> foregroundMaximizedAppId, as it has nothing to do with an index.
> >
> > Shouldn't it rather be foregroundMaximizedAppIndex? It is an index in fact,
> it has nothing to do with "appId" in fact.
>
> ok, if it is an index, fine. but the current mix of IdIndex is broken in
> any case :)

Done

[Michael Zanetti (mzanetti)]

 * Did you perform an exploratory manual test run of the code change and any related functionality?

yes

 * Did CI run pass? If not, please explain why.

unrelated failures, no new regressions from this branch

 * Did you make sure that the branch does not contain spurious tags?

yes, stripped now

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2110
http://jenkins.qa.ubuntu.com/job/unity8-ci/7034/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5974
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/449/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1739
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/442
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1634
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1634
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/441
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/440
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4618
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5985
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5985/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26510
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-xenial-touch/199/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/447
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/447/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26511

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/7034/rebuild

review: Needs Fixing (continuous-integration)

=== modified file 'qml/Stages/DesktopStage.qml'
--- qml/Stages/DesktopStage.qml	2015-11-30 17:27:47 +0000
+++ qml/Stages/DesktopStage.qml	2016-01-08 13:29:28 +0000
@@ -124,16 +124,20 @@
         onFocusedAppDelegateChanged: updateForegroundMaximizedApp();
 
         property int foregroundMaximizedAppZ: -1
+        property int foregroundMaximizedAppIndex: -1 // for stuff like drop shadow and focusing maximized app by clicking panel
 
         function updateForegroundMaximizedApp() {
             var tmp = -1;
+            var tmpAppId = -1;
             for (var i = appRepeater.count - 1; i >= 0; i--) {
                 var item = appRepeater.itemAt(i);
                 if (item && item.visuallyMaximized) {
+                    tmpAppId = i;
                     tmp = Math.max(tmp, item.normalZ);
                 }
             }
             foregroundMaximizedAppZ = tmp;
+            foregroundMaximizedAppIndex = tmpAppId;
         }
 
         function indexOf(appId) {
@@ -176,8 +180,8 @@
         onMinimize: priv.focusedAppDelegate && priv.focusedAppDelegate.minimize();
         onMaximize: priv.focusedAppDelegate // don't restore minimized apps when double clicking the panel
                     && priv.focusedAppDelegate.restoreFromMaximized();
-        onFocusMaximizedApp: if (priv.foregroundMaximizedAppIdIndex != -1) {
-                                 ApplicationManager.focusApplication(appRepeater.itemAt(priv.foregroundMaximizedAppIdIndex).appId);
+        onFocusMaximizedApp: if (priv.foregroundMaximizedAppIndex != -1) {
+                                 ApplicationManager.focusApplication(appRepeater.itemAt(priv.foregroundMaximizedAppIndex).appId);
                              }
     }
 
@@ -206,7 +210,7 @@
     Binding {
         target: PanelState
         property: "dropShadow"
-        value: priv.focusedAppDelegate && !priv.focusedAppDelegate.maximized && priv.foregroundMaximizedAppIdIndex !== -1
+        value: priv.focusedAppDelegate && !priv.focusedAppDelegate.maximized && priv.foregroundMaximizedAppIndex !== -1
     }
 
     Component.onDestruction: {
 
=== modified file 'tests/qmltests/Stages/tst_DesktopStage.qml'
--- tests/qmltests/Stages/tst_DesktopStage.qml	2015-12-08 15:37:51 +0000
+++ tests/qmltests/Stages/tst_DesktopStage.qml	2016-01-08 13:29:28 +0000
@@ -25,6 +25,7 @@
 import ".." // For EdgeBarrierControls
 import "../../../qml/Stages"
 import "../../../qml/Components"
+import "../../../qml/Components/PanelState"
 
 Item {
     id: root
@@ -500,5 +501,33 @@
             tryCompare(dialerAppDelegate, "visible", true);
             tryCompare(facebookAppDelegate, "visible", false);
         }
+
+        function test_dropShadow() {
+            killAllRunningApps();
+
+            // verify the drop shadow is not visible initially
+            verify(PanelState.dropShadow == false);
+
+            // start an app, maximize it
+            var facebookApp = startApplication("facebook-webapp");
+            var facebookAppDelegate = findChild(desktopStage, "appDelegate_facebook-webapp");
+            facebookAppDelegate.maximize();
+
+            // verify the drop shadow is still not visible
+            verify(PanelState.dropShadow == false);
+
+            // start a foreground app, not maximized
+            var dialerApp = startApplication("dialer-app");
+            var dialerAppDelegate = findChild(desktopStage, "appDelegate_dialer-app");
+
+            // verify the drop shadow becomes visible
+            verify(PanelState.dropShadow == true);
+
+            // close the maximized app
+            ApplicationManager.stopApplication("facebook-webapp");
+
+            // verify the drop shadow is gone
+            verify(PanelState.dropShadow == false);
+        }
     }
 }