Merge lp:~jindallo/nearby-scope/convergence-ui-support into lp:nearby-scope

Proposed by Jin on 2016-07-12
Status: Merged
Approved by: Jin on 2016-08-03
Approved revision: 100
Merged at revision: 102
Proposed branch: lp:~jindallo/nearby-scope/convergence-ui-support
Merge into: lp:nearby-scope
Diff against target: 257 lines (+28/-28) (has conflicts)
1 file modified
click-src/aggregator/child_scopes.json (+28/-28)
Contents conflict in click-src/aggregator/libcom.canonical.scopes.aggregator_aggregator.so
To merge this branch: bzr merge lp:~jindallo/nearby-scope/convergence-ui-support
Reviewer Review Type Date Requested Status
Kyle Nitzsche 2016-07-12 Approve on 2016-08-02
NearBy Scope Team 2016-07-12 Pending
Review via email: mp+299778@code.launchpad.net
To post a comment you must log in.
100. By Jin on 2016-07-19

Library rebased to 4.9 for NearBy aggregator

Kyle Nitzsche (knitzsche) wrote :

Hi Jin,

I assume you tested:
* in a fake location (like London) that has full results
* on tablet landscape & portrait
* on something small (like krillin)

Kyle Nitzsche (knitzsche) wrote :

Hi Jin,

The general approach we discussed is to:
* use high cardinalities to ensure their are enough results to fill in the rows
* limit vertical space in different layouts with collapsed-rows

It seems that this often takes a different approach:
* set collapsed-rows to 0 (which IIRC means show them all without limit)
* limit the number of results with cardinality

Is there a reason you did it this way? Or would you consider doing it the way we discussed?

cheers

Jin (jindallo) wrote :

Hello Kyle,

I used the way we discussed/agreed to implement this patch,
which means:
    1. set collapsed-rows to 0 -> 1
    2. set cardinality to big enough -> 10

Please kindly have a look on below diff,
it followed the approach we discussed.

Many thanks!

Kyle Nitzsche (knitzsche) wrote :

Looks good Jin. Thanks.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'click-src/aggregator/child_scopes.json'
2--- click-src/aggregator/child_scopes.json 2015-12-24 05:43:05 +0000
3+++ click-src/aggregator/child_scopes.json 2016-07-19 01:18:48 +0000
4@@ -14,7 +14,7 @@
5 "card-layout": "vertical",
6 "card-size": "medium",
7 "category-layout": "grid",
8- "collapsed-rows": 0
9+ "collapsed-rows": 1
10 }
11 }
12 },
13@@ -29,7 +29,7 @@
14 "template": {
15 "card-size": "small",
16 "category-layout": "grid",
17- "collapsed-rows": 0
18+ "collapsed-rows": 1
19 }
20 }
21 },
22@@ -44,7 +44,7 @@
23 "template": {
24 "card-size": "small",
25 "category-layout": "grid",
26- "collapsed-rows": 0
27+ "collapsed-rows": 1
28 }
29 }
30 },
31@@ -91,7 +91,7 @@
32 "template": {
33 "card-size": "small",
34 "category-layout": "grid",
35- "collapsed-rows": 0
36+ "collapsed-rows": 1
37 }
38 }
39 },
40@@ -108,7 +108,7 @@
41 "template": {
42 "card-size": "small",
43 "category-layout": "grid",
44- "collapsed-rows": 0
45+ "collapsed-rows": 1
46 }
47 }
48 },
49@@ -174,7 +174,7 @@
50 "template": {
51 "card-size": "small",
52 "category-layout": "grid",
53- "collapsed-rows": 0
54+ "collapsed-rows": 1
55 }
56 }
57 }
58@@ -230,7 +230,7 @@
59 "scope": {
60 "_category_title": "Weather from The Weather Channel",
61 "department": "dept-how",
62- "cardinality": 3,
63+ "cardinality": 10,
64 "id": "com.canonical.scopes.weatherchannel",
65 "link_to_child": "true",
66 "local_id": "twc1",
67@@ -252,7 +252,7 @@
68 {
69 "scope": {
70 "_category_title": "Landmarks around you from Yelp",
71- "cardinality": 6,
72+ "cardinality": 10,
73 "child_department": "landmarks",
74 "department": "dept-how",
75 "id": "com.canonical.scopes.yelp_yelp",
76@@ -264,7 +264,7 @@
77 {
78 "scope": {
79 "_category_title": "Closest from Yelp",
80- "cardinality": 4,
81+ "cardinality": 10,
82 "department": "dept-how",
83 "id": "com.canonical.scopes.yelp_yelp",
84 "link_to_child": "true",
85@@ -281,7 +281,7 @@
86 {
87 "scope": {
88 "_category_title": "NearBy from Flickr",
89- "cardinality": 9,
90+ "cardinality": 10,
91 "department": "dept-how",
92 "id": "com.canonical.scopes.flickr_nearby",
93 "link_to_child": "true",
94@@ -330,7 +330,7 @@
95 {
96 "scope": {
97 "_category_title": "Weather from The Weather Channel",
98- "cardinality": 3,
99+ "cardinality": 10,
100 "department": "dept-bored",
101 "id": "com.canonical.scopes.weatherchannel",
102 "link_to_child": "true",
103@@ -353,7 +353,7 @@
104 {
105 "scope": {
106 "_category_title": "Landmarks around you from Yelp",
107- "cardinality": 6,
108+ "cardinality": 10,
109 "child_department": "landmarks",
110 "department": "dept-bored",
111 "id": "com.canonical.scopes.yelp_yelp",
112@@ -365,7 +365,7 @@
113 {
114 "scope": {
115 "_category_title": "Closest from Yelp",
116- "cardinality": 4,
117+ "cardinality": 10,
118 "department": "dept-bored",
119 "id": "com.canonical.scopes.yelp_yelp",
120 "link_to_child": "true",
121@@ -382,7 +382,7 @@
122 {
123 "scope": {
124 "_category_title": "NearBy from Flickr",
125- "cardinality": 9,
126+ "cardinality": 10,
127 "department": "dept-bored",
128 "id": "com.canonical.scopes.flickr_nearby",
129 "link_to_child": "true",
130@@ -431,7 +431,7 @@
131 {
132 "scope": {
133 "_category_title": "Weather from The Weather Channel",
134- "cardinality": 3,
135+ "cardinality": 10,
136 "department": "dept-onthemove",
137 "id": "com.canonical.scopes.weatherchannel",
138 "link_to_child": "true",
139@@ -510,14 +510,14 @@
140 "id": "com.canonical.scopes.poi_poi",
141 "link_to_child": "true",
142 "local_id": "poi-atm",
143- "cardinality": 6,
144+ "cardinality": 10,
145 "renderer_common_id": "point-of-interest"
146 }
147 },
148 {
149 "scope": {
150 "_category_title": "Traffic cameras around you",
151- "cardinality": 3,
152+ "cardinality": 10,
153 "child_department": "Cameras",
154 "department": "dept-onthemove",
155 "id": "com.canonical.scopes.inrix_inrix",
156@@ -534,7 +534,7 @@
157 "template": {
158 "card-size": "small",
159 "category-layout": "grid",
160- "collapsed-rows": 0
161+ "collapsed-rows": 1
162 }
163 }
164 }
165@@ -579,7 +579,7 @@
166 {
167 "scope": {
168 "_category_title": "ATMs near you",
169- "cardinality": 3,
170+ "cardinality": 10,
171 "child_department": "atm",
172 "department": "dept-hungry",
173 "id": "com.canonical.scopes.poi_poi",
174@@ -610,7 +610,7 @@
175 {
176 "scope": {
177 "_category_title": "Bars from Yelp",
178- "cardinality": 4,
179+ "cardinality": 10,
180 "child_department": "bars",
181 "department": "dept-thirsty",
182 "id": "com.canonical.scopes.yelp_yelp",
183@@ -628,7 +628,7 @@
184 {
185 "scope": {
186 "_category_title": "Coffee and Tea from Yelp",
187- "cardinality": 4,
188+ "cardinality": 10,
189 "child_department": "coffee",
190 "department": "dept-thirsty",
191 "id": "com.canonical.scopes.yelp_yelp",
192@@ -646,7 +646,7 @@
193 {
194 "scope": {
195 "_category_title": "ATMs near you",
196- "cardinality": 3,
197+ "cardinality": 10,
198 "child_department": "atm",
199 "department": "dept-thirsty",
200 "id": "com.canonical.scopes.poi_poi",
201@@ -665,7 +665,7 @@
202 {
203 "scope": {
204 "_category_title": "Weather from The Weather Channel",
205- "cardinality": 3,
206+ "cardinality": 10,
207 "department": "dept-stressed",
208 "id": "com.canonical.scopes.weatherchannel",
209 "link_to_child": "true",
210@@ -688,7 +688,7 @@
211 {
212 "scope": {
213 "_category_title": "Spas from Yelp",
214- "cardinality": 4,
215+ "cardinality": 10,
216 "child_department": "beautysvc",
217 "department": "dept-stressed",
218 "id": "com.canonical.scopes.yelp_yelp",
219@@ -706,7 +706,7 @@
220 {
221 "scope": {
222 "_category_title": "Fashion from Yelp",
223- "cardinality": 4,
224+ "cardinality": 10,
225 "child_department": "fashion",
226 "department": "dept-stressed",
227 "id": "com.canonical.scopes.yelp_yelp",
228@@ -724,7 +724,7 @@
229 {
230 "scope": {
231 "_category_title": "ATMs near you",
232- "cardinality": 3,
233+ "cardinality": 10,
234 "child_department": "atm",
235 "department": "dept-stressed",
236 "id": "com.canonical.scopes.poi_poi",
237@@ -736,7 +736,7 @@
238 {
239 "scope": {
240 "_category_title": "Games to let off some steam",
241- "cardinality": 3,
242+ "cardinality": 10,
243 "child_department": "games",
244 "department": "dept-stressed",
245 "id": "com.canonical.scopes.clickstore",
246@@ -748,7 +748,7 @@
247 {
248 "scope": {
249 "_category_title": "Relax with some music",
250- "cardinality": 3,
251+ "cardinality": 10,
252 "child_department": "genre-ambient",
253 "department": "dept-stressed",
254 "id": "com.canonical.scopes.sevendigital",
255
256=== renamed file 'click-src/aggregator/libcom.canonical.scopes.aggregator_aggregator.so' => 'click-src/aggregator/libcom.canonical.scopes.aggregator_aggregator.so.OTHER' (properties changed: +x to -x)
257Binary files click-src/aggregator/libcom.canonical.scopes.aggregator_aggregator.so 2016-07-13 15:33:23 +0000 and click-src/aggregator/libcom.canonical.scopes.aggregator_aggregator.so.OTHER 2016-07-19 01:18:48 +0000 differ

Subscribers

People subscribed via source and target branches

to all changes: