Merge lp:~lukas-kde/unity8/unsnap-from-panel into lp:unity8
- unsnap-from-panel
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Michał Sawicz |
Approved revision: | 2851 |
Merged at revision: | 2877 |
Proposed branch: | lp:~lukas-kde/unity8/unsnap-from-panel |
Merge into: | lp:unity8 |
Prerequisite: | lp:~lukas-kde/unity8/fix-window-buttons-touch |
Diff against target: |
194 lines (+104/-11) 3 files modified
qml/ApplicationMenus/MenuBar.qml (+5/-0) qml/Panel/Panel.qml (+8/-2) tests/qmltests/tst_Shell.qml (+91/-9) |
To merge this branch: | bzr merge lp:~lukas-kde/unity8/unsnap-from-panel |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Unity8 CI Bot | continuous-integration | Approve | |
Michał Sawicz | Approve | ||
Albert Astals Cid (community) | Approve | ||
Review via email:
|
Commit message
Fix unsnapping a maximized window from the panel
Description of the change
Fix unsnapping a maximized window from the panel
Load the menubar only if the call hint isn't active
* Are there any related MPs required for this MP to build/function as expected? Please list.
Yes, prereq
* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes
* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A
* If you changed the UI, has there been a design review?
N/A
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2843
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2844
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
Two minor things in the tests:
+ mouseMove(panel, panel.width/2, panel.panelHeig
Maybe we can add a try compare here to actaully check the menu is shown here?
+ // start dialer, maximize it
You're not starting the dialer, s/dialer/app ?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
> Two minor things in the tests:
>
> + mouseMove(panel, panel.width/2, panel.panelHeig
> reveal the menus
> Maybe we can add a try compare here to actaully check the menu is shown here?
Done, removed the delay
> + // start dialer, maximize it
>
> You're not starting the dialer, s/dialer/app ?
Yup fixed. Originally it was dialer but then I realized dialer-app has some special treatment in shell, such as it wouldn't show the call hint in the panel.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
I can't open the menus anymore on maximized windows
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2845
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2846
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
> I can't open the menus anymore on maximized windows
Fixed \o/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2848
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
Can you add a test for the thing that broke and you just fixed (i.e. showing menus while maximized)
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
> Can you add a test for the thing that broke and you just fixed (i.e. showing
> menus while maximized)
Test added
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
test failed here http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2849
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes, works good
* Did CI run pass? If not, please explain why.
Let's wait before top approving
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2850
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2850
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
The newly added test failed on CI :/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
It's not the new one, just flaky run imo, will restart CI
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2850
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michał Sawicz (saviq) wrote : | # |
The touch version of test_dragPanelT
$ sh ./builddir/
[...]
QINFO : qmltestrunner:
FAIL! : qmltestrunner:
Actual (): maximized
Expected (): restored
Loc: [/home/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michał Sawicz (saviq) wrote : | # |
Same with just this branch, the touch version of the test fails.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
Fails here too (with xvfbtestShell works quite reliably but with testShell fails, seems to be dragging the wrong window)
- 2851. By Lukáš Tinkl
-
stabilize test
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
Yep, testfix fixes it :)
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2851
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Preview Diff
1 | === modified file 'qml/ApplicationMenus/MenuBar.qml' |
2 | --- qml/ApplicationMenus/MenuBar.qml 2017-03-08 09:53:21 +0000 |
3 | +++ qml/ApplicationMenus/MenuBar.qml 2017-03-21 10:44:49 +0000 |
4 | @@ -52,6 +52,10 @@ |
5 | d.dismissAll(); |
6 | } |
7 | |
8 | + function invokeMenu(mouseEvent) { |
9 | + mouseArea.onClicked(mouseEvent); |
10 | + } |
11 | + |
12 | GlobalShortcut { |
13 | shortcut: Qt.Key_Alt|Qt.AltModifier |
14 | active: enableKeyFilter |
15 | @@ -226,6 +230,7 @@ |
16 | } // Row |
17 | |
18 | MouseArea { |
19 | + id: mouseArea |
20 | anchors.fill: parent |
21 | hoverEnabled: d.currentItem |
22 | |
23 | |
24 | === modified file 'qml/Panel/Panel.qml' |
25 | --- qml/Panel/Panel.qml 2017-03-08 09:53:35 +0000 |
26 | +++ qml/Panel/Panel.qml 2017-03-21 10:44:49 +0000 |
27 | @@ -174,6 +174,10 @@ |
28 | // let it fall through to the window decoration of the maximized window behind, if any |
29 | mouse.accepted = false; |
30 | } |
31 | + var menubar = menuBarLoader.item; |
32 | + if (menubar) { |
33 | + menubar.invokeMenu(mouse); |
34 | + } |
35 | } |
36 | |
37 | Row { |
38 | @@ -200,16 +204,17 @@ |
39 | |
40 | Loader { |
41 | id: menuBarLoader |
42 | + objectName: "menuBarLoader" |
43 | height: parent.height |
44 | enabled: d.enablePointerMenu |
45 | opacity: d.showPointerMenu ? 1 : 0 |
46 | visible: opacity != 0 |
47 | Behavior on opacity { UbuntuNumberAnimation { duration: UbuntuAnimation.SnapDuration } } |
48 | - active: __applicationMenus.model |
49 | + active: __applicationMenus.model && !callHint.visible |
50 | |
51 | width: parent.width - windowControlButtons.width - units.gu(2) - __indicators.barWidth |
52 | |
53 | - property bool menusRequested: menuBarLoader.item ? menuBarLoader.item.showRequested : false |
54 | + readonly property bool menusRequested: menuBarLoader.item ? menuBarLoader.item.showRequested : false |
55 | |
56 | sourceComponent: MenuBar { |
57 | id: bar |
58 | @@ -231,6 +236,7 @@ |
59 | } |
60 | |
61 | onDoubleClicked: PanelState.restoreClicked() |
62 | + onPressed: mouse.accepted = false // let the parent mouse area handle this, so it can both unsnap window and show menu |
63 | } |
64 | } |
65 | } |
66 | |
67 | === modified file 'tests/qmltests/tst_Shell.qml' |
68 | --- tests/qmltests/tst_Shell.qml 2017-03-08 09:54:00 +0000 |
69 | +++ tests/qmltests/tst_Shell.qml 2017-03-21 10:44:49 +0000 |
70 | @@ -2513,27 +2513,44 @@ |
71 | tryCompare(app2Surface, "keymap", "fr"); |
72 | } |
73 | |
74 | - function test_dragPanelToRestoreMaximizedWindow() { |
75 | + function test_dragPanelToRestoreMaximizedWindow_data() { |
76 | + return [ |
77 | + { tag: "with mouse", mouse: true }, |
78 | + { tag: "with touch", mouse: false } |
79 | + ] |
80 | + } |
81 | + |
82 | + function test_dragPanelToRestoreMaximizedWindow(data) { |
83 | loadShell("desktop"); |
84 | shell.usageScenario = "desktop"; |
85 | waitForRendering(shell); |
86 | var panel = findChild(shell, "windowControlArea"); |
87 | verify(panel); |
88 | |
89 | - var appSurfaceId = topLevelSurfaceList.nextId; |
90 | - var app = ApplicationManager.startApplication("dialer-app") |
91 | - waitUntilAppWindowIsFullyLoaded(appSurfaceId); |
92 | - |
93 | // start dialer, maximize it |
94 | - var appContainer = findChild(shell, "appContainer"); |
95 | - var appDelegate = findChild(appContainer, "appDelegate_" + appSurfaceId); |
96 | + var appDelegate = startApplication("dialer-app"); |
97 | verify(appDelegate); |
98 | + |
99 | var maximizeButton = findChild(appDelegate, "maximizeWindowButton"); |
100 | - mouseClick(maximizeButton); |
101 | + if (data.mouse) { |
102 | + mouseClick(maximizeButton); |
103 | + } else { |
104 | + tap(maximizeButton); |
105 | + } |
106 | |
107 | + waitUntilTransitionsEnd(appDelegate); |
108 | tryCompare(appDelegate, "state", "maximized"); |
109 | |
110 | - mouseDrag(panel, panel.width/2, panel.height/2, 0, shell.height/3, Qt.LeftButton, Qt.NoModifier, 500); |
111 | + if (data.mouse) { |
112 | + mouseMove(panel, panel.width/2, panel.panelHeight/2); // to reveal the menus |
113 | + var menuBarLoader = findInvisibleChild(panel, "menuBarLoader"); |
114 | + verify(menuBarLoader); |
115 | + tryCompare(menuBarLoader.item, "visible", true); |
116 | + mouseDrag(panel, panel.width/2, panel.height/2, 0, shell.height/3, Qt.LeftButton, Qt.NoModifier, 500); |
117 | + } else { |
118 | + touchFlick(panel, panel.width/2, panel.panelHeight/2, panel.width/2, shell.height/3); |
119 | + } |
120 | + |
121 | tryCompare(appDelegate, "state", "restored"); |
122 | } |
123 | |
124 | @@ -2964,5 +2981,70 @@ |
125 | mouseDoubleClickSequence(panel, panel.width/2, PanelState.panelHeight/2, Qt.LeftButton, Qt.NoModifier, 300); |
126 | tryCompare(appDelegate, "state", "restored"); |
127 | } |
128 | + |
129 | + function test_noMenusWithActiveCall() { |
130 | + loadShell("desktop"); |
131 | + shell.usageScenario = "desktop"; |
132 | + waitForRendering(shell); |
133 | + swipeAwayGreeter(); |
134 | + |
135 | + // start music-app, maximize it |
136 | + var appDelegate = startApplication("music-app") |
137 | + verify(appDelegate); |
138 | + appDelegate.requestMaximize(); |
139 | + |
140 | + // move the mouse over panel to reveal the menus |
141 | + var panel = findChild(shell, "panel"); |
142 | + verify(panel); |
143 | + mouseMove(panel, panel.width/2, panel.panelHeight/2); // to reveal the menus |
144 | + var menuBarLoader = findInvisibleChild(panel, "menuBarLoader"); |
145 | + verify(menuBarLoader); |
146 | + tryCompare(menuBarLoader.item, "visible", true); |
147 | + |
148 | + // place a phone call |
149 | + callManager.foregroundCall = phoneCall; |
150 | + |
151 | + // menu bar should be hidden |
152 | + tryCompare(menuBarLoader, "active", false); |
153 | + tryCompare(menuBarLoader, "item", null); |
154 | + |
155 | + // remove call |
156 | + callManager.foregroundCall = null; |
157 | + |
158 | + // menu bar should be revealed |
159 | + tryCompare(menuBarLoader, "active", true); |
160 | + tryCompare(menuBarLoader.item, "visible", true); |
161 | + } |
162 | + |
163 | + function test_maximizedWindowAndMenuInPanel() { |
164 | + loadShell("desktop"); |
165 | + shell.usageScenario = "desktop"; |
166 | + waitForRendering(shell); |
167 | + swipeAwayGreeter(); |
168 | + |
169 | + // start music-app, maximize it |
170 | + var appDelegate = startApplication("music-app") |
171 | + verify(appDelegate); |
172 | + appDelegate.requestMaximize(); |
173 | + tryCompare(appDelegate, "state", "maximized"); |
174 | + |
175 | + // move the mouse over panel to reveal the menus |
176 | + var panel = findChild(shell, "panel"); |
177 | + verify(panel); |
178 | + mouseMove(panel, panel.width/2, panel.panelHeight/2); // to reveal the menus |
179 | + var menuBar = findChild(panel, "menuBar"); |
180 | + verify(menuBar); |
181 | + tryCompare(menuBar, "visible", true); |
182 | + |
183 | + // check that the menu popup appears |
184 | + var priv = findInvisibleChild(menuBar, "d"); |
185 | + var menuItem0 = findChild(menuBar, "menuBar-item0"); |
186 | + verify(menuItem0); |
187 | + mouseMove(menuItem0, menuItem0.width/2, menuItem0.height/2, 200); |
188 | + tryCompare(menuItem0, "visible", true); |
189 | + mouseClick(menuItem0); |
190 | + tryCompare(priv, "currentItem", menuItem0); |
191 | + tryCompare(priv.currentItem, "popupVisible", true); |
192 | + } |
193 | } |
194 | } |
PASSED: Continuous integration, rev:2842 /unity8- jenkins. ubuntu. com/job/ lp-unity8- ci/3337/ /unity8- jenkins. ubuntu. com/job/ build/4391 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= xenial+ overlay, testname= qmluitests. sh/2628 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= zesty,testname= qmluitests. sh/2628 /unity8- jenkins. ubuntu. com/job/ build-0- fetch/4419 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4252 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4252/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4252 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4252/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4252 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4252/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4252 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4252/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4252 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4252/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4252 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4252/ artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /unity8- jenkins. ubuntu. com/job/ lp-unity8- ci/3337/ rebuild
https:/