Unity 8

Merge lp:~diegosarmentero/unity8/disable-ui-on-actions into lp:unity8

disable-ui-on-actions
Merge into trunk

Proposed by Diego Sarmentero on 2013-10-09

Status:

Merged

Approved by:

Michał Sawicz on 2013-10-16

Approved revision:

412

Merged at revision:

468

Proposed branch:

lp:~diegosarmentero/unity8/disable-ui-on-actions

Merge into:

lp:unity8

Diff against target:

161 lines (+38/-3)

9 files modified

Dash/DashPreview.qml (+8/-0)
Dash/Generic/GenericPreview.qml (+4/-1)
Dash/GenericScopeView.qml (+7/-0)
plugins/Unity/scope.cpp (+2/-0)
plugins/Unity/scope.h (+1/-0)
tests/qmltests/Dash/Apps/tst_AppPreview.qml (+3/-2)
tests/qmltests/Dash/Movie/tst_MoviePreview.qml (+1/-0)
tests/qmltests/Dash/tst_GenericPreview.qml (+1/-0)
tests/qmltests/Dash/tst_GenericScopeView.qml (+11/-0)

To merge this branch:

bzr merge lp:~diegosarmentero/unity8/disable-ui-on-actions

Medium

Invalid

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2013-10-16
Michał Sawicz			Approve on 2013-10-16
Alejandro J. Cura (community)		2013-10-09	Approve on 2013-10-09
Review via email: mp+190145@code.launchpad.net

Commit message

- Disable Preview UI and show spinner until the scope responds.

Description of the change

Some actions might take some time to responds to the Dash, currently in this situations the user is able to keep pressing the same button again and again, with this branch the Preview UI is disable and a spinner is shown to let the user know the action is being processed, and when the scope responds (whatever is the response) the spinner is hidden.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-09:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1329/rebuild

review: Needs Fixing (continuous-integration)

lp:~diegosarmentero/unity8/disable-ui-on-actions updated on 2013-10-09

405. By Diego Sarmentero on 2013-10-09: close the spinner if we have a preview
406. By Diego Sarmentero on 2013-10-09: emit signal on preview ready

Revision history for this message

Alejandro J. Cura (alecu) wrote on 2013-10-09:

Just tested this branch, and it shows the spinner while installing and uninstalling, preventing the bug that happens if the user keeps clicking on the buttons.

Great work!

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-09:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1335/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-09:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1339/rebuild

review: Needs Fixing (continuous-integration)

lp:~diegosarmentero/unity8/disable-ui-on-actions updated on 2013-10-10

407. By Diego Sarmentero on 2013-10-10: tests fixed

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-10:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1350/rebuild

review: Needs Fixing (continuous-integration)

lp:~diegosarmentero/unity8/disable-ui-on-actions updated on 2013-10-10

408. By Diego Sarmentero on 2013-10-10: trigger jenkins
409. By Diego Sarmentero on 2013-10-10: removing testing code

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-10:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1355/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-10:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1356/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2013-10-11:

You still have

117 + console.log("\n");
118 + console.log(data);
119 + console.log("\n");

that you probably don't want in?

lp:~diegosarmentero/unity8/disable-ui-on-actions updated on 2013-10-11

410. By Diego Sarmentero on 2013-10-11: removing debug prints

Revision history for this message

Diego Sarmentero (diegosarmentero) wrote on 2013-10-11:

> You still have
>
> 117 + console.log("\n");
> 118 + console.log(data);
> 119 + console.log("\n");
>
> that you probably don't want in?

Removed

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-11:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1362/rebuild

review: Needs Fixing (continuous-integration)

lp:~diegosarmentero/unity8/disable-ui-on-actions updated on 2013-10-11

411. By Diego Sarmentero on 2013-10-11: [ David Callé ]
* Small logic change in preview descriptions line breaks. Multiple new
  lines are now replaced by multiple br tags.
[ Nick Dedekind ]
* Use indicator identifier for indicators-client list item label.
* Make sure overflow indicators are hidden and not the search label.
[ Mirco Müller ]
* Tweaks to notifications to improve spec-compliance. - updated
  notifications-qmltest to reflect the use of a variant-map for the
  hints - made text-fields as heigh as buttons - made the summary
  align horizontally to the icon - updated button-height and bubble-
  background.
[ Pawel Stolowski ]
* Handle category_order_changes signal from scopes (used in Home only)
  and reorder categories accordingly.
* Implementation of albumart image provider for audio content.
[ Michael Terry ]
* Hide the greeter when an app is focused, fixing snap decisions
  launching an app in the greeter (like receiving a call).
[ Michael Zanetti ]
* Launcher - remove support for pinning items in the backend After the
  latest design changes, recent apps is everything that is contained
  in the ApplicationManager. So no need for storing anything else than
  pinned apps in the config.
[ Michał Sawicz ]
* Remove the mir socket before starting unity8 during autopilot tests.
[ Ubuntu daily release ]
* Automatic snapshot from revision 420
[ Michal Hruby ]
* Add Scope::isActive property and corresponding tests. (LP: #1230352)
[ Daniel d'Andrada ]
* Move OSKController to the front as it will now also block input
  meant to the OSK That's in the unity-mir implementation. The
  SurfaceFlinger one is still an empty noop. Currently, when the OSK
  is up, both shell and OSK get user input, thus we need the
  OSKController to shield our shell components from them. (LP:
  #1236773)
[ Michał Sawicz ]
* Add Scope::isActive property and corresponding tests. (LP: #1230352)
* Force focus back on shell in case it loses it for some reason.
[ Ubuntu daily release ]
* Automatic snapshot from revision 409
[ Michał Sawicz ]
* update previews to match design. (LP: #1224555)
* Add an InputFilterArea in Notifications. (LP: #1233411, #1235215)
[ Ying-Chun Liu ]
* update previews to match design. (LP: #1224555)
[ Albert Astals ]
* Unrevert 376 by reverting r395 and a small fix to fix the cpu
  hogging issue . (LP: #1124567)
[ Michael Terry ]
* Add Showable.showNow() method and use it in Shell to immediately
  show greeter when we blank the screen rather than animating it. (LP:
  #1233564)
[ Michael Zanetti ]
* update previews to match design. (LP: #1224555)
[ Diego Sarmentero ]
* update previews to match design. (LP: #1224555)
[ Ubuntu daily release ]
* Automatic snapshot from revision 404

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-10-11:

The overlay shouldn't be a part of the preview, 'cause it can't be animated this way.

If we only care about the action not being triggered twice, please leave just the MouseArea in and show no UI for it. Otherwise rebase on lp:~unity-team/unity8/switching-previews to get an overlay on top of previews that can be used.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-11:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1369/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2013-10-16:

> The overlay shouldn't be a part of the preview, 'cause it can't be animated
> this way.
>
> If we only care about the action not being triggered twice, please leave just
> the MouseArea in and show no UI for it. Otherwise rebase on lp:~unity-
> team/unity8/switching-previews to get an overlay on top of previews that can
> be used.

Not entirely sure about this. would need to see how it looks on top of the switching-previews branch. Diego, can you please rebase it on that one and make sure it still looks good there?

lp:~diegosarmentero/unity8/disable-ui-on-actions updated on 2013-10-16

412. By Diego Sarmentero on 2013-10-16: removing rectangle, just leaving mouse area
413. By Diego Sarmentero on 2013-10-16: changing visible for enabled

Revision history for this message

Michał Sawicz (saviq) on 2013-10-16:

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-16:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1435/rebuild

review: Needs Fixing (continuous-integration)

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:

Diego Sarmentero

Mohammad Rakibul Hasan

Unity Team

1	=== modified file 'Dash/DashPreview.qml'
2	--- Dash/DashPreview.qml 2013-10-04 08:21:42 +0000
3	+++ Dash/DashPreview.qml 2013-10-16 15:49:53 +0000
4	@@ -22,6 +22,7 @@
5
6	property int keyboardSize: Qt.inputMethod.visible ? Qt.inputMethod.keyboardRectangle.height : 0
7	property var previewData
8	+ property alias showProcessingAction: waitingForActionMouseArea.enabled
9
10	property real previewWidthRatio: 0.5
11
12	@@ -196,4 +197,11 @@
13	}
14	sourceComponent: root.header
15	}
16	+
17	+ MouseArea {
18	+ id: waitingForActionMouseArea
19	+ objectName: "waitingForActionMouseArea"
20	+ anchors.fill: parent
21	+ enabled: false
22	+ }
23	}
24
25	=== modified file 'Dash/Generic/GenericPreview.qml'
26	--- Dash/Generic/GenericPreview.qml 2013-10-08 12:17:14 +0000
27	+++ Dash/Generic/GenericPreview.qml 2013-10-16 15:49:53 +0000
28	@@ -71,7 +71,10 @@
29	text: modelData.displayName
30	iconSource: modelData.iconHint
31	iconPosition: "right"
32	- onClicked: previewData.execute(modelData.id, { })
33	+ onClicked: {
34	+ previewData.execute(modelData.id, { })
35	+ genericPreview.showProcessingAction = true;
36	+ }
37	}
38	}
39	}
40
41	=== modified file 'Dash/GenericScopeView.qml'
42	--- Dash/GenericScopeView.qml 2013-10-11 10:45:36 +0000
43	+++ Dash/GenericScopeView.qml 2013-10-16 15:49:53 +0000
44	@@ -59,6 +59,7 @@
45	target: scopeView.scope
46	onShowDash: previewLoader.open = false;
47	onHideDash: previewLoader.open = false;
48	+ onActivated: previewLoader.closePreviewSpinner();
49	}
50
51	ScopeListView {
52	@@ -345,6 +346,12 @@
53	onLoaded: {
54	item.previewData = Qt.binding(function() { return previewLoader.previewData })
55	}
56	+
57	+ function closePreviewSpinner() {
58	+ if(item) {
59	+ item.showProcessingAction = false;
60	+ }
61	+ }
62	}
63
64	// TODO: Move as InverseMouseArea to DashPreview
65
66	=== modified file 'plugins/Unity/scope.cpp'
67	--- plugins/Unity/scope.cpp 2013-10-15 11:19:35 +0000
68	+++ plugins/Unity/scope.cpp 2013-10-16 15:49:53 +0000
69	@@ -228,6 +228,7 @@
70
71	void Scope::onActivated(unity::dash::LocalResult const& result, unity::dash::ScopeHandledType type, unity::glib::HintsMap const& hints)
72	{
73	+ Q_EMIT activated();
74	// note: we will not get called on SHOW_PREVIEW, instead UnityCore will signal preview_ready.
75	switch (type)
76	{
77	@@ -261,6 +262,7 @@
78
79	void Scope::onPreviewReady(unity::dash::LocalResult const& /* result */, unity::dash::Preview::Ptr const& preview)
80	{
81	+ Q_EMIT activated();
82	auto prv = Preview::newFromUnityPreview(preview);
83	// is this the best solution? QML may need to keep more than one preview instance around, so we can't own it.
84	// passing it by value is not possible.
85
86	=== modified file 'plugins/Unity/scope.h'
87	--- plugins/Unity/scope.h 2013-10-07 10:09:21 +0000
88	+++ plugins/Unity/scope.h 2013-10-16 15:49:53 +0000
89	@@ -117,6 +117,7 @@
90	void showDash();
91	void hideDash();
92	void gotoUri(const QString &uri);
93	+ void activated();
94
95	void activateApplication(const QString &desktop);
96
97
98	=== modified file 'tests/qmltests/Dash/Apps/tst_AppPreview.qml'
99	--- tests/qmltests/Dash/Apps/tst_AppPreview.qml 2013-10-07 08:51:36 +0000
100	+++ tests/qmltests/Dash/Apps/tst_AppPreview.qml 2013-10-16 15:49:53 +0000
101	@@ -137,11 +137,12 @@
102	for(var i = 0; i < buttons.count; i++) {
103	var button = findChild(appPreview, "button" + i);
104	mouseClick(button, 1, 1);
105	+ appPreview.showProcessingAction = false;
106	}
107
108	var actions = data.actions;
109	- for(var i = 0; i < actions.length; i++) {
110	- compare(root.calls[i][0], actions[i].id, "Id of action not found.");
111	+ for(var j = 0; j < actions.length; j++) {
112	+ compare(root.calls[j][0], actions[j].id, "Id of action not found.");
113	}
114	}
115
116
117	=== modified file 'tests/qmltests/Dash/Movie/tst_MoviePreview.qml'
118	--- tests/qmltests/Dash/Movie/tst_MoviePreview.qml 2013-10-04 08:19:54 +0000
119	+++ tests/qmltests/Dash/Movie/tst_MoviePreview.qml 2013-10-16 15:49:53 +0000
120	@@ -74,6 +74,7 @@
121	for(var i = 0; i < buttons.count; i++) {
122	var button = findChild(moviePreview, "button" + i);
123	mouseClick(button, 1, 1);
124	+ moviePreview.showProcessingAction = false;
125	}
126
127	var actions = get_actions_data();
128
129	=== modified file 'tests/qmltests/Dash/tst_GenericPreview.qml'
130	--- tests/qmltests/Dash/tst_GenericPreview.qml 2013-10-02 14:18:07 +0000
131	+++ tests/qmltests/Dash/tst_GenericPreview.qml 2013-10-16 15:49:53 +0000
132	@@ -76,6 +76,7 @@
133	for(var i = 0; i < buttons.count; i++) {
134	var button = findChild(genericPreview, "button" + i);
135	mouseClick(button, 1, 1);
136	+ genericPreview.showProcessingAction = false;
137	}
138
139	var actions = get_actions_data();
140
141	=== modified file 'tests/qmltests/Dash/tst_GenericScopeView.qml'
142	--- tests/qmltests/Dash/tst_GenericScopeView.qml 2013-09-26 11:48:17 +0000
143	+++ tests/qmltests/Dash/tst_GenericScopeView.qml 2013-10-16 15:49:53 +0000
144	@@ -66,6 +66,17 @@
145	scopes.get(0).hideDash();
146	tryCompare(genericScopeView, "previewShown", false);
147	}
148	+
149	+ function test_show_spinner() {
150	+ var previewLoader = findChild(genericScopeView, "previewLoader");
151	+ previewLoader.open = true;
152	+ previewLoader.source = "../../../Dash/Generic/GenericPreview.qml";
153	+ previewLoader.item.showProcessingAction = true;
154	+ var waitingForAction = findChild(genericScopeView, "waitingForActionMouseArea");
155	+ tryCompare(waitingForAction, "enabled", true);
156	+ previewLoader.closePreviewSpinner();
157	+ tryCompare(waitingForAction, "enabled", false);
158	+ }
159	}
160	}
161	}