Merge lp:~vthompson/ubuntu-calendar-app/fix-1276788 into lp:ubuntu-calendar-app

Proposed by Victor Thompson on 2014-03-02
Status: Work in progress
Proposed branch: lp:~vthompson/ubuntu-calendar-app/fix-1276788
Merge into: lp:ubuntu-calendar-app
Diff against target: 31 lines (+8/-2)
1 file modified
PathViewBase.qml (+8/-2)
To merge this branch: bzr merge lp:~vthompson/ubuntu-calendar-app/fix-1276788
Reviewer Review Type Date Requested Status
David Planella Needs Information on 2014-04-16
Olivier Tilloy 2014-03-02 Needs Fixing on 2014-03-07
Ubuntu Phone Apps Jenkins Bot continuous-integration Needs Fixing on 2014-03-07
Review via email: mp+208979@code.launchpad.net

Commit message

Prevent views from changing more than one item while dragging

Description of the change

Prevent views from changing more than one item while dragging. This change does have the drawback of when dragging you are not able to change the direction of which direction you are dragging as the view is not interactive again until the dragging action has ended.

An alternative solution I was looking into used a combination of maximumFlickVelocity and flickDeceleration to possibly prevent the flicking/dragging action from skipping an item. I couldn't find a sweet spot on my N4 in terms of values and I'm not sure how any changes would affect the app's usability with say the Google Nexus.

To post a comment you must log in.
Olivier Tilloy (osomon) wrote :

This fixes the issue with autopilot tests sometimes flicking two items at a time, however it’s a regression in terms of usability, as one can’t undo a flick movement that was started. Also, once the current item has changed (i.e. mid-flick), the view is blocked in between two items.

If we can’t come up with a better solution for now, we should at least add a comment to explain the fix and explicitly mark it as temporary. According to the documentation for PathView (http://qt-project.org/doc/qt-5.0/qtquick/qml-qtquick2-pathview.html), the right combination of values for snapMode, preferredHighlightBegin and preferredHighlightEnd should ensure that no more than one item can be flicked at a time, so what we’re seeing might very well be a bug in Qt itself. This requires deeper investigation.

Victor Thompson (vthompson) wrote :

I'll continue to investigate the PathView behavior later on today.

211. By Victor Thompson on 2014-03-07

Fix attemp #2

Victor Thompson (vthompson) wrote :

I've pushed a second fix. This time the only draw back I can see is that when you are able to create do a swipe that should go from, say day 1 to day 3, you can see the hours go by twice--but the actual effect is that the day is only incremented once. Seems like a fair solution.

Olivier Tilloy (osomon) wrote :

> I've pushed a second fix. This time the only draw back I can see is that when
> you are able to create do a swipe that should go from, say day 1 to day 3, you
> can see the hours go by twice--but the actual effect is that the day is only
> incremented once. Seems like a fair solution.

Functionally this is better (and seems to fix bug #1276788 indeed), however IMHO it’s still not good enough. Try swiping from one month to the next (on desktop it’s easier to reproduce, with a long click and drag gesture that goes out of the app’s window), and you will see the month name changing from March (current) to April, then briefly to May and then May becomes April again. Pretty confusing from a usability standpoint.

review: Needs Fixing
Roman Shchekin (mrqtros) wrote :

From documentation of PathView.snapMode property

PathView.SnapOneItem - the items settle no more than one item away from the item nearest preferredHighlightBegin at the time the press is released. This mode is particularly useful for moving one page at a time.

Use it, please. You can find example of usage (of similar property in ListView) in Shorts.

Kunal Parmar (pkunal-parmar) wrote :

> From documentation of PathView.snapMode property
>
> PathView.SnapOneItem - the items settle no more than one item away from the
> item nearest preferredHighlightBegin at the time the press is released. This
> mode is particularly useful for moving one page at a time.
>
> Use it, please. You can find example of usage (of similar property in
> ListView) in Shorts.

Hi roman,
If we see full code of PathViewBase.qml, pathview is already using snapOneItem. Do you think we are missing anything else ?

PathView {
    id: root

    model: 3
    snapMode: PathView.SnapOneItem

 ...

    preferredHighlightBegin: 0.5
    preferredHighlightEnd: 0.5

Roman Shchekin (mrqtros) wrote :

Let me play with it a little today :)
I'll find solution.

David Planella (dpm) wrote :

Thanks Roman for your help in getting the pending Calendar branches in
shape, this will be very useful in getting a solid app for 14.04!

On Tue, Apr 1, 2014 at 6:28 PM, Roman Shchekin <email address hidden> wrote:

> Let me play with it a little today :)
> I'll find solution.
> --
>
> https://code.launchpad.net/~vthompson/ubuntu-calendar-app/fix-1276788/+merge/208979
> Your team Ubuntu Calendar Developers is subscribed to branch
> lp:ubuntu-calendar-app.
>

Roman Shchekin (mrqtros) wrote :

Hmm... Seems that SnapOneItem doestn't work at all. I will test it in separate project now.

Roman Shchekin (mrqtros) wrote :

Wow, we have the same behavior in Shorts (in ListView). And there are no any way how we can fix it =(

Victor Thompson (vthompson) wrote :

I don't think SnapOneItem is intended to ensure that only one item is progressed. I've tried to make the pathview lock on either the previous or next item with no luck really. I'm not really sure what else to try here.

Kunal Parmar (pkunal-parmar) wrote :

I think we need something in which we have some more control.

I tried to create a dirty prototype for the same,
It seems to work fine, I will comments when I upload code. Let's see if that something which we can use.

David Planella (dpm) wrote :

Kunal, have you had the chance to upload the code with your prototype?

review: Needs Information
Victor Thompson (vthompson) wrote :

Marking this MP as a WIP and removing myself as the assignee. Branch will still be related if anyone wants to build off of what is in place.

Unmerged revisions

211. By Victor Thompson on 2014-03-07

Fix attemp #2

210. By Victor Thompson on 2014-03-02

Prevent view from changing more than one item while dragging

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'PathViewBase.qml'
2--- PathViewBase.qml 2013-12-14 02:44:09 +0000
3+++ PathViewBase.qml 2014-03-07 06:43:48 +0000
4@@ -8,6 +8,7 @@
5
6 signal nextItemHighlighted();
7 signal previousItemHighlighted();
8+ property int previousDiff: 0
9
10 preferredHighlightBegin: 0.5
11 preferredHighlightEnd: 0.5
12@@ -39,12 +40,17 @@
13
14 intern.previousIndex = currentIndex
15
16- if ( diff > 0 ) {
17+ if ( diff > 0 && previousDiff <= 0) {
18 root.nextItemHighlighted();
19 }
20- else {
21+ else if ( diff < 0 && previousDiff >= 0) {
22 root.previousItemHighlighted();
23 }
24+ previousDiff = diff;
25+ }
26+
27+ onMovementEnded: {
28+ previousDiff = 0;
29 }
30
31 QtObject{

Subscribers

People subscribed via source and target branches

to status/vote changes: