Merge lp://staging/~zsombi/ubuntu-ui-toolkit/ubuntutestcase-extras into lp://staging/ubuntu-ui-toolkit/staging

Proposed by Zsombor Egri
Status: Merged
Approved by: Tim Peeters
Approved revision: 1007
Merged at revision: 1002
Proposed branch: lp://staging/~zsombi/ubuntu-ui-toolkit/ubuntutestcase-extras
Merge into: lp://staging/ubuntu-ui-toolkit/staging
Diff against target: 275 lines (+181/-18)
5 files modified
components.api (+2/-0)
modules/Ubuntu/Test/UbuntuTestCase.qml (+69/-1)
modules/Ubuntu/Test/deployment.pri (+6/-1)
tests/unit/runtest.sh (+1/-1)
tests/unit_x11/tst_test/tst_ubuntutestcase.qml (+103/-15)
To merge this branch: bzr merge lp://staging/~zsombi/ubuntu-ui-toolkit/ubuntutestcase-extras
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Tim Peeters Approve
Review via email: mp+214927@code.staging.launchpad.net

This proposal supersedes a proposal from 2014-04-08.

Commit message

Flick and mouse long press simulation added to UbuntuTestCase.

To post a comment you must log in.
Revision history for this message
Tim Peeters (tpeeters) wrote : Posted in a previous version of this proposal

12 + The function simulates a flick event over an \item. The flick is executed
13 + between \a from and \to points (built using Qt.point()) with a given \a speed

I think \item should be \a item and \to should be \a to.

Revision history for this message
Tim Peeters (tpeeters) wrote : Posted in a previous version of this proposal

Why is poinCount = 5 needed? Does a single mouseMove not work as a flick?

Revision history for this message
Tim Peeters (tpeeters) wrote : Posted in a previous version of this proposal

This code can be written a bit nicer:
39 + for (var i = 0; i < pointCount; i++) {
40 + mouseMove(item, from.x + (i + 1) * dx / pointCount, from.y + (i + 1) * dy / pointCount, speed);
41 + }

var dx = (to.x - from.x) / pointCount;
var dy = (to.y - from.y) / pointCount;

for (var i = 1; i <= pointCount; i++) {
  mouseMove(item, from.x + i*dx, from.y + i*dy, speed);
}

Revision history for this message
Zsombor Egri (zsombi) wrote : Posted in a previous version of this proposal

> Why is poinCount = 5 needed? Does a single mouseMove not work as a flick?

No, it doesn't. I took the function btw from the tst_textarea_in_flickable.qml, and that's how we did there. One mouse move only takes a small move, and may not even be recognized.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Zsombor Egri (zsombi) wrote : Posted in a previous version of this proposal

> This code can be written a bit nicer:
> 39 + for (var i = 0; i < pointCount; i++) {
> 40 + mouseMove(item, from.x + (i + 1) * dx / pointCount, from.y + (i + 1)
> * dy / pointCount, speed);
> 41 + }
>
>
> var dx = (to.x - from.x) / pointCount;
> var dy = (to.y - from.y) / pointCount;
>
> for (var i = 1; i <= pointCount; i++) {
> mouseMove(item, from.x + i*dx, from.y + i*dy, speed);
> }

Yep, used. I copied the code as was in the test. I modified the API and implementation to be closer to mouseXXX() functions. Please check it once more.

Revision history for this message
Zsombor Egri (zsombi) wrote : Posted in a previous version of this proposal

> > Why is poinCount = 5 needed? Does a single mouseMove not work as a flick?
>
> No, it doesn't. I took the function btw from the
> tst_textarea_in_flickable.qml, and that's how we did there. One mouse move
> only takes a small move, and may not even be recognized.

So: we need at least two mouse moves to trigger a flick on a Flickable. I defaulted the implementation to 5 points (as in mouseDrag() function) and it can be altered by the steps parameter - the bigger the steps number is the longer the flick will be.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Tim Peeters (tpeeters) wrote :

53 + var ddx = (to.x - from.x) / steps;
54 + var ddy = (to.y - from.y) / steps;

You don't need to compute "to" anymore. Just use ddx = dx / steps.

review: Needs Fixing
Revision history for this message
Tim Peeters (tpeeters) wrote :

"from" is also not needed. You can use x and y.

You only use "to" here:
64 + mouseRelease(item, to.x, to.y, button, modifiers, delay);
but there you can easily say x+dx, y+dy.

Revision history for this message
Tim Peeters (tpeeters) wrote :

30 + The default flick velocity is built up using 5 move points. This can be altered
31 + by setting a positive value to \a steps parameter. The bigger the number the
32 + longer the flick will be.

Can you add something like this?
"When a negative value is given, the default of 5 move points will be used."

Revision history for this message
Tim Peeters (tpeeters) wrote :

also for the pressTimeout. "Setting a negative value will disable the press timeout."

Revision history for this message
Tim Peeters (tpeeters) wrote :

First MR ready to go in staging :)

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches