Merge lp:~zsombi/ubuntu-ui-toolkit/70-cpos into lp:ubuntu-ui-toolkit/staging

[Zsombor Egri]

CPOs

[Tim Peeters (tpeeters)]

 + def _click_on_panel_action(self, swipe_direction, action_object):
101 + self._swipe_content(swipe_direction)
102 + try:
103 + button_name = 'actionbutton_' + action_object
104 + action_button = self.select_single(objectName=button_name)
105 + except dbus.StateNotFound:
106 + raise _common.ToolkitException(
107 + 'The requested action not found on leading side')

the exception text is incorrect, swipe direction may be 'left', then you are looking for an action on trailing side.

review: Needs Fixing

[Tim Peeters (tpeeters)]

swipe_direction is ambiguous. Is it swipe TO this direction or swipe FROM? Perhaps rename the variable to "swipe_to_direction"?

[Tim Peeters (tpeeters)]

115 + def click_leading_action(self, action_objectName):
120 + def click_trailing_action(self, action_objectName):

Maybe trigger_{leading,trailing}_action(..) is better here. You click on a button to trigger the action.

[Tim Peeters (tpeeters)]

114 + @autopilot_logging.log_action(logger.info)
115 + def click_leading_action(self, action_objectName):
116 + """Swipe the item in from left to right to open leading actions."""
117 + self._click_on_panel_action('right', action_objectName)
118 +
119 + @autopilot_logging.log_action(logger.info)
120 + def click_trailing_action(self, action_objectName):
121 + """Swipe the item in from right to left to open trailing actions."""
122 + self._click_on_panel_action('left', action_objectName)

You do more than just swipe in these functions.
Add something similar to this to the descriptions: "and click on the button representing the requested action".

[Tim Peeters (tpeeters)]

244 + Label {
245 + id: triggeredAction
246 + height: paintedHeight
247 + objectName: "triggeredAction"
248 + }

It is better to anchor this somewhere. Who knows where it may end up now?

[Tim Peeters (tpeeters)]

237 +MainView {

I propose to use a MainView with a Page inside (then there is a header), or simply to use an Item in which you test the ListView with ListItems.

[Tim Peeters (tpeeters)]

244 + Label {
245 + id: triggeredAction
246 + height: paintedHeight
247 + objectName: "triggeredAction"
248 + }

I prefer an initial label of "no action triggered", and when an action is triggered label.text = objectName + " action triggered". Adds a little bit of clarity when watching the videos of a failed test.

[Tim Peeters (tpeeters)]

We should also test what happens when you self.test_listitem.click_leading_action('this_action_does_not_exist')

[Zsombor Egri (zsombi)]

> 244 + Label {
> 245 + id: triggeredAction
> 246 + height: paintedHeight
> 247 + objectName: "triggeredAction"
> 248 + }
>
> It is better to anchor this somewhere. Who knows where it may end up now?

Any item unanchored is positioned to (0,0), you should know that. Everyone should know that.

[Zsombor Egri (zsombi)]

> + def _click_on_panel_action(self, swipe_direction, action_object):
> 101 + self._swipe_content(swipe_direction)
> 102 + try:
> 103 + button_name = 'actionbutton_' + action_object
> 104 + action_button =
> self.select_single(objectName=button_name)
> 105 + except dbus.StateNotFound:
> 106 + raise _common.ToolkitException(
> 107 + 'The requested action not found on leading side')
>
>
> the exception text is incorrect, swipe direction may be 'left', then you are
> looking for an action on trailing side.

Aah, right, I forgot to adjust that message when moved the code in one function. Fix in the next revision.

[Zsombor Egri (zsombi)]

> swipe_direction is ambiguous. Is it swipe TO this direction or swipe FROM?
> Perhaps rename the variable to "swipe_to_direction"?

Uhm... I think this naming is widely used and means always the destination direction. You specify "to" and "from" only if you have both parameters in a function.

[Zsombor Egri (zsombi)]

> 115 + def click_leading_action(self, action_objectName):
> 120 + def click_trailing_action(self, action_objectName):
>
> Maybe trigger_{leading,trailing}_action(..) is better here. You click on a
> button to trigger the action.

Ack, changed.

[Zsombor Egri (zsombi)]

> > swipe_direction is ambiguous. Is it swipe TO this direction or swipe FROM?
> > Perhaps rename the variable to "swipe_to_direction"?
>
> Uhm... I think this naming is widely used and means always the destination
> direction. You specify "to" and "from" only if you have both parameters in a
> function.

Actually I reformatted that part so now it uses panel_item (leading/trailing) to identify which panel to be swiped in.

[Zsombor Egri (zsombi)]

> 237 +MainView {
>
> I propose to use a MainView with a Page inside (then there is a header), or
> simply to use an Item in which you test the ListView with ListItems.

Ok, I'll do that. But then having a label in a view and under it the list view makes not much sense, we can use the app title to store the action triggered.

[Zsombor Egri (zsombi)]

> We should also test what happens when you
> self.test_listitem.click_leading_action('this_action_does_not_exist')

Done.

[Tim Peeters (tpeeters)]

> > 244 + Label {
> > 245 + id: triggeredAction
> > 246 + height: paintedHeight
> > 247 + objectName: "triggeredAction"
> > 248 + }
> >
> > It is better to anchor this somewhere. Who knows where it may end up now?
>
> Any item unanchored is positioned to (0,0), you should know that. Everyone
> should know that.

Do you have a reference for that?

[Tim Peeters (tpeeters)]

should we rename _click_on_panel_action() to _trigger_action() or _click_on_panel_button() ?

[Tim Peeters (tpeeters)]

good

review: Approve

[Leo Arias (elopio)]

Looks pretty good. Just, please change the last two tests to something like this:

def test_trigger_nonexistent_leading_action(self):
    error = self.assertRaises(
       ubuntuuitoolkit.ToolkitException,
       self.test_listitem.trigger_leading_action,
       'this_action_does_not_exist')
    self.assertEqual(
        str(error),
       'The requested action not found on leading side')

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-autolanding/751/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-autolanding/58/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-autolanding/58/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-autolanding/58/console

review: Needs Fixing (continuous-integration)

[Tim Peeters (tpeeters)]

thanks

review: Approve

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'modules/Ubuntu/Components/Themes/Ambiance/ListItemPanel.qml'
--- modules/Ubuntu/Components/Themes/Ambiance/ListItemPanel.qml	2014-12-01 14:50:13 +0000
+++ modules/Ubuntu/Components/Themes/Ambiance/ListItemPanel.qml	2014-12-08 14:37:55 +0000
@@ -157,6 +157,7 @@
                         // use action's objectName to identify the visualized action
                         if (item && item.objectName === "") {
                             item.objectName = modelData.objectName;
+                            actionButton.objectName = "actionbutton_" + modelData.objectName
                         }
                     }
                 }

=== modified file 'tests/autopilot/ubuntuuitoolkit/__init__.py'
--- tests/autopilot/ubuntuuitoolkit/__init__.py	2014-08-20 08:52:11 +0000
+++ tests/autopilot/ubuntuuitoolkit/__init__.py	2014-12-08 14:37:55 +0000
@@ -28,6 +28,7 @@
     'get_pointing_device',
     'Header',
     'listitems',
+    'UCListItem',
     'MainView',
     'OptionSelector',
     'pickers',
@@ -63,6 +64,7 @@
     get_pointing_device,
     Header,
     listitems,
+    UCListItem,
     MainView,
     OptionSelector,
     pickers,

=== modified file 'tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/__init__.py'
--- tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/__init__.py	2014-08-03 02:50:45 +0000
+++ tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/__init__.py	2014-12-08 14:37:55 +0000
@@ -25,6 +25,7 @@
     'get_pointing_device',
     'Header',
     'listitems',
+    'UCListItem',
     'MainView',
     'OptionSelector',
     'pickers',
@@ -56,6 +57,9 @@
     Header,
 )
 from ubuntuuitoolkit._custom_proxy_objects import listitems
+from ubuntuuitoolkit._custom_proxy_objects._uclistitem import (
+    UCListItem
+)
 from ubuntuuitoolkit._custom_proxy_objects._mainview import MainView
 from ubuntuuitoolkit._custom_proxy_objects._optionselector import (
     OptionSelector

=== added file 'tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/_uclistitem.py'
--- tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/_uclistitem.py	1970-01-01 00:00:00 +0000
+++ tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/_uclistitem.py	2014-12-08 14:37:55 +0000
@@ -0,0 +1,87 @@
+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
+#
+# Copyright (C) 2012, 2013, 2014 Canonical Ltd.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; version 3.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+import logging
+
+from autopilot import logging as autopilot_logging
+from ubuntuuitoolkit._custom_proxy_objects import _common
+
+
+logger = logging.getLogger(__name__)
+
+
+class UCListItem(_common.UbuntuUIToolkitCustomProxyObjectBase):
+    """Base class to emulate swipe for leading and trailing actions."""
+
+    @autopilot_logging.log_action(logger.info)
+    def _swipe_in_panel(self, panel_item):
+        """ Swipe in panel (leading/trailing)"""
+        x, y, width, height = self.globalRect
+        if panel_item == 'leading':
+            start_x = x + (width * 0.2)
+            stop_x = x + (width * 0.8)
+        elif panel_item == 'trailing':
+            start_x = x + (width * 0.8)
+            stop_x = x + (width * 0.2)
+        else:
+            raise _common.ToolkitException(
+                'No {0} panel found in a ListItem'.format(panel_item))
+        start_y = stop_y = y + (height // 2)
+        self.pointing_device.drag(start_x, start_y, stop_x, stop_y)
+
+    def _click_on_panel_action(self, panel_item, action_object, wait_function):
+        self._swipe_in_panel(panel_item)
+        try:
+            button_name = 'actionbutton_' + action_object
+            action_button = self.select_single(objectName=button_name)
+        except:
+            raise _common.ToolkitException(
+                'The requested action not found on {0} side'.
+                format(panel_item))
+
+        self.pointing_device.click_object(action_button)
+        if wait_function is None:
+            # wait for the animation to finish
+            contentItem = self.select_single(objectName='ListItemHolder')
+            contentItem.x.wait_for(0)
+        else:
+            wait_function()
+
+    @autopilot_logging.log_action(logger.info)
+    def trigger_leading_action(self, action_objectName, wait_function=None):
+        """Swipe the item in from left to right to open leading actions
+           and click on the button representing the requested action.
+
+           parameters: action_objectName - object name of the action to be
+                       triggered.
+                       wait_function - a custom wait function to wait till the
+                       action is triggered
+        """
+        self._click_on_panel_action('leading', action_objectName,
+                                    wait_function)
+
+    @autopilot_logging.log_action(logger.info)
+    def trigger_trailing_action(self, action_objectName, wait_function=None):
+        """Swipe the item in from right to left to open trailing actions
+           and click on the button representing the requested action.
+
+           parameters: action_objectName - object name of the action to be
+                       triggered.
+                       wait_function - a custom wait function to wait till the
+                       action is triggered
+        """
+        self._click_on_panel_action('trailing', action_objectName,
+                                    wait_function)

=== modified file 'tests/autopilot/ubuntuuitoolkit/emulators.py'
--- tests/autopilot/ubuntuuitoolkit/emulators.py	2014-08-20 06:45:28 +0000
+++ tests/autopilot/ubuntuuitoolkit/emulators.py	2014-12-08 14:37:55 +0000
@@ -37,6 +37,7 @@
     'CheckBox',
     'ComposerSheet',
     'Empty',
+    'UCListItem',
     'Header',
     'ItemSelector',
     'MainView',
@@ -63,6 +64,7 @@
     get_pointing_device,
     CheckBox,
     Header,
+    UCListItem,
     MainView,
     OptionSelector,
     QQuickFlickable,

=== added file 'tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_listitem.ListItemTestCase.qml'
--- tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_listitem.ListItemTestCase.qml	1970-01-01 00:00:00 +0000
+++ tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_listitem.ListItemTestCase.qml	2014-12-08 14:37:55 +0000
@@ -0,0 +1,68 @@
+/*
+ * Copyright 2014 Canonical Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import QtQuick 2.0
+import Ubuntu.Components 1.2
+
+MainView {
+    width: units.gu(48)
+    height: units.gu(60)
+
+    // make sure we're not messed up by the deprecated toolbar
+    useDeprecatedToolbar: false
+
+    Page {
+        id: testPage
+        objectName: "test_page"
+        title: "No action triggered"
+        ListView {
+            id: listView
+            anchors.fill: parent
+            model: 25
+            delegate: ListItem {
+                objectName: "listitem" + index
+                leadingActions: ListItemActions {
+                    actions: [
+                        Action {
+                            iconName: "delete"
+                            objectName: 'delete_action'
+                            onTriggered: testPage.title = objectName + " action triggered";
+                        }
+                    ]
+                }
+                trailingActions: ListItemActions {
+                    actions: [
+                        Action {
+                            iconName: "search"
+                            objectName: 'search_action'
+                            onTriggered: testPage.title = objectName + " action triggered";
+                        },
+                        Action {
+                            iconName: "edit"
+                            objectName: 'edit_action'
+                            onTriggered: testPage.title = objectName + " action triggered";
+                        },
+                        Action {
+                            iconName: "email"
+                            objectName: 'email_action'
+                            onTriggered: testPage.title = objectName + " action triggered";
+                        }
+                    ]
+                }
+            }
+        }
+    }
+}

=== added file 'tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_listitem.py'
--- tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_listitem.py	1970-01-01 00:00:00 +0000
+++ tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_listitem.py	2014-12-08 14:37:55 +0000
@@ -0,0 +1,86 @@
+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
+#
+# Copyright (C) 2013, 2014 Canonical Ltd.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; version 3.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+import os
+
+import ubuntuuitoolkit
+from ubuntuuitoolkit import tests
+
+
+class ListItemTestCase(tests.QMLFileAppTestCase):
+    path = os.path.abspath(__file__)
+    dir_path = os.path.dirname(path)
+    test_qml_file_path = os.path.join(
+        dir_path, 'test_listitem.ListItemTestCase.qml')
+
+    def setUp(self):
+        super(ListItemTestCase, self).setUp()
+        self.test_listitem = self.main_view.select_single(
+            'UCListItem', objectName='listitem0')
+        self.test_page = self.main_view.select_single(
+            objectName='test_page')
+        self.assertEqual(self.test_page.title, 'No action triggered')
+
+    def test_trigger_delete(self):
+        self.test_listitem.trigger_leading_action('delete_action')
+        self.assertEqual(self.test_page.title,
+                         'delete_action action triggered')
+
+    def test_trigger_search(self):
+        self.test_listitem.trigger_trailing_action('search_action')
+        self.assertEqual(self.test_page.title,
+                         'search_action action triggered')
+
+    def test_trigger_edit(self):
+        self.test_listitem.trigger_trailing_action('edit_action')
+        self.assertEqual(self.test_page.title,
+                         'edit_action action triggered')
+
+    def test_trigger_email(self):
+        self.test_listitem.trigger_trailing_action('email_action')
+        self.assertEqual(self.test_page.title,
+                         'email_action action triggered')
+
+    def test_trigger_all_actions(self):
+        self.test_listitem.trigger_leading_action('delete_action')
+        self.assertEqual(self.test_page.title,
+                         'delete_action action triggered')
+
+        self.test_listitem.trigger_trailing_action('search_action')
+        self.assertEqual(self.test_page.title,
+                         'search_action action triggered')
+
+        self.test_listitem.trigger_trailing_action('edit_action')
+        self.assertEqual(self.test_page.title,
+                         'edit_action action triggered')
+
+        self.test_listitem.trigger_trailing_action('email_action')
+        self.assertEqual(self.test_page.title,
+                         'email_action action triggered')
+
+    def test_trigger_nonexistent_leading_action(self):
+        error = self.assertRaises(ubuntuuitoolkit.ToolkitException,
+                                  self.test_listitem.trigger_leading_action,
+                                  ('this_action_does_not_exist'))
+        self.assertEqual(str(error),
+                         'The requested action not found on leading side')
+
+    def test_trigger_nonexistent_trailing_action(self):
+        error = self.assertRaises(ubuntuuitoolkit.ToolkitException,
+                                  self.test_listitem.trigger_trailing_action,
+                                  'this_action_does_not_exist')
+        self.assertEqual(str(error),
+                         'The requested action not found on trailing side')