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

[handsome_feng]

Convert the MouseArea to MultiPointTouchArea to track multiple touch points.

 * 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

[Andrea Cimitan (cimi)]

Feels like we should add a small qml test for that!

review: Needs Fixing

[lp:~feng-kylin/unity8/cantClickOnDashWhileDraggingAnIcon]

1833. By handsome_feng on 2015-06-25

add a qml test

[handsome_feng (feng-kylin)]

To be honest, I think the function test_dragndrop_cancel() has already test this, but it is weird that the "tap(root)" can cancel the dragging rather than reach the dash.

[lp:~feng-kylin/unity8/cantClickOnDashWhileDraggingAnIcon]

1834. By handsome_feng on 2015-06-29

remove the qml test

[Michael Zanetti (mzanetti)]

Ok. I've investigated a bit here:

The reason why test_dragndrop_cancel fails to catch the bug seems to be that the test uses a mouse device to start the drag and drop operation. Then the tap(root) uses a touch. However, as there's no prior touch ongoing the MouseArea (only being capable of one finger) still catches it. In the real world the drag gesture is done with a touch too, so the tap is activated for the second finger and fails to trigger the MouseArea.

Also, turns out that converting the existing MouseArea to a MultiPointTouchArea is enough to fix the bug. No need to have a MouseArea and a MultiPointTouchArea.

For the test I think it would make sense to change the drag'n'drop to be performed with touch input too. Maybe we could add a _data() function that does the test once with touch and once with mouse.

What do you think?

[handsome_feng (feng-kylin)]

> Also, turns out that converting the existing MouseArea to a
> MultiPointTouchArea is enough to fix the bug. No need to have a MouseArea and
> a MultiPointTouchArea.

Simply converting the MouseArea to MultiPointTouchArea will lead to a problem that
when running unity8 with argument "-mousetouch", it needs click twice to dismiss
the Launcher. Maybe something wrong between MouseTouchAdaptor and MultiPointTouchArea?
:/

> For the test I think it would make sense to change the drag'n'drop to be
> performed with touch input too. Maybe we could add a _data() function that
> does the test once with touch and once with mouse.

yeah, I have done it.

[lp:~feng-kylin/unity8/cantClickOnDashWhileDraggingAnIcon]

1835. By handsome_feng on 2015-07-23

merge trunk

1836. By handsome_feng on 2015-07-23

update the test_dragndrop_cancel function

[Daniel d'Andrada (dandrader)]

You should update the commit message.

[Michael Zanetti (mzanetti)]

Thanks a lot!

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

not running on non-team-members. I've ran launcher tests manually, they're passing

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

yes

review: Approve

=== modified file 'qml/Launcher/Launcher.qml'
--- qml/Launcher/Launcher.qml	2015-04-15 20:21:11 +0000
+++ qml/Launcher/Launcher.qml	2015-07-23 07:31:40 +0000
@@ -177,7 +177,8 @@
         }
 
     }
-    MouseArea {
+
+    MultiPointTouchArea {
         id: closeMouseArea
         anchors {
             left: launcherDragArea.right

=== modified file 'tests/qmltests/Launcher/tst_Launcher.qml'
--- tests/qmltests/Launcher/tst_Launcher.qml	2015-04-24 09:53:37 +0000
+++ tests/qmltests/Launcher/tst_Launcher.qml	2015-07-23 07:31:40 +0000
@@ -459,7 +459,14 @@
             waitUntilLauncherDisappears();
         }
 
-        function test_dragndrop_cancel() {
+        function test_dragndrop_cancel_data() {
+            return [
+                {tag: "by mouse", mouse: true},
+                {tag: "by touch", mouse: false}
+            ]
+        }
+
+        function test_dragndrop_cancel(data) {
             dragLauncherIntoView();
             var draggedItem = findChild(launcher, "launcherDelegate4")
             var item0 = findChild(launcher, "launcherDelegate0")
@@ -468,22 +475,37 @@
             // Doing longpress
             var currentMouseX = draggedItem.width / 2
             var currentMouseY = draggedItem.height / 2
-            mousePress(draggedItem, currentMouseX, currentMouseY)
+
+            if(data.mouse) {
+                mousePress(draggedItem, currentMouseX, currentMouseY)
+            } else {
+                touchPress(draggedItem, currentMouseX, currentMouseY)
+            }
+
             // DraggedItem needs to hide and fakeDragItem become visible
             tryCompare(draggedItem, "itemOpacity", 0)
             tryCompare(fakeDragItem, "visible", true)
 
             // Dragging
             currentMouseX -= units.gu(20)
-            mouseMove(draggedItem, currentMouseX, currentMouseY)
+
+            if(data.mouse) {
+                mouseMove(draggedItem, currentMouseX, currentMouseY)
+            } else {
+                touchMove(draggedItem, currentMouseX, currentMouseY)
+            }
 
             // Make sure we're in the dragging state
             var dndArea = findChild(launcher, "dndArea");
             tryCompare(draggedItem, "dragging", true)
             tryCompare(dndArea, "draggedIndex", 4)
 
-            // Tap somewhere in the middle of the screen to close/hide the launcher
-            tap(root)
+            // Click/Tap somewhere in the middle of the screen to close/hide the launcher
+            if(data.mouse) {
+                mouseClick(root)
+            } else {
+                tap(root)
+            }
 
             // Make sure the dnd operation has been stopped
             tryCompare(draggedItem, "dragging", false)

=== modified file 'tests/utils/modules/Unity/Test/UnityTestCase.qml'
--- tests/utils/modules/Unity/Test/UnityTestCase.qml	2015-06-22 12:36:47 +0000
+++ tests/utils/modules/Unity/Test/UnityTestCase.qml	2015-07-23 07:31:40 +0000
@@ -351,6 +351,15 @@
         event.commit()
     }
 
+    function touchMove(item, tox, toy) {
+        var root = fetchRootItem(item)
+        var rootPoint = item.mapToItem(root, tox, toy)
+
+        var event = touchEvent(item);
+        event.move(0 /* touchId */, rootPoint.x, rootPoint.y);
+        event.commit();
+    }
+
     function touchPinch(item, x1Start, y1Start, x1End, y1End, x2Start, y2Start, x2End, y2End) {
         // Make sure the item is rendered
         waitForRendering(item);