Ubuntu UI Toolkit

Merge lp:~zsombi/ubuntu-ui-toolkit/slider-blocks-flickable into lp:ubuntu-ui-toolkit/staging

slider-blocks-flickable
Merge into staging

Proposed by Zsombor Egri on 2014-08-12

Status:

Merged

Approved by:

Tim Peeters on 2014-08-20

Approved revision:

1194

Merged at revision:

1198

Proposed branch:

lp:~zsombi/ubuntu-ui-toolkit/slider-blocks-flickable

Merge into:

lp:ubuntu-ui-toolkit/staging

Diff against target:

261 lines (+240/-0)

3 files modified

modules/Ubuntu/Components/Slider.qml (+20/-0)
tests/resources/sliders/SliderTest.qml (+92/-0)
tests/unit_x11/tst_components/tst_slider.qml (+128/-0)

To merge this branch:

bzr merge lp:~zsombi/ubuntu-ui-toolkit/slider-blocks-flickable

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2014-08-20
Tim Peeters		2014-08-12	Approve on 2014-08-20
Review via email: mp+230438@code.launchpad.net

Commit message

Slider blocks Flickable while sliding.

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-08-12:

This will mess up the flickable settings if for some reason Flickable.interactive must be set to false. When interacting with the slider, it will be reset to true.

Is it not possible to block the events going to the flickable by setting propagateComposedEvents of the Slider's MouseArea to false?

review: Needs Fixing

Revision history for this message

Zsombor Egri (zsombi) wrote on 2014-08-12:

It's not possible. Flickable filters mouse events. And even if it would be possible, it would block the vertical swipes as well, which we don't want to, right?

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-08-12:

Update the (C) year

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-12:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-13:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Zsombor Egri (zsombi) wrote on 2014-08-15:

> Update the (C) year

Which one?

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-08-19:

> > Update the (C) year
>
> Which one?

Slider.qml

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-08-19:

> > Update the (C) year
>
> Which one?

Slider.qml

Revision history for this message

Zsombor Egri (zsombi) wrote on 2014-08-19:

> > > Update the (C) year
> >
> > Which one?
>
> Slider.qml

Whu should I update Slider.qml? It has been made 2 years ago...

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-08-19:

If this output is intentional, add some extra info to the print()
75 + onInteractiveChanged: print(interactive)

if not, remove please

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-08-19:

This MR will make flicking hard when there is a slider in the way.

What happens when you are flicking trough a big list, and then at one point (you keep flicking) you "grab" on a slider to flick more.. Flicking stops and the slider value changes?

Revision history for this message

Zsombor Egri (zsombi) wrote on 2014-08-19:

> This MR will make flicking hard when there is a slider in the way.
>
> What happens when you are flicking trough a big list, and then at one point
> (you keep flicking) you "grab" on a slider to flick more.. Flicking stops and
> the slider value changes?

I see two patterns here:
1) when flicking or kinetic scrolling, there should be no component getting the focus/interaction
2) when a slider is touched, it would grab the focus for ~500-800 msecs, then the Flickable behind would take the control; this would give more time for a Slider to handle its events
2.1) however a better gesture handling would be needed here than simple MouseAreas. Like adding a threshold value that specifies what is the interval the other direction is still handled by the component, and after which the gesture is forwarded to the component behind.

lp:~zsombi/ubuntu-ui-toolkit/slider-blocks-flickable updated on 2014-08-19

1192. By Zsombor Egri on 2014-08-19: rogue print removed

Revision history for this message

Zsombor Egri (zsombi) wrote on 2014-08-19:

> > This MR will make flicking hard when there is a slider in the way.
> >
> > What happens when you are flicking trough a big list, and then at one point
> > (you keep flicking) you "grab" on a slider to flick more.. Flicking stops
> and
> > the slider value changes?
>
> I see two patterns here:
> 1) when flicking or kinetic scrolling, there should be no component getting
> the focus/interaction
> 2) when a slider is touched, it would grab the focus for ~500-800 msecs, then
> the Flickable behind would take the control; this would give more time for a
> Slider to handle its events
> 2.1) however a better gesture handling would be needed here than simple
> MouseAreas. Like adding a threshold value that specifies what is the interval
> the other direction is still handled by the component, and after which the
> gesture is forwarded to the component behind.

Ok, had a meeting with Giorgio, and this is the preferred way to do it.

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-08-19:

Why did you add a separate directory resources/sliders/? Do you anticipate a lot of files to be added there

61 + property bool trueValue: true
I think this can be removed.

SliderTest.qml and tst_slider.qml have a lot of similar code.

Is there a way that we can simply run tst_slider.qml with qmlscene? Then we don't need to keep SliderTest any more.

Revision history for this message

Zsombor Egri (zsombi) wrote on 2014-08-19:

> Why did you add a separate directory resources/sliders/? Do you anticipate a
> lot of files to be added there

No, just that I thought our policy is to have a separate folder for each component, but yeah, it can be straight in tests/resources.

>
> 61 + property bool trueValue: true
> I think this can be removed.
Ah, yes. Good catch!

>
> SliderTest.qml and tst_slider.qml have a lot of similar code.
>
> Is there a way that we can simply run tst_slider.qml with qmlscene? Then we
> don't need to keep SliderTest any more.

I tried but didn't work... I can try once more a bit harder.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-19:

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

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-20:

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

review: Approve (continuous-integration)

Revision history for this message

Zsombor Egri (zsombi) wrote on 2014-08-20:

Whoaaah, Jenkins works!!!

lp:~zsombi/ubuntu-ui-toolkit/slider-blocks-flickable updated on 2014-08-20

1193. By Zsombor Egri on 2014-08-20: unneeded property removed
1194. By Zsombor Egri on 2014-08-20: staging merge

Revision history for this message

Tim Peeters (tpeeters) wrote on 2014-08-20:

good, thanks!

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-20:

FAILED: Continuous integration, rev:1194
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/847/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/3818
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/2934
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/679
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/679
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/679/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/679
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3680
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5065
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5065/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11797
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2384
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3218
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3218/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (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:

Dan Chapman 

Tom Mirmar

Ubuntu SDK team

Zsombor Egri

 === modified file 'modules/Ubuntu/Components/Slider.qml'
 --- modules/Ubuntu/Components/Slider.qml	2014-04-23 08:50:20 +0000
 +++ modules/Ubuntu/Components/Slider.qml	2014-08-20 10:02:27 +0000
@@ -140,6 +140,26 @@
          property real dragInitMouseX: 0.0
          property real dragInitNormalizedValue: 0.0
++        property Flickable flickable: {
++            // traverse parents to catch whether we have an ancestor Flickable
++            var pl = slider.parent;
++            while (pl) {
++                if (pl.hasOwnProperty("flicking")) {
++                    return pl;
++                }
++                pl = pl.parent;
++            }
++            return null;
++        }
++
++        states: State {
++            name: "sliding"
++            when: mouseArea.flickable && mouseArea.pressed
++            PropertyChanges {
++                target: mouseArea.flickable
++                interactive: false
++            }
++        }
          function normalizedValueFromValue(value) {
              if (Qt.application.layoutDirection == Qt.RightToLeft) {
                  return MathUtils.clampAndProject(value, slider.minimumValue,
 === added directory 'tests/resources/sliders'
 === added file 'tests/resources/sliders/SliderTest.qml'
 --- tests/resources/sliders/SliderTest.qml	1970-01-01 00:00:00 +0000
 +++ tests/resources/sliders/SliderTest.qml	2014-08-20 10:02:27 +0000
@@ -0,0 +1,92 @@
++/*
++ * 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.1
++import Ubuntu.Components.ListItems 1.0
++
++MainView {
++    width: units.gu(50)
++    height: units.gu(100)
++
++    Column {
++        anchors {
++            fill: parent
++            margins: units.gu(1)
++        }
++        UbuntuListView {
++            id: listView
++            width: parent.width
++            height: units.gu(20)
++            clip: true
++            model: 10
++            interactive: true
++            delegate: Standard {
++                control: Slider {
++                    objectName: "testSlider" + index
++                }
++            }
++        }
++        UbuntuListView {
++            id: listView2
++            width: parent.width
++            height: units.gu(20)
++            clip: true
++            model: 10
++            delegate: Slider {
++                objectName: "testSlider" + index
++            }
++        }
++        Flickable {
++            id: flickable
++            width: parent.width
++            height: units.gu(20)
++            clip: true
++            contentHeight: column.height
++            Column {
++                id: column
++                width: parent.width
++                height: childrenRect.height
++                Repeater {
++                    model: 10
++                    Standard {
++                        control: Slider {
++                            objectName: "testSlider" + index
++                        }
++                    }
++                }
++            }
++        }
++        Flickable {
++            id: flickable2
++            width: parent.width
++            height: units.gu(20)
++            clip: true
++            contentHeight: column2.height
++            Column {
++                id: column2
++                width: parent.width
++                height: childrenRect.height
++                Repeater {
++                    model: 10
++                    Slider {
++                        objectName: "testSlider" + index
++                    }
++                }
++            }
++        }
++    }
++}
 === added file 'tests/unit_x11/tst_components/tst_slider.qml'
 --- tests/unit_x11/tst_components/tst_slider.qml	1970-01-01 00:00:00 +0000
 +++ tests/unit_x11/tst_components/tst_slider.qml	2014-08-20 10:02:27 +0000
@@ -0,0 +1,128 @@
++/*
++ * 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 QtTest 1.0
++import Ubuntu.Test 1.0
++import Ubuntu.Components 1.1
++import Ubuntu.Components.ListItems 1.0
++
++MainView {
++    width: units.gu(40)
++    height: units.gu(71)
++
++    Column {
++        anchors {
++            fill: parent
++            margins: units.gu(1)
++        }
++        UbuntuListView {
++            id: listView
++            width: parent.width
++            height: units.gu(20)
++            clip: true
++            model: 10
++            delegate: Standard {
++                control: Slider {
++                    objectName: "testSlider" + index
++                }
++            }
++        }
++        UbuntuListView {
++            id: listView2
++            width: parent.width
++            height: units.gu(20)
++            clip: true
++            model: 10
++            delegate: Slider {
++                objectName: "testSlider" + index
++            }
++        }
++        Flickable {
++            id: flickable
++            width: parent.width
++            height: units.gu(20)
++            clip: true
++            contentHeight: column.height
++            Column {
++                id: column
++                width: parent.width
++                height: childrenRect.height
++                Repeater {
++                    model: 10
++                    Standard {
++                        control: Slider {
++                            objectName: "testSlider" + index
++                        }
++                    }
++                }
++            }
++        }
++        Flickable {
++            id: flickable2
++            width: parent.width
++            height: units.gu(20)
++            clip: true
++            contentHeight: column2.height
++            Column {
++                id: column2
++                width: parent.width
++                height: childrenRect.height
++                Repeater {
++                    model: 10
++                    Slider {
++                        objectName: "testSlider" + index
++                    }
++                }
++            }
++        }
++    }
++
++    UbuntuTestCase {
++        name: "SliderAPI"
++        when: windowShown
++
++        SignalSpy {
++            id: flickSpy
++            signalName: "onMovementStarted"
++        }
++
++        function cleanup() {
++            flickSpy.target = null;
++            flickSpy.clear();
++        }
++
++        function test_slider_blocks_flickable_data() {
++            return [
++                {tag: "ListView with Slider in ListItem", flickable: listView},
++                {tag: "ListView with Slider alone", flickable: listView2},
++                {tag: "Flickable with Column of Slider in ListItem", flickable: flickable},
++                {tag: "Flickable with Column of Slider alone", flickable: flickable2},
++            ];
++        }
++
++        function test_slider_blocks_flickable(data) {
++            flickSpy.target = data.flickable;
++            var slider = findChild(data.flickable, "testSlider1");
++            verify(slider, "cannot find test slider in " + data.tag);
++            var sliderPos = slider.mapToItem(data.flickable, units.gu(10), 0);
++            mouseDrag(data.flickable, sliderPos.x, sliderPos.y, units.gu(20), units.gu(20));
++            waitForRendering(data.flickable, 200);
++            compare(flickSpy.count, 0, "The Flickable should not move while Slider is active.");
++            compare(data.flickable.interactive, true, "The Flickable aint got back to interactive mode.");
++        }
++    }
++}

Ubuntu UI Toolkit

Merge lp:~zsombi/ubuntu-ui-toolkit/slider-blocks-flickable into lp:ubuntu-ui-toolkit/staging

Commit message

Description of the change

Preview Diff

Subscribers