Merge lp:~feng-kylin/unity8/AddIndicationForRunningApp into lp:unity8

Proposed by handsome_feng
Status: Merged
Approved by: Albert Astals Cid
Approved revision: 1854
Merged at revision: 1910
Proposed branch: lp:~feng-kylin/unity8/AddIndicationForRunningApp
Merge into: lp:unity8
Prerequisite: lp:~lukas-kde/unity8/closeAppsFromQuicklist
Diff against target: 100 lines (+38/-1)
5 files modified
plugins/Unity/Launcher/launchermodel.cpp (+0/-1)
qml/Launcher/LauncherDelegate.qml (+12/-0)
qml/Launcher/LauncherPanel.qml (+1/-0)
tests/plugins/Unity/Launcher/launchermodeltest.cpp (+16/-0)
tests/qmltests/Launcher/tst_Launcher.qml (+9/-0)
To merge this branch: bzr merge lp:~feng-kylin/unity8/AddIndicationForRunningApp
Reviewer Review Type Date Requested Status
Albert Astals Cid (community) Approve
Michael Zanetti Pending
Review via email: mp+266181@code.launchpad.net

This proposal supersedes a proposal from 2015-07-14.

Commit message

Added indication for running apps.

Description of the change

Add indication for the running apps.

* 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?

n/a

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

no

To post a comment you must log in.
Revision history for this message
Michael Zanetti (mzanetti) wrote : Posted in a previous version of this proposal

The code looks mostly ok (I've left 3 inline comments) and it works fine in make tryLauncher. However, testing it on the phone it doesn't work. No running icon painted :/

review: Needs Fixing
Revision history for this message
handsome_feng (feng-kylin) wrote : Posted in a previous version of this proposal

> The code looks mostly ok (I've left 3 inline comments) and it works fine in
> make tryLauncher. However, testing it on the phone it doesn't work. No running
> icon painted :/

hmm... unfortunately,there are some problems on my nexus4 now.
But i will test this as soon as possible when my phone is repaired.

Revision history for this message
handsome_feng (feng-kylin) wrote : Posted in a previous version of this proposal

Seems like a bug in unity8. I use the trunk code to test,run unity8 trough qtcreate repeatedly, it also got a "QML Image: Cannot open: file:///home/phablet/shell/qml/Launcher/graphics/focused_app_arrow.png" error, and the indication for focused app disappeared. hm... Keep investigating this.

Revision history for this message
Michael Zanetti (mzanetti) wrote : Posted in a previous version of this proposal

actually I'm wondering if we really need to add the running_app_icon. isn't it just the same as the "focused_app_arrow" one? I guess we could rotate in code, at least until design gives a new one.

Revision history for this message
handsome_feng (feng-kylin) wrote : Posted in a previous version of this proposal

> actually I'm wondering if we really need to add the running_app_icon. isn't it
> just the same as the "focused_app_arrow" one? I guess we could rotate in code,
> at least until design gives a new one.

hm, I agree with you, so rotating it in code first.

Revision history for this message
Michael Zanetti (mzanetti) wrote : Posted in a previous version of this proposal

Ok, I've tested this and it seems to work fine now.

Please rebase on https://code.launchpad.net/~lukas-kde/unity8/closeAppsFromQuicklist/+merge/265669 which is already in a silo and about to land.

We'll get this in with the next silo then.

Thanks a lot for your contributions!

Revision history for this message
Albert Astals Cid (aacid) wrote :

 * 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.
CI can't even compile at this stage, i've run relevant tests locally and seems fine.

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

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/Unity/Launcher/launchermodel.cpp'
--- plugins/Unity/Launcher/launchermodel.cpp 2015-07-29 05:57:57 +0000
+++ plugins/Unity/Launcher/launchermodel.cpp 2015-07-29 05:57:57 +0000
@@ -517,7 +517,6 @@
517 Q_EMIT dataChanged(index(itemIndex), index(itemIndex), {RoleRecent});517 Q_EMIT dataChanged(index(itemIndex), index(itemIndex), {RoleRecent});
518 }518 }
519 item->setRunning(true);519 item->setRunning(true);
520 // TODO Shall we paint some running/recent app highlight? If yes, do it here.
521 } else {520 } else {
522 LauncherItem *item = new LauncherItem(app->appId(), app->name(), app->icon().toString(), this);521 LauncherItem *item = new LauncherItem(app->appId(), app->name(), app->icon().toString(), this);
523 item->setRecent(true);522 item->setRecent(true);
524523
=== modified file 'qml/Launcher/LauncherDelegate.qml'
--- qml/Launcher/LauncherDelegate.qml 2015-07-29 05:57:57 +0000
+++ qml/Launcher/LauncherDelegate.qml 2015-07-29 05:57:57 +0000
@@ -24,6 +24,7 @@
24 property int count: 024 property int count: 0
25 property bool countVisible: false25 property bool countVisible: false
26 property int progress: -126 property int progress: -1
27 property bool itemRunning: false
27 property bool itemFocused: false28 property bool itemFocused: false
28 property real maxAngle: 029 property real maxAngle: 0
29 property bool inverted: false30 property bool inverted: false
@@ -217,6 +218,17 @@
217 }218 }
218219
219 Image {220 Image {
221 objectName: "runningHighlight"
222 anchors {
223 left: parent.left
224 verticalCenter: parent.verticalCenter
225 }
226 visible: root.itemRunning
227 rotation: 180
228 source: "graphics/focused_app_arrow.png"
229 }
230
231 Image {
220 objectName: "focusedHighlight"232 objectName: "focusedHighlight"
221 anchors {233 anchors {
222 right: parent.right234 right: parent.right
223235
=== modified file 'qml/Launcher/LauncherPanel.qml'
--- qml/Launcher/LauncherPanel.qml 2015-07-29 05:57:57 +0000
+++ qml/Launcher/LauncherPanel.qml 2015-07-29 05:57:57 +0000
@@ -196,6 +196,7 @@
196 count: model.count196 count: model.count
197 countVisible: model.countVisible197 countVisible: model.countVisible
198 progress: model.progress198 progress: model.progress
199 itemRunning: model.running
199 itemFocused: model.focused200 itemFocused: model.focused
200 inverted: root.inverted201 inverted: root.inverted
201 alerting: model.alerting202 alerting: model.alerting
202203
=== modified file 'tests/plugins/Unity/Launcher/launchermodeltest.cpp'
--- tests/plugins/Unity/Launcher/launchermodeltest.cpp 2015-07-29 05:57:57 +0000
+++ tests/plugins/Unity/Launcher/launchermodeltest.cpp 2015-07-29 05:57:57 +0000
@@ -299,6 +299,22 @@
299 QCOMPARE(launcherModel->rowCount(QModelIndex()), 1);299 QCOMPARE(launcherModel->rowCount(QModelIndex()), 1);
300 }300 }
301301
302 void testApplicationRunning() {
303 launcherModel->pin("abs-icon");
304 launcherModel->pin("no-icon");
305
306 QCOMPARE(launcherModel->get(0)->running(), true);
307 QCOMPARE(launcherModel->get(1)->running(), true);
308
309 appManager->stopApplication("abs-icon");
310 QCOMPARE(launcherModel->get(0)->running(), false);
311 QCOMPARE(launcherModel->get(1)->running(), true);
312
313 appManager->stopApplication("no-icon");
314 QCOMPARE(launcherModel->get(0)->running(), false);
315 QCOMPARE(launcherModel->get(1)->running(), false);
316 }
317
302 void testApplicationFocused() {318 void testApplicationFocused() {
303 // all apps unfocused at beginning...319 // all apps unfocused at beginning...
304 QCOMPARE(launcherModel->get(0)->focused(), false);320 QCOMPARE(launcherModel->get(0)->focused(), false);
305321
=== modified file 'tests/qmltests/Launcher/tst_Launcher.qml'
--- tests/qmltests/Launcher/tst_Launcher.qml 2015-07-29 05:57:57 +0000
+++ tests/qmltests/Launcher/tst_Launcher.qml 2015-07-29 05:57:57 +0000
@@ -382,6 +382,15 @@
382 }382 }
383 }383 }
384384
385 function test_runningHighlight() {
386 dragLauncherIntoView();
387 var launcherListView = findChild(launcher, "launcherListView");
388 for (var i = 0; i < launcherListView.count; ++i) {
389 var delegate = findChild(launcherListView, "launcherDelegate" + i)
390 compare(findChild(delegate, "runningHighlight").visible, LauncherModel.get(i).running)
391 }
392 }
393
385 function test_focusedHighlight() {394 function test_focusedHighlight() {
386 dragLauncherIntoView();395 dragLauncherIntoView();
387 var launcherListView = findChild(launcher, "launcherListView");396 var launcherListView = findChild(launcher, "launcherListView");

Subscribers

People subscribed via source and target branches