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

Proposed by handsome_feng on 2015-05-13
Status: Merged
Approved by: Michael Zanetti on 2015-05-29
Approved revision: 1781
Merged at revision: 1824
Proposed branch: lp:~feng-kylin/unity8/forbidClosingAppsDuringGesture
Merge into: lp:unity8
Diff against target: 64 lines (+32/-0)
3 files modified
qml/Stages/PhoneStage.qml (+7/-0)
qml/Stages/TabletStage.qml (+6/-0)
tests/qmltests/Stages/tst_PhoneStage.qml (+19/-0)
To merge this branch: bzr merge lp:~feng-kylin/unity8/forbidClosingAppsDuringGesture
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing on 2015-05-29
Michael Zanetti (community) 2015-05-13 Approve on 2015-05-29
Albert Astals Cid (community) Needs Information on 2015-05-21
Review via email: mp+258975@code.launchpad.net

Commit message

Forbid closing apps during the edge gesture.

Description of the change

Forbid closing apps during the edge gesture.

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

n/a

To post a comment you must log in.
Michael Zanetti (mzanetti) wrote :

Looks good. Please add a test for it.

review: Needs Fixing
1778. By handsome_feng on 2015-05-15

add a test for it

handsome_feng (feng-kylin) wrote :

> Looks good. Please add a test for it.

done

Michael Zanetti (mzanetti) wrote :

Thanks!

I noticed that the email address you've set up on Launchpad to sign the CLA is a different one that you've used to commit this patch.

Can you please either add this qq.com mail address to your Launchpad account or change the commits to use <email address hidden>

review: Needs Information
handsome_feng (feng-kylin) wrote :

> Thanks!
>
> I noticed that the email address you've set up on Launchpad to sign the CLA is
> a different one that you've used to commit this patch.
>
> Can you please either add this qq.com mail address to your Launchpad account
> or change the commits to use <email address hidden>

Done, I add the qq.com mail address to my Launchpad account.

Albert Astals Cid (aacid) wrote :

Triggered CI

Albert Astals Cid (aacid) wrote :

This looks nice, do you think you could try making it so that when you vertically swipe starting the horizontal swipe the animation does not break?

review: Needs Information
1779. By handsome_feng on 2015-05-26

Use a MouseArea to eat touch events while edge gesture, because the second touch point will cause the valuse of contendX of the spreadView unpredictable

1780. By handsome_feng on 2015-05-26

Use a MouseArea to eat touch events while edge gesture, because the second touch point will cause the value of contentX of the spreadView unpredictable

Daniel d'Andrada (dandrader) wrote :

"""
8+ //eat touch events during the right edge gesture
9+ MouseArea {
10+ objectName: "MouseArea"
11+ anchors.fill: parent
"""

Could use a more descriptive object name like "eventEaterArea"

1781. By handsome_feng on 2015-05-27

use a more descriptive object name

handsome_feng (feng-kylin) wrote :

> """
> 8+ //eat touch events during the right edge gesture
> 9+ MouseArea {
> 10+ objectName: "MouseArea"
> 11+ anchors.fill: parent
> """
>
> Could use a more descriptive object name like "eventEaterArea"

Yeah, done. thanks!

Michael Zanetti (mzanetti) wrote :

Thanks a lot for this! Looks and works great.

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

Need to be manually triggered on 3rd Party contribution. I've triggered it but I also verified manually that the tests are working.

 * 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
1=== modified file 'qml/Stages/PhoneStage.qml'
2--- qml/Stages/PhoneStage.qml 2015-05-18 23:03:25 +0000
3+++ qml/Stages/PhoneStage.qml 2015-05-27 01:21:49 +0000
4@@ -449,6 +449,13 @@
5 }
6 }
7
8+ //eat touch events during the right edge gesture
9+ MouseArea {
10+ objectName: "eventEaterArea"
11+ anchors.fill: parent
12+ enabled: spreadDragArea.dragging
13+ }
14+
15 DirectionalDragArea {
16 id: spreadDragArea
17 objectName: "spreadDragArea"
18
19=== modified file 'qml/Stages/TabletStage.qml'
20--- qml/Stages/TabletStage.qml 2015-05-18 23:03:25 +0000
21+++ qml/Stages/TabletStage.qml 2015-05-27 01:21:49 +0000
22@@ -596,6 +596,12 @@
23 }
24 }
25
26+ //eat touch events during the right edge gesture
27+ MouseArea {
28+ anchors.fill: parent
29+ enabled: spreadDragArea.dragging
30+ }
31+
32 DirectionalDragArea {
33 id: spreadDragArea
34 anchors { top: parent.top; right: parent.right; bottom: parent.bottom }
35
36=== modified file 'tests/qmltests/Stages/tst_PhoneStage.qml'
37--- tests/qmltests/Stages/tst_PhoneStage.qml 2015-04-10 21:16:37 +0000
38+++ tests/qmltests/Stages/tst_PhoneStage.qml 2015-05-27 01:21:49 +0000
39@@ -399,6 +399,25 @@
40 tryCompare(spreadView, "contentX", -spreadView.shift)
41 }
42
43+ function test_cantAccessPhoneStageWhileRightEdgeGesture() {
44+ var spreadView = findChild(phoneStage, "spreadView");
45+ var eventEaterArea = findChild(phoneStage, "eventEaterArea")
46+
47+ var startX = phoneStage.width - 2;
48+ var startY = phoneStage.height / 2;
49+ var endY = startY;
50+ var endX = phoneStage.width / 2;
51+
52+ touchFlick(phoneStage, startX, startY, endX, endY,
53+ true /* beginTouch */, false /* endTouch */, units.gu(10), 50);
54+
55+ compare(eventEaterArea.enabled, true);
56+
57+ touchRelease(phoneStage, endX, endY);
58+
59+ compare(eventEaterArea.enabled, false);
60+ }
61+
62 function test_leftEdge_data() {
63 return [
64 { tag: "normal", inSpread: false, leftEdgeDragWidth: units.gu(5), shouldMoveApp: true },

Subscribers

People subscribed via source and target branches