Ubuntu UI Toolkit

tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/_flickable.py (+18/-10)
tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_flickable.py (+44/-1)

To merge this branch:

bzr merge lp:~canonical-platform-qa/ubuntu-ui-toolkit/fix1401517-overwrite_swipe_borders

Undecided

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration	2015-02-07	Approve on 2015-04-16
Zsombor Egri		2015-02-07	Approve on 2015-04-16
Brendan Donegan (community)			Approve on 2015-04-16
Leo Arias (community)			Approve on 2015-04-15
Albert Astals Cid (community)		2015-02-07	Abstain on 2015-02-13
Review via email: mp+248986@code.launchpad.net

This proposal supersedes a proposal from 2014-12-17.

Commit message

Make it possible to overwrite the bottom margin for the flickable autopilot helper.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-12-18: Posted in a previous version of this proposal

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1337/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-06: Posted in a previous version of this proposal

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1345/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-12: Posted in a previous version of this proposal

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1356/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Tim Peeters (tpeeters) wrote on 2015-01-13: Posted in a previous version of this proposal

37 elif direction == 'above':
38 - start_y = top
39 - stop_y = bottom
40 + start_y = top + 5
41 + stop_y = bottom - 5

to be consistent, you should use
start_y = top + self.margin_to_swipe_from_top

Revision history for this message

Tim Peeters (tpeeters) wrote on 2015-01-13: Posted in a previous version of this proposal

+ # Swiping from below can open the toolbar or trigger the bottom edge
9 + # gesture. Use this margin to start a swipe that will not be that close to
10 + # the bottom edge.
11 + margin_to_swipe_from_bottom = 25

Where did you get the 25 pixels from? Shouldn't we use grid units here?

We have this in the UITK Panel.qml:

    /*!
      The size (height for top or bottom-aligned panels, width for left or right-aligned
      panels) of the mouse area used to detect edge swipes to open the panel, when
      the panel is not opened. Default value: units.gu(2).
     */
    property real triggerSize: units.gu(2)

So apps that use the Panel with default settings will need a margin of 2 grid units. Of course apps can override this with their own value (or not use the Panel at all and implement their own bottom-edge).

Revision history for this message

Leo Arias (elopio) wrote on 2015-02-07:

@albert, I more or less understand what you did with the drags and the rate.
What I don't understand is why you add the rate to the top and subtract it from the bottom. I removed that part, so can you please take a look and confirm I'm not breaking anything from your change?

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-02-07:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1422/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-02-09:

Leo, the idea +/- rate was already there, i kept it as "buffer" zone since we're doing a integer division and then multiplying back byu the rate we may get a big enough number and i did not want to end up past top/bottom but i guess that it's not really possible.

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-02-13:

I'll leave it to the SDK guys

review: Abstain

Revision history for this message

Leo Arias (elopio) wrote on 2015-02-13:

The prerequisite has a test failing in mako. Work in progress while we find the issue.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-14:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1666/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Federico Gimenez (fgimenez) wrote on 2015-04-15:

The failing tests have been fixed and the prerequisite has already landed, back to Needs review

Revision history for this message

Brendan Donegan (brendan-donegan) wrote on 2015-04-15:

LGTM +1

Revision history for this message

Leo Arias (elopio) wrote on 2015-04-15:

We already have functional tests that do the swiping for real, so I think it's ok for this case to use a mock. One of those swiping tests failed in the latest execution, which makes me uneasy. Can someone retrigger the jenkins job? I don't have the vpn configured in my laptop, I will do it as soon as I get some quiet time.

lp:~canonical-platform-qa/ubuntu-ui-toolkit/fix1401517-overwrite_swipe_borders updated on 2015-04-15

1381. By Federico Gimenez on 2015-04-15: Merged with staging

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-15:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1671/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Leo Arias (elopio) wrote on 2015-04-15:

+1. There seems to be an qmlscene crash making some tests fail, but on this run all the tests affected are passing.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-04-15:

PASSED: Continuous integration, rev:1381
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1674/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/2253
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/401
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/404
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/404/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/401
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/1978
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2251
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/2251/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19659

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/1674/rebuild

review: Approve (continuous-integration)

Revision history for this message

Brendan Donegan (brendan-donegan) wrote on 2015-04-16:

My previous comment didn't come out as an Approve. Approving again

review: Approve

Revision history for this message

Zsombor Egri (zsombi) wrote on 2015-04-16:

Looks good to go. Thank you!

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) on 2015-04-16:

review: Approve (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:

Canonical Platform QA Team

Dan Chapman 

Tom Mirmar

Ubuntu SDK team

 === modified file 'tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/_flickable.py'
 --- tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/_flickable.py	2015-02-03 08:34:58 +0000
 +++ tests/autopilot/ubuntuuitoolkit/_custom_proxy_objects/_flickable.py	2015-04-15 13:48:54 +0000
@@ -19,6 +19,7 @@
  import autopilot.exceptions
  from autopilot import logging as autopilot_logging
++from ubuntuuitoolkit import units
  from ubuntuuitoolkit._custom_proxy_objects import _common
@@ -98,6 +99,14 @@
  class QQuickFlickable(Scrollable):
++    # Swiping from below can open the toolbar or trigger the bottom edge
++    # gesture. Use this margin to start a swipe that will not be that close to
++    # the bottom edge.
++    margin_to_swipe_from_bottom = units.gu(2)
++    # Swiping from above can open the indicators or resize the window. Use this
++    # margin to start a swipe that will not be that close to the top edge.
++    margin_to_swipe_from_top = units.gu(1)
++
      @autopilot_logging.log_action(logger.info)
      def swipe_child_into_view(self, child):
          """Make the child visible.
@@ -145,24 +154,23 @@
              containers = self._get_containers()
          start_x = stop_x = self.globalRect.x + (self.globalRect.width // 2)
++        top = _get_visible_container_top(containers)
++        bottom = _get_visible_container_bottom(containers)
++
          # Make the drag range be a multiple of the drag "rate" value.
          # Workarounds https://bugs.launchpad.net/mir/+bug/1399690
          rate = self._slow_drag_rate()
--        # Start and stop just a little under the top and a little over the
--        # bottom.
--        top = _get_visible_container_top(containers) + rate
--        bottom = _get_visible_container_bottom(containers) - rate
--
++        # The swipes are not done from right at the top and bottom because
++        # they could trigger edge gestures or resize windows.
          if direction == 'below':
--            # Take into account that swiping from below can open the toolbar or
--            # trigger the bottom edge gesture.
--            # XXX Do this only if we are close to the bottom edge.
--            start_y = bottom - 20
++            start_y = bottom - self.margin_to_swipe_from_bottom
              stop_y = start_y + (top - start_y) // rate * rate
++
          elif direction == 'above':
--            start_y = top
++            start_y = top + self.margin_to_swipe_from_top
              stop_y = start_y + (bottom - start_y) // rate * rate
++
          else:
              raise _common.ToolkitException(
                  'Invalid direction {}.'.format(direction))
 === modified file 'tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_flickable.py'
 --- tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_flickable.py	2015-03-03 13:20:06 +0000
 +++ tests/autopilot/ubuntuuitoolkit/tests/custom_proxy_objects/test_flickable.py	2015-04-15 13:48:54 +0000
@@ -21,7 +21,11 @@
  import testtools
  import ubuntuuitoolkit
--from ubuntuuitoolkit import tests
++from ubuntuuitoolkit import (
++    tests,
++    units,
++)
++from ubuntuuitoolkit._custom_proxy_objects import _flickable
  class FlickableTestCase(testtools.TestCase):
@@ -218,6 +222,45 @@
          self.flickable.swipe_to_show_more_below()
          self.assertFalse(self.flickable.atYBeginning)
++    def test_swipe_to_show_more_below_with_bottom_margin(self):
++        """Calling swipe to show more below will use the margin in the drag."""
++        qquickflickable = self.main_view.select_single(
++            ubuntuuitoolkit.QQuickFlickable, objectName='flickable')
++        qquickflickable.margin_to_swipe_from_bottom = units.gu(6)
++        containers = qquickflickable._get_containers()
++        bottom = _flickable._get_visible_container_bottom(containers)
++
++        with mock.patch.object(
++                qquickflickable.pointing_device, 'drag') as mock_drag:
++            try:
++                qquickflickable.swipe_to_show_more_below()
++            except ubuntuuitoolkit.ToolkitException:
++                # An exception will be raised because the drag was faked.
++                pass
++
++        mock_drag.assert_called_with(
++            mock.ANY, bottom - units.gu(6), mock.ANY, mock.ANY, rate=mock.ANY)
++
++    def test_swipe_to_show_more_above_with_top_margin(self):
++        """Calling swipe to show more above will use the margin in the drag."""
++        qquickflickable = self.main_view.select_single(
++            ubuntuuitoolkit.QQuickFlickable, objectName='flickable')
++        qquickflickable.margin_to_swipe_from_top = units.gu(6)
++        containers = qquickflickable._get_containers()
++        top = _flickable._get_visible_container_top(containers)
++
++        qquickflickable.swipe_to_bottom()
++        with mock.patch.object(
++                qquickflickable.pointing_device, 'drag') as mock_drag:
++            try:
++                qquickflickable.swipe_to_show_more_above()
++            except ubuntuuitoolkit.ToolkitException:
++                # An exception will be raised because the drag was faked.
++                pass
++
++        mock_drag.assert_called_with(
++            mock.ANY, top + units.gu(6), mock.ANY, mock.ANY, rate=mock.ANY)
++
      def test_failed_drag_must_raise_exception(self):
          dummy_coordinates = (0, 0, 10, 10)
          # Patch the pointing device so it does nothing and the swipe fails.

Ubuntu UI Toolkit

Merge lp:~canonical-platform-qa/ubuntu-ui-toolkit/fix1401517-overwrite_swipe_borders into lp:ubuntu-ui-toolkit/staging

Commit message

Description of the change

Preview Diff

Subscribers