Unity

Merge lp:~aacid/unity/filtergrid_remove_itemY into lp:unity/phablet

  1. filtergrid_remove_itemY
  2. Merge into phablet
Proposed by Albert Astals Cid
Status: Merged
Approved by: Albert Astals Cid
Approved revision: no longer in the source branch.
Merged at revision: 620
Proposed branch: lp:~aacid/unity/filtergrid_remove_itemY
Merge into: lp:unity/phablet
Prerequisite: lp:~aacid/unity/filtergrid_test_findChild
Diff against target: 144 lines (+13/-28)
6 files modified
Components/Carousel.qml (+2/-6)
Components/FilterGrid.qml (+0/-11)
Dash/DashPeople.qml (+4/-6)
Dash/DashVideos.qml (+1/-1)
Dash/People/PeopleFilterGrid.qml (+3/-2)
Dash/Video/VideosFilterGrid.qml (+3/-2)
To merge this branch: bzr merge lp:~aacid/unity/filtergrid_remove_itemY
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Michael Zanetti (community) Approve
Review via email: mp+160050@code.launchpad.net

Commit message

Remove the itemY function, we can just pass the item.y and use it

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:615
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/692/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/506
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/694
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/694/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/692
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/568
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/568/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/572
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1223

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/692/rebuild

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

PASSED: Continuous integration, rev:617
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/693/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/508
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/695
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/695/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/693
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/569
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/569/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/573
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1226

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/693/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

61 + effect.positionPx = mapToItem(categoryView, 0, itemY).y;
62 previewData.model = delegateItem.dataModel;
63 previewData.uri = delegateItem.dataModel.uri;

Hmm.. seeing this I'm not sure if my opinion is the good one any more. If we agree on a policy to not pass delegates around we should remove it also from the other 2 lines.

Approving as it gets rid of the todos and the change itself is clean.

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-phablet-autolanding/237/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/538
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/237
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/237/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-autolanding/237
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/171
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/171/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-autolanding/171
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1256

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-phablet-autolanding/238/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/541/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/238
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/238/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-autolanding/238
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/172
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/172/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-autolanding/172
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1259

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

Autolanding got stuck, reapproving

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-phablet-autolanding/239/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/542
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/239
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/239/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-autolanding/239
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/173/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-autolanding/173
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1261

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

Got stuck again

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-phablet-autolanding/240/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/547/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/240
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-autolanding/240/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-autolanding/240
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/174
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/174/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-autolanding/174
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1265

review: Needs Fixing (continuous-integration)
Revision history for this message
Albert Astals Cid (aacid) wrote :

And again...

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
1=== modified file 'Components/Carousel.qml'
2--- Components/Carousel.qml 2013-03-28 09:50:26 +0000
3+++ Components/Carousel.qml 2013-04-22 10:31:27 +0000
4@@ -29,14 +29,10 @@
5 property int cacheBuffer: 0
6 property real selectedItemScaleFactor: 1.1
7
8- signal clicked(int index, var delegateItem)
9+ signal clicked(int index, var delegateItem, real itemY)
10
11 implicitHeight: flickable.tileHeight * selectedItemScaleFactor
12
13- function itemY(index) {
14- return y;
15- }
16-
17 /* TODO: evaluate if the component could be more efficient with a ListView,
18 using this technique https://bugreports.qt-project.org/browse/QTBUG-29173 */
19
20@@ -103,7 +99,7 @@
21 /* We're clicking the selected item and
22 we're in the neighbourhood of radius 1 pixel from it.
23 Let's emit the clicked signal. */
24- carousel.clicked(index, delegateItem)
25+ carousel.clicked(index, delegateItem, delegateItem.y)
26 return
27 }
28
29
30=== modified file 'Components/FilterGrid.qml'
31--- Components/FilterGrid.qml 2013-04-22 10:31:27 +0000
32+++ Components/FilterGrid.qml 2013-04-22 10:31:27 +0000
33@@ -57,17 +57,6 @@
34
35 height: childrenRect.height
36
37- // TODO: Evaluate better ways how to know where to split to show the Preview
38- function itemX(index) {
39- var col = index % iconTileGrid.columns;
40- return (delegateWidth + horizontalCenter) * col;
41- }
42-
43- function itemY(index) {
44- var row = Math.floor(index / iconTileGrid.columns);
45- return (delegateHeight + verticalSpacing) * row;
46- }
47-
48 ResponsiveGridView {
49 id: iconTileGrid
50
51
52=== modified file 'Dash/DashPeople.qml'
53--- Dash/DashPeople.qml 2013-03-19 12:00:47 +0000
54+++ Dash/DashPeople.qml 2013-04-22 10:31:27 +0000
55@@ -128,8 +128,7 @@
56 property int categoryIndex
57
58 onClicked: {
59- // TODO: investigate in better solution than the itemY() getter.
60- effect.positionPx = mapToItem(categoryView, 0, peopleCarousel.itemY(index)).y;
61+ effect.positionPx = mapToItem(categoryView, 0, itemY).y;
62 previewData.model = delegateItem.dataModel;
63 previewData.uri = delegateItem.dataModel.uri;
64 previewLoader.open = true;
65@@ -145,15 +144,14 @@
66 property int categoryIndex
67
68 onClicked: {
69- // TODO: investigate in better solution than the itemY() getter.
70 if (peopleGrid.columnCount == 1) {
71 if (categoryIndex >= categoryView.model.count -1 && index >= peopleGrid.model.count - 1) {
72- effect.positionPx = mapToItem(categoryView, 0, peopleGrid.itemY(index)).y;
73+ effect.positionPx = mapToItem(categoryView, 0, itemY).y;
74 } else {
75- effect.positionPx = mapToItem(categoryView, 0, peopleGrid.itemY(index) + peopleGrid.cellHeight).y
76+ effect.positionPx = mapToItem(categoryView, 0, itemY + peopleGrid.cellHeight).y
77 }
78 } else {
79- effect.positionPx = mapToItem(categoryView, 0, peopleGrid.itemY(index)).y
80+ effect.positionPx = mapToItem(categoryView, 0, itemY).y
81 }
82 previewData.model = data;
83 previewData.uri = data.uri
84
85=== modified file 'Dash/DashVideos.qml'
86--- Dash/DashVideos.qml 2013-03-19 12:00:47 +0000
87+++ Dash/DashVideos.qml 2013-04-22 10:31:27 +0000
88@@ -121,7 +121,7 @@
89 if (item.column_5 != "") {
90 previewLoader.videoItem = loader.item.model.get(index);
91 previewLoader.open = true;
92- effect.positionPx = mapToItem(categoryView, 0, loader.item.itemY(index)).y;
93+ effect.positionPx = mapToItem(categoryView, 0, itemY).y;
94 }
95 }
96 }
97
98=== modified file 'Dash/People/PeopleFilterGrid.qml'
99--- Dash/People/PeopleFilterGrid.qml 2013-04-15 14:31:47 +0000
100+++ Dash/People/PeopleFilterGrid.qml 2013-04-22 10:31:27 +0000
101@@ -32,15 +32,16 @@
102
103 readonly property int columnCount: width / cellWidth
104
105- signal clicked(int index, variant data)
106+ signal clicked(int index, variant data, real itemY)
107
108 delegate: ListItems.Base {
109+ id: tile
110 objectName: "delegate" + index
111 width: filterGrid.cellWidth
112 showDivider: index < Math.floor((filterGrid.model.count-1) / filterGrid.columnCount) * filterGrid.columnCount
113
114 onClicked: {
115- filterGrid.clicked(index, data);
116+ filterGrid.clicked(index, data, tile.y);
117 }
118
119 Delegate {
120
121=== modified file 'Dash/Video/VideosFilterGrid.qml'
122--- Dash/Video/VideosFilterGrid.qml 2013-04-15 14:31:47 +0000
123+++ Dash/Video/VideosFilterGrid.qml 2013-04-22 10:31:27 +0000
124@@ -30,9 +30,10 @@
125 readonly property int iconWidth: (width / columns) * 0.8
126 readonly property int iconHeight: iconWidth * 16 / 11
127
128- signal clicked(int index)
129+ signal clicked(int index, real itemY)
130
131 delegate: Tile {
132+ id: tile
133 objectName: "delegate" + index
134 width: filtergrid.cellWidth
135 height: filtergrid.cellHeight
136@@ -42,7 +43,7 @@
137 source: model.column_1
138 fillMode: Image.PreserveAspectCrop
139 onClicked: {
140- filtergrid.clicked(index);
141+ filtergrid.clicked(index, tile.y);
142 }
143 }
144 }

Subscribers

People subscribed via source and target branches