Unity 8

Merge lp:~cimi/unity8/fix-1214423 into lp:unity8

fix-1214423
Merge into trunk

Proposed by Andrea Cimitan on 2013-10-28

Status:

Merged

Merged at revision:

724

Proposed branch:

lp:~cimi/unity8/fix-1214423

Merge into:

lp:unity8

Diff against target:

217 lines (+116/-55)

3 files modified

Components/RatingStars.qml (+45/-16)
tests/qmltests/CMakeLists.txt (+1/-1)
tests/qmltests/Components/tst_RatingStars.qml (+70/-38)

To merge this branch:

bzr merge lp:~cimi/unity8/fix-1214423

Undecided

Invalid

Link a bug report

Reviewer	Review Type	Date Requested	Status
Michael Zanetti (community)		2013-10-28	Needs Fixing on 2013-11-22
Michał Sawicz			Needs Fixing on 2013-11-13
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2013-10-28
Review via email: mp+192868@code.launchpad.net

Commit message

Adds interactive property to RatingStars, accepts user interaction, adds tests

Description of the change

Adds interactive property to RatingStars, accepts user interaction, adds tests

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2013-10-28:

29 + readonly property int starCount: 5

this shouldn't be readonly as it is intended to be configurable

Rest looks good to me.

review: Needs Fixing

Revision history for this message

Gerry Boland (gerboland) wrote on 2013-10-28:

Could you not use 1 MouseArea, instead of having one for each star image?

+ Math.max(0, Math.min(starCount * rating / maximumRating, maximumRating))
Using clamp from the SDK would be easier to read.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-28:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1501/rebuild

review: Needs Fixing (continuous-integration)

lp:~cimi/unity8/fix-1214423 updated on 2013-10-28

481. By Andrea Cimitan on 2013-10-28: Use single mouse area, make starCount writable
482. By Andrea Cimitan on 2013-10-28: renames...
483. By Andrea Cimitan on 2013-10-28: Use starCount as harmattan

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-28:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1504/rebuild

review: Needs Fixing (continuous-integration)

lp:~cimi/unity8/fix-1214423 updated on 2013-10-28

484. By Andrea Cimitan on 2013-10-28: revert previous commit

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-28:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1506/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-28:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1507/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-11-13:

Please add a test that ensures that non-interactive stars... are non-interactive :)

Think we should move this to the SDK?

Longer-term we'll need:
* support custom icon urls
* support dragging (not just tapping) to select rating

review: Needs Fixing

Revision history for this message

Michael Zanetti (mzanetti) wrote on 2013-11-22:

width/height are not set properly. this breaks all the places where it is used.

review: Needs Fixing

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-12-20:

Cimi?

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:

Andrea Cimitan

Mohammad Rakibul Hasan

Unity Team

 === modified file 'Components/RatingStars.qml'
 --- Components/RatingStars.qml	2013-06-05 22:03:08 +0000
 +++ Components/RatingStars.qml	2013-10-28 16:55:24 +0000
@@ -15,27 +15,56 @@
   */
  import QtQuick 2.0
++import Ubuntu.Components 0.1
--Row {
++Item {
      id: root
++
++    /*!
++      True if it accepts user input.
++     */
++    property bool interactive: true
++
++    /*!
++      Current rating.
++     */
      property int rating
++
++    /*!
++      Maximum rating.
++     */
      property int maximumRating: 5
--    property int starCount: 5
--    height: childrenRect.height
--    width: childrenRect.width
--
--    readonly property int effectiveRating: Math.max(0, Math.min(root.starCount * root.rating / root.maximumRating, root.maximumRating))
--
--    Repeater {
--        model: root.effectiveRating
--        Image {
--            source: "graphics/icon_star_on.png"
++
++    /*!
++      Number of rating stars.
++     */
++    property int starsCount: 5
++
++    readonly property int effectiveRating: MathUtils.clamp(starsCount * rating / maximumRating, 0, maximumRating)
++
++    Row {
++        id: row
++        anchors.centerIn: parent
++        height: childrenRect.height
++        width: childrenRect.width
++
++        Repeater {
++            id: repeater
++
++            property int averageDelegateWidth: row.width / root.starsCount
++
++            model: root.starsCount
++
++            Image {
++                objectName: "ratingStar" + index
++                source: index < root.effectiveRating ? "graphics/icon_star_on.png" : "graphics/icon_star_off.png"
++            }
+         }
+     }
--    Repeater {
--        model: root.starCount - root.effectiveRating
--        Image {
--            source: "graphics/icon_star_off.png"
--        }
++
++    MouseArea {
++        anchors.fill: row
++        enabled: root.interactive
++        onClicked: root.rating = Math.ceil(mouse.x / repeater.averageDelegateWidth) * root.maximumRating / root.starsCount
+     }
+ }
 === modified file 'tests/qmltests/CMakeLists.txt'
 --- tests/qmltests/CMakeLists.txt	2013-10-11 12:14:33 +0000
 +++ tests/qmltests/CMakeLists.txt	2013-10-28 16:55:24 +0000
@@ -15,7 +15,6 @@
  add_qml_test(Components Carousel)
  add_qml_test(Components MathLocal)
  add_qml_test(Components OpenEffect)
--add_qml_test(Components RatingStars)
  add_qml_test(Components TimeLocal)
  add_qml_test(Greeter Clock IMPORT_PATHS ${qmltest_DEFAULT_IMPORT_PATHS} ${CMAKE_BINARY_DIR}/plugins)
  add_qml_test(Panel IndicatorItem)
@@ -32,6 +31,7 @@
  add_qml_test(Components EdgeDemoOverlay)
  add_qml_test(Components FilterGrid IMPORT_PATHS ${qmltest_DEFAULT_IMPORT_PATHS} ${CMAKE_BINARY_DIR}/plugins)
  add_qml_test(Components LazyImage)
++add_qml_test(Components RatingStars)
  add_qml_test(Components ResponsiveFlowView)
  add_qml_test(Components ResponsiveGridView)
  add_qml_test(Components Revealer)
 === modified file 'tests/qmltests/Components/tst_RatingStars.qml'
 --- tests/qmltests/Components/tst_RatingStars.qml	2013-06-05 22:03:08 +0000
 +++ tests/qmltests/Components/tst_RatingStars.qml	2013-10-28 16:55:24 +0000
@@ -17,46 +17,78 @@
  import QtQuick 2.0
  import QtTest 1.0
  import "../../../Components"
--
--TestCase {
--    name: "RatingStars"
--
--    property int maximumRating: 5
--    property int rating: 3
--    property alias effectiveRating: ratingStars.effectiveRating
++import Ubuntu.Components 0.1
++import Unity.Test 0.1 as UT
++
++Rectangle {
++    id: root
++    width: units.gu(20)
++    height: units.gu(10)
++    color: "black"
      RatingStars {
          id: ratingStars
--        maximumRating: parent.maximumRating
--        rating: parent.rating
--    }
--
--    function test_rating_init() {
--        compare(effectiveRating, rating, "EffectiveRating not initialized properly")
--    }
--
--    function test_rating_negative() {
--        rating = -3
--        compare(effectiveRating, 0, "EffectiveRating not calculated correctly")
--    }
--
--    function test_rating_set_ok() {
--        rating = 2
--        compare(effectiveRating, rating, "EffectiveRating not calculated correctly")
--    }
--
--    function test_rating_set_too_big() {
--        rating = 200
--        compare(effectiveRating, maximumRating, "EffectiveRating not calculated correctly")
--    }
--
--    function test_rating_set_min() {
--        rating = 0
--        compare(effectiveRating, 0, "EffectiveRating not calculated correctly")
--    }
--
--    function test_rating_set_max() {
--        rating = maximumRating
--        compare(effectiveRating, maximumRating, "EffectiveRating not calculated correctly")
++        anchors.centerIn: parent
++        maximumRating: 5
++        rating: 3
++    }
++
++    UT.UnityTestCase {
++        name: "RatingStars"
++        when: windowShown
++
++        function init() {
++            ratingStars.maximumRating = 5
++            ratingStars.rating = 3
++        }
++
++        function test_interactive_rating_data() {
++            return [
++                {tag: "first star without interactive", interactive: false, maximumRating: 5, index: 0, rating: 3},
++                {tag: "first star", interactive: true, maximumRating: 5, index: 0, rating: 1},
++                {tag: "second star with big maximumRating", interactive: true, maximumRating: 100, index: 1, rating: 40},
++                {tag: "last star", interactive: true, maximumRating: 5, index: 4, rating: 5},
++            ];
++        }
++
++        function test_interactive_rating(data) {
++            ratingStars.interactive = data.interactive
++            ratingStars.maximumRating = data.maximumRating
++
++            var ratingStar = findChild(ratingStars, "ratingStar"+data.index)
++            mouseClick(ratingStar, ratingStar.width / 2, ratingStar.height / 2)
++            compare(ratingStars.rating, data.rating)
++
++            ratingStars.interactive = false
++        }
++
++        function test_rating_init() {
++            compare(ratingStars.effectiveRating, ratingStars.rating, "EffectiveRating not initialized properly")
++        }
++
++        function test_rating_negative() {
++            ratingStars.rating = -3
++            compare(ratingStars.effectiveRating, 0, "EffectiveRating not calculated correctly")
++        }
++
++        function test_rating_set_ok() {
++            ratingStars.rating = 2
++            compare(ratingStars.effectiveRating, ratingStars.rating, "EffectiveRating not calculated correctly")
++        }
++
++        function test_rating_set_too_big() {
++            ratingStars.rating = 200
++            compare(ratingStars.effectiveRating, ratingStars.maximumRating, "EffectiveRating not calculated correctly")
++        }
++
++        function test_rating_set_min() {
++            ratingStars.rating = 0
++            compare(ratingStars.effectiveRating, 0, "EffectiveRating not calculated correctly")
++        }
++
++        function test_rating_set_max() {
++            ratingStars.rating = ratingStars.maximumRating
++            compare(ratingStars.effectiveRating, ratingStars.maximumRating, "EffectiveRating not calculated correctly")
++        }
+     }
+ }