Unity 8

Merge lp:~lukas-kde/unity8/fix-window-title-vs-menu into lp:unity8

fix-window-title-vs-menu
Merge into trunk

Proposed by Lukáš Tinkl on 2017-03-15

Status:	Merged
Approved by:	Albert Astals Cid on 2017-04-05
Approved revision:	2878
Merged at revision:	2921
Proposed branch:	lp:~lukas-kde/unity8/fix-window-title-vs-menu
Merge into:	lp:unity8
Prerequisite:	lp:~lukas-kde/unity8/unsnap-from-panel
Diff against target:	176 lines (+17/-40) 6 files modified qml/Panel/Panel.qml (+8/-8) qml/Stage/DecoratedWindow.qml (+1/-0) qml/Stage/Stage.qml (+3/-2) qml/Stage/WindowDecoration.qml (+1/-0) tests/qmltests/Panel/tst_Panel.qml (+3/-25) tests/qmltests/Stage/tst_DesktopStage.qml (+1/-5)
To merge this branch:	bzr merge lp:~lukas-kde/unity8/fix-window-title-vs-menu
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Albert Astals Cid (community)		2017-03-15	Approve on 2017-04-05
Unity8 CI Bot	continuous-integration		Approve on 2017-04-04
Review via email: mp+319975@code.launchpad.net

This proposal supersedes a proposal from 2017-03-15.

Commit Message

Sanitize the logic for pointer vs. touch menus, fixes hiding the window title in staged mode, when hovering the panel with mouse and no menus available.

Also enable the binding for PanelState.decorationsVisible only in windowed mode; the panel otherwise still displays the window controls in staged mode for apps that have been started in windowed mode.

Description of the Change

Sanitize the logic for pointer vs. touch menus, fixes hiding the window title in staged mode, when hovering the panel with mouse and no menus available.

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-03-15:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3402/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-03-15:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3404/rebuild

review: Approve (continuous-integration)

Albert Astals Cid (aacid) wrote on 2017-03-17:

* 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.
Yes

review: Approve

Albert Astals Cid (aacid) wrote on 2017-03-24:

Text conflict in qml/Panel/Panel.qml
Text conflict in tests/qmltests/Panel/tst_Panel.qml
2 conflicts encountered.

Note: was already top approved.

review: Needs Fixing

Lukáš Tinkl (lukas-kde) wrote on 2017-03-24:

> Text conflict in qml/Panel/Panel.qml
> Text conflict in tests/qmltests/Panel/tst_Panel.qml
> 2 conflicts encountered.
>
> Note: was already top approved.

Done

Albert Astals Cid (aacid) on 2017-03-24:

review: Approve

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-03-24:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3523/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-03-24:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3536/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-03-24:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3541/rebuild

review: Approve (continuous-integration)

Albert Astals Cid (aacid) wrote on 2017-03-27:

Still good

review: Approve

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-04-03:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3641/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-04-03:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3645/rebuild

review: Approve (continuous-integration)

Albert Astals Cid (aacid) wrote on 2017-04-04:

The "decoratedWindow.windowControlButtonsVisible = true;" seems like a bug in the test where the test is clicking in a button that shouldn't be visible.

I'd prefer you fix the test instead of showing the buttons in a way the user could never do.

review: Needs Fixing

Lukáš Tinkl (lukas-kde) wrote on 2017-04-04:

> The "decoratedWindow.windowControlButtonsVisible = true;" seems like a bug in
> the test where the test is clicking in a button that shouldn't be visible.
>
> I'd prefer you fix the test instead of showing the buttons in a way the user
> could never do.

Done

Albert Astals Cid (aacid) wrote on 2017-04-04:

Good

review: Approve

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-04-04:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3659/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-04-04:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3663/rebuild

review: Approve (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-04-04:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3670/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-04-04:

FAILED: Continuous integration, rev:2878
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3672/
Executed test runs:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3672/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2017-04-04:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/3675/rebuild

review: Approve (continuous-integration)

Albert Astals Cid (aacid) wrote on 2017-04-05:

How hard is to get an autotest for that last commit you made?

review: Needs Fixing

Albert Astals Cid (aacid) wrote on 2017-04-05:

Since we're a bit pressed on time we'll settle for a test in a different MR

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Lukáš Tinkl

Mohammad Rakibul Hasan

Unity Team

1	=== modified file 'qml/Panel/Panel.qml'
2	--- qml/Panel/Panel.qml 2017-03-21 10:55:52 +0000
3	+++ qml/Panel/Panel.qml 2017-04-04 16:31:11 +0000
4	@@ -1,5 +1,5 @@
5	/*
6	- * Copyright (C) 2013-2016 Canonical, Ltd.
7	+ * Copyright (C) 2013-2017 Canonical, Ltd.
8	*
9	* This program is free software; you can redistribute it and/or modify
10	* it under the terms of the GNU General Public License as published by
11	@@ -85,15 +85,15 @@
12	property bool showWindowDecorationControls: (revealControls && PanelState.decorationsVisible) \|\|
13	PanelState.decorationsAlwaysVisible
14
15	- property bool showPointerMenu: revealControls &&
16	+ property bool showPointerMenu: revealControls && enablePointerMenu &&
17	(PanelState.decorationsVisible \|\| mode == "staged")
18
19	- property bool enablePointerMenu: revealControls &&
20	- applicationMenus.available &&
21	+ property bool enablePointerMenu: applicationMenus.available &&
22	applicationMenus.model
23
24	property bool showTouchMenu: !greeterShown &&
25	- !showPointerMenu
26	+ !showPointerMenu &&
27	+ !showWindowDecorationControls
28
29	property bool enableTouchMenus: showTouchMenu &&
30	applicationMenus.available &&
31	@@ -211,7 +211,7 @@
32	opacity: d.showPointerMenu ? 1 : 0
33	visible: opacity != 0
34	Behavior on opacity { UbuntuNumberAnimation { duration: UbuntuAnimation.SnapDuration } }
35	- active: __applicationMenus.model && !callHint.visible
36	+ active: d.showPointerMenu && !callHint.visible
37
38	width: parent.width - windowControlButtons.width - units.gu(2) - __indicators.barWidth
39
40	@@ -220,7 +220,7 @@
41	sourceComponent: MenuBar {
42	id: bar
43	objectName: "menuBar"
44	- anchors.left: parent.left
45	+ anchors.left: parent ? parent.left : undefined
46	anchors.margins: units.gu(1)
47	height: menuBarLoader.height
48	enableKeyFilter: valid && PanelState.decorationsVisible
49	@@ -339,7 +339,7 @@
50	maximumLineCount: 1
51	fontSize: "medium"
52	font.weight: Font.Medium
53	- color: Theme.palette.selected.backgroundText
54	+ color: theme.palette.selected.backgroundText
55	opacity: __applicationMenus.visible && !__applicationMenus.expanded ? 1 : 0
56	visible: opacity != 0
57	Behavior on opacity { NumberAnimation { duration: UbuntuAnimation.SnapDuration } }
58
59	=== modified file 'qml/Stage/DecoratedWindow.qml'
60	--- qml/Stage/DecoratedWindow.qml 2017-03-24 14:04:50 +0000
61	+++ qml/Stage/DecoratedWindow.qml 2017-04-04 16:31:11 +0000
62	@@ -38,6 +38,7 @@
63	property alias maximizeButtonShown: decoration.maximizeButtonShown
64	property alias interactive: applicationWindow.interactive
65	readonly property alias orientationChangesEnabled: applicationWindow.orientationChangesEnabled
66	+ property alias windowControlButtonsVisible: decoration.windowControlButtonsVisible
67
68	// Changing this will actually add/remove a decoration, meaning, requestedHeight will take the decoration into account.
69	property bool hasDecoration: true
70
71	=== modified file 'qml/Stage/Stage.qml'
72	--- qml/Stage/Stage.qml 2017-03-28 21:46:37 +0000
73	+++ qml/Stage/Stage.qml 2017-04-04 16:31:11 +0000
74	@@ -390,7 +390,7 @@
75	Binding {
76	target: PanelState
77	property: "decorationsVisible"
78	- value: priv.focusedAppDelegate !== null && priv.focusedAppDelegate.maximized // FIXME for Locally integrated menus
79	+ value: mode == "windowed" && priv.focusedAppDelegate && priv.focusedAppDelegate.maximized && !root.spreadShown
80	}
81
82	Binding {
83	@@ -1385,6 +1385,7 @@
84	requestedHeight: appContainer.height;
85	}
86	PropertyChanges { target: touchControls; enabled: true }
87	+ PropertyChanges { target: decoratedWindow; windowControlButtonsVisible: false }
88	},
89	State {
90	name: "fullscreen"; when: appDelegate.fullscreen && !appDelegate.minimized
91	@@ -1407,7 +1408,7 @@
92	}
93	PropertyChanges { target: touchControls; enabled: true }
94	PropertyChanges { target: resizeArea; enabled: true }
95	- PropertyChanges { target: decoratedWindow; shadowOpacity: .3}
96	+ PropertyChanges { target: decoratedWindow; shadowOpacity: .3; windowControlButtonsVisible: true}
97	},
98	State {
99	name: "restored";
100
101	=== modified file 'qml/Stage/WindowDecoration.qml'
102	--- qml/Stage/WindowDecoration.qml 2017-03-01 12:18:59 +0000
103	+++ qml/Stage/WindowDecoration.qml 2017-04-04 16:31:11 +0000
104	@@ -34,6 +34,7 @@
105	property var menu: undefined
106	property bool enableMenus: true
107	property bool windowMoving: false
108	+ property alias windowControlButtonsVisible: buttons.visible
109
110	readonly property real buttonsWidth: buttons.width + row.spacing
111
112
113	=== modified file 'tests/qmltests/Panel/tst_Panel.qml'
114	--- tests/qmltests/Panel/tst_Panel.qml 2017-03-17 13:44:30 +0000
115	+++ tests/qmltests/Panel/tst_Panel.qml 2017-04-04 16:31:11 +0000
116	@@ -779,40 +779,18 @@
117
118	var appTitle = findChild(panel, "panelTitle"); verify(appTitle);
119	var appMenuRow = findChild(panel.applicationMenus, "panelRow"); verify(appMenuRow);
120	- var appMenuBar = findChild(panel, "menuBar"); verify(appMenuBar);
121	+ var menuBarLoader = findChild(panel, "menuBarLoader"); verify(menuBarLoader);
122
123	tryCompare(appTitle, "visible", true, undefined, "App title should be visible");
124	- tryCompare(appMenuBar, "visible", false, undefined, "App menu bar should not be visible");
125	+ tryCompare(menuBarLoader, "visible", false, undefined, "App menu bar should not be visible");
126
127	mouseMove(panel, panel.width/2, panel.panelHeight);
128
129	+ var appMenuBar = findChild(panel, "menuBar"); verify(appMenuBar);
130	tryCompare(appTitle, "visible", false, undefined, "App title should not be visible on mouse hover");
131	tryCompare(appMenuBar, "visible", true, undefined, "App menu bar should be visible on mouse hover");
132	}
133
134	- function test_windowedApplicationMenuShowOnMouseHoverWhenDecorationsShown() {
135	- PanelState.title = "Fake Title";
136	- panel.mode = "windowed";
137	- mouseEmulation.checked = false;
138	-
139	- var appTitle = findChild(panel, "panelTitle"); verify(appTitle);
140	- var appMenuRow = findChild(panel.applicationMenus, "panelRow"); verify(appMenuRow);
141	- var appMenuBar = findChild(panel, "menuBar"); verify(appMenuBar);
142	-
143	- tryCompare(appTitle, "visible", true, undefined, "App title should be visible");
144	- tryCompare(appMenuBar, "visible", false, undefined, "App menu bar should not be visible");
145	-
146	- mouseMove(panel, panel.width/2, panel.panelHeight);
147	-
148	- tryCompare(appTitle, "visible", true, undefined, "App title should still be visible on mouse hover when panel decorations are not visible");
149	- tryCompare(appMenuBar, "visible", false, undefined, "App menu bar should be visible on mouse hover when panel decorations are not visible");
150	-
151	- PanelState.decorationsVisible = true;
152	-
153	- tryCompare(appTitle, "visible", false, undefined, "App title should still be visible on mouse hover when panel decorations are visible");
154	- tryCompare(appMenuBar, "visible", true, undefined, "App menu bar should be visible on mouse hover when panel decorations not visible");
155	- }
156	-
157	function test_keyboardNavigation_data() {
158	return [
159	{tag: "tab to start", doTab: false},
160
161	=== modified file 'tests/qmltests/Stage/tst_DesktopStage.qml'
162	--- tests/qmltests/Stage/tst_DesktopStage.qml 2017-03-24 14:04:50 +0000
163	+++ tests/qmltests/Stage/tst_DesktopStage.qml 2017-04-04 16:31:11 +0000
164	@@ -870,11 +870,7 @@
165	maximizeAppDelegate(appDelegate);
166
167	// Close the window and restart the application
168	- var closeButton = findChild(appDelegate, "closeWindowButton");
169	- appDelegate = null;
170	- verify(closeButton);
171	- mouseClick(closeButton);
172	- closeButton = null;
173	+ appDelegate.close();
174	tryCompare(topSurfaceList, "count", originalWindowCount);
175	wait(100); // plus some spare room
176	appDelegate = startApplication("dialer-app");