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

Proposed by Victor Thompson
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
Olivier Tilloy (community) Needs Fixing
Ubuntu Phone Apps Jenkins Bot continuous-integration Needs Fixing
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.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
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.

Revision history for this message
Victor Thompson (vthompson) wrote :

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

211. By Victor Thompson

Fix attemp #2

Revision history for this message
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.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
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
Revision history for this message
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.

Revision history for this message
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

Revision history for this message
Roman Shchekin (mrqtros) wrote :

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

Revision history for this message
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.
>

Revision history for this message
Roman Shchekin (mrqtros) wrote :

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

Revision history for this message
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 =(

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
David Planella (dpm) wrote :

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

review: Needs Information
Revision history for this message
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

Fix attemp #2

210. By Victor Thompson

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
=== modified file 'PathViewBase.qml'
--- PathViewBase.qml 2013-12-14 02:44:09 +0000
+++ PathViewBase.qml 2014-03-07 06:43:48 +0000
@@ -8,6 +8,7 @@
88
9 signal nextItemHighlighted();9 signal nextItemHighlighted();
10 signal previousItemHighlighted();10 signal previousItemHighlighted();
11 property int previousDiff: 0
1112
12 preferredHighlightBegin: 0.513 preferredHighlightBegin: 0.5
13 preferredHighlightEnd: 0.514 preferredHighlightEnd: 0.5
@@ -39,12 +40,17 @@
3940
40 intern.previousIndex = currentIndex41 intern.previousIndex = currentIndex
4142
42 if ( diff > 0 ) {43 if ( diff > 0 && previousDiff <= 0) {
43 root.nextItemHighlighted();44 root.nextItemHighlighted();
44 }45 }
45 else {46 else if ( diff < 0 && previousDiff >= 0) {
46 root.previousItemHighlighted();47 root.previousItemHighlighted();
47 }48 }
49 previousDiff = diff;
50 }
51
52 onMovementEnded: {
53 previousDiff = 0;
48 }54 }
4955
50 QtObject{56 QtObject{

Subscribers

People subscribed via source and target branches

to status/vote changes: