Merge lp:~joergberroth/unav/20160328_fixes into lp:unav
- 20160328_fixes
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 8 |
Proposed branch: | lp:~joergberroth/unav/20160328_fixes |
Merge into: | lp:unav |
Diff against target: |
415 lines (+92/-73) 10 files modified
nav/index.html (+14/-8) qml/AboutPage.qml (+1/-1) qml/PoiListPage.qml (+1/-0) qml/PoiPage.qml (+1/-2) qml/RoutePage.qml (+1/-1) qml/SearchPage.qml (+1/-1) qml/SettingsPage.qml (+41/-28) qml/components/ExpandableListItem.qml (+2/-2) qml/components/HeaderListItem.qml (+0/-30) qml/components/ListItemHeader.qml (+30/-0) |
To merge this branch: | bzr merge lp:~joergberroth/unav/20160328_fixes |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Nekhelesh Ramananthan | qml code testing | Approve | |
Review via email: mp+290203@code.launchpad.net |
Commit message
*changed Naming of HeaderListItem.qml to ListItemHeader.qml and fixed it in PoiPage.qml, SettingsPage.qml, SearchPage.qml
*PoiListPage.qml: Added WordWrap to subtitles
*RoutePage.qml: reverted topMargin for Grid
**SettingsPage.qml: Made Design of Expandables more intuitive. minor design changes.
***index.html: comment the debug route simulation
Description of the change
The changes from yesterday against the latest branch.
With some more design change proposals to SettingsPage.qml
The way, the expandables expanded felt a bit diffuse. So, here is my proposal
Best Joerg
JkB (joergberroth) wrote : | # |
Am 2016-03-28 um 18:04 schrieb Nekhelesh Ramananthan:
> Review: Needs Fixing
>
> Hi Joerg, thanks for the fixes. They're mostly good except for some minor issues that I have listed below,
>
Hi, thx, too.
> 1. The listitem text is not vertically centered in the settings page. See http://
Ah yes, I see.
I also think we shouldn't change the left padding of the text since the
rest of the core apps don't do that.
Well, I think there is no clear separation than. The former approach had
a visible separation. dekko does it via checkboxes.
The only other app I see with the new expandable than is clock.
Try this in clock:
-expand a setting list
-focus another app
- refocus the setting page
...you are totaly lost as you don't get the states at first sight.
I think this is not the best approach.
I did some testing and this looks good -> http://
I could agree on that, too.
But, please re-think it on base of the expl. above.
Still, i like the indent more as it separates the list/action from the
"header".
>
> Here is the code diff, http://
>
> 2. The delete icon on the list item is also weird. I think the text "Clear History" makes it clear what the listitem action does.
I think the icon clarifys that there is not only text but an action to take.
I had this discussion with marcos, too.
As it is 1:2 now, i will leave it out so far, again ;-) .
>
> An example from the core app "Clock", we have a settings option "Change Timezone"..the text alone does the job.
Can not find this action.... ;-)
> 3. I already agreed to the name change of HeaderListItem. Just one minor thing, the next time you move/rename a file, please do "bzr mv oldfile newfile". That is more cleaner. For this MP it is good.
ah yes, I'll do.
Thanks.
Nekhelesh Ramananthan (nik90) wrote : | # |
> I also think we shouldn't change the left padding of the text since the
> rest of the core apps don't do that.
> Well, I think there is no clear separation than. The former approach had
> a visible separation. dekko does it via checkboxes.
> The only other app I see with the new expandable than is clock.
> Try this in clock:
> -expand a setting list
> -focus another app
> - refocus the setting page
> ...you are totaly lost as you don't get the states at first sight.
> I think this is not the best approach.
>
> I did some testing and this looks good -> http://
> I could agree on that, too.
> But, please re-think it on base of the expl. above.
> Still, i like the indent more as it separates the list/action from the
> "header".
Alright go with the indentation. But please use padding.leading: units.gu(1) to move the text. Otherwise it breaks the listitemlayout visually. In this case, you might also then want to indent the tick icon. SO add padding.trailing: units.gu(1). Please test this.
> > Here is the code diff, http://
> >
> > 2. The delete icon on the list item is also weird. I think the text "Clear
> History" makes it clear what the listitem action does./me
>
> I think the icon clarifys that there is not only text but an action to take.
> I had this discussion with marcos, too.
> As it is 1:2 now, i will leave it out so far, again ;-) .
Now that uNav is a core app shipped by default on the phone, it is even more important that it follows the design standards of other core apps like Dekko, Clock, Weather, Music app etc. None of those apps display such an icon in a list item. We want to maintain consistency. That's what I am trying to get at.
> >
> > An example from the core app "Clock", we have a settings option "Change
> Timezone"..the text alone does the job.
> Can not find this action.... ;-)
Its in the clock app settings page right at the bottom.
JkB (joergberroth) wrote : | # |
Am 2016-03-28 um 19:45 schrieb Nekhelesh Ramananthan:
>
> Alright go with the indentation. But please use padding.leading: units.gu(1) to move the text. Otherwise it breaks the listitemlayout visually. In this case, you might also then want to indent the tick icon. SO add padding.trailing: units.gu(1). Please test this.
Just updated the branch. Have a look. What do you think?
> Now that uNav is a core app shipped by default on the phone, it is even more important that it follows the design standards of other core apps like Dekko, Clock, Weather, Music app etc. None of those apps display such an icon in a list item. We want to maintain consistency. That's what I am trying to get at.
I fully see this!,
>>> An example from the core app "Clock", we have a settings option "Change
>> Timezone"..the text alone does the job.
>> Can not find this action.... ;-)
> Its in the clock app settings page right at the bottom.
but this action also (at least in the latest rc-prop version I am using)
has an arrow as action indication ( and leads me to the system-setting page)
>
Best Joerg
Nekhelesh Ramananthan (nik90) wrote : | # |
> > Alright go with the indentation. But please use padding.leading: units.gu(1)
> to move the text. Otherwise it breaks the listitemlayout visually. In this
> case, you might also then want to indent the tick icon. SO add
> padding.trailing: units.gu(1). Please test this.
>
> Just updated the branch. Have a look. What do you think?
I think visually it is okay, but I'm not happy with the code. Please use padding.leading and padding.trailing to adjust the indentation instead of anchors. Refer to SlotsLayout documentation [1] Also while running your code, I saw warnings like Reference error: Unit is not defined and so on.
Also since you hid the subtitle and the divider when its expanded, how about we remove the indentation and instead change the font color of the options shown?
[1] https:/
>
> > Now that uNav is a core app shipped by default on the phone, it is even more
> important that it follows the design standards of other core apps like Dekko,
> Clock, Weather, Music app etc. None of those apps display such an icon in a
> list item. We want to maintain consistency. That's what I am trying to get at.
>
> I fully see this!,
>
> >>> An example from the core app "Clock", we have a settings option "Change
> >> Timezone"..the text alone does the job.
> >> Can not find this action.... ;-)
> > Its in the clock app settings page right at the bottom.
> but this action also (at least in the latest rc-prop version I am using)
> has an arrow as action indication ( and leads me to the system-setting page)
>
Yes the arrow is a standard design pattern used to indicate that it opens a new page. Had we instead shown a dialog, we would not have used any icon there. I spoke to the canonical designers about this.
JkB (joergberroth) wrote : | # |
Am 2016-03-28 um 20:16 schrieb Nekhelesh Ramananthan:
>
> Also since you hid the subtitle and the divider when its expanded, how about we remove the indentation and instead change the font color of the options shown?
That does the trick. Branch updated
Nekhelesh Ramananthan (nik90) wrote : | # |
Looks great Joerg! Awesome work! One minor nitpick I have to point out,
why is the listview height (12+1)? It leaves 1 gu gap at the bottom that doesn't look so good :P. Can you remove that? Everything else is good.
JkB (joergberroth) wrote : | # |
My one designer eye ;-) says that it looks better with that gap.
Really, it separates it better to the following...
Am 2016-03-28 um 21:15 schrieb Nekhelesh Ramananthan:
> Review: Needs Fixing
>
> Looks great Joerg! Awesome work! One minor nitpick I have to point out,
>
> why is the listview height (12+1)? It leaves 1 gu gap at the bottom that doesn't look so good :P. Can you remove that? Everything else is good.
>
Nekhelesh Ramananthan (nik90) wrote : | # |
Alrite I give in ;-). Code looks good and works well. Approving. I will leave this in the hands of Marcos to review the index.html code change.
costales (costales) wrote : | # |
Thanks a lot Joerg |o/
The index.html is right for me :)
Merged!
Preview Diff
1 | === modified file 'nav/index.html' |
2 | --- nav/index.html 2016-03-26 18:53:17 +0000 |
3 | +++ nav/index.html 2016-03-28 19:01:41 +0000 |
4 | @@ -260,10 +260,12 @@ |
5 | if (!navigator.geolocation) { |
6 | $('#gps_denied').show(); |
7 | return; |
8 | - } |
9 | - |
10 | + } |
11 | |
12 | - // TODO release BEGIN remove debug |
13 | + /** |
14 | + // *TESTING* This block is for testing purposes only. It simulates a route |
15 | + // and should be removed before every release. |
16 | + // TODO release BEGIN remove debug |
17 | var route = [ |
18 | [43.51557,-5.655146666666667],[43.515764999999995,-5.655245000000001],[43.515955,-5.655354999999999],[43.51614333333334,-5.655476666666668],[43.51633,-5.655601666666667],[43.516515,-5.6557216666666665],[43.516695,-5.655835],[43.516871666666674,-5.655946666666667],[43.517039999999994,-5.656053333333333],[43.517208333333336,-5.6561650000000006],[43.517363333333336,-5.656270000000001],[43.517515,-5.656368333333334],[43.51765833333334,-5.656464999999999],[43.51779333333334,-5.6565666666666665],[43.51792333333333,-5.656670000000001],[43.518055000000004,-5.656765],[43.518188333333335,-5.656851666666667],[43.518323333333335,-5.656935000000001],[43.518458333333335,-5.657018333333333],[43.518591666666666,-5.657103333333334],[43.518724999999996,-5.657188333333333],[43.51885666666667,-5.657273333333334],[43.51898500000001,-5.657354999999999],[43.51910833333333,-5.6574366666666664],[43.51923,-5.657515000000001],[43.51935166666667,-5.657591666666667],[43.51946833333333,-5.657673333333334],[43.51958333333334,-5.65776],[43.51969166666666,-5.657848333333334],[43.519800000000004,-5.657933333333333],[43.519909999999996,-5.658021666666667],[43.520021666666665,-5.658115],[43.520134999999996,-5.658213333333333],[43.520224999999996,-5.658365],[43.520273333333336,-5.658556666666668],[43.520296666666674,-5.6587733333333325],[43.52031,-5.65899],[43.520329999999994,-5.659196666666666],[43.52035166666666,-5.6594],[43.52037833333334,-5.6595949999999995],[43.520405,-5.659795000000001],[43.520431666666674,-5.659996666666666],[43.52046166666667,-5.660196666666667],[43.52049999999999,-5.660386666666667],[43.52054,-5.660563333333332],[43.52058166666667,-5.660741666666667],[43.52062166666666,-5.6609266666666676],[43.52066833333333,-5.6611183333333335],[43.52072166666667,-5.661316666666666],[43.520786666666666,-5.661518333333333],[43.520865,-5.661720000000001],[43.52095333333333,-5.661916666666667],[43.521049999999995,-5.662096666666668],[43.521151666666675,-5.662258333333334],[43.52125833333333,-5.662405],[43.521363333333326,-5.662538333333333],[43.52146666666666,-5.662656666666667],[43.52155833333333,-5.662758333333333],[43.521634999999996,-5.66284],[43.52169666666667,-5.6629016666666665],[43.52174,-5.662941666666668],[43.52177,-5.66297],[43.52178833333334,-5.662984999999999],[43.52179666666666,-5.662993333333333],[43.5218,-5.6629950000000004],[43.5218,-5.662993333333333],[43.52182,-5.663011666666667],[43.52185166666666,-5.663041666666667],[43.521894999999994,-5.663076666666666],[43.521945,-5.6631149999999995],[43.52201,-5.663164999999999],[43.52208166666666,-5.663216666666667],[43.52216,-5.66327],[43.52225,-5.663326666666667],[43.52235166666666,-5.663381666666667],[43.52246333333333,-5.663433333333334],[43.52258,-5.663478333333334],[43.52269666666666,-5.663518333333333],[43.522816666666664,-5.66355],[43.52293666666666,-5.663576666666668],[43.52306,-5.663595],[43.52318166666667,-5.663603333333333],[43.523303333333324,-5.663605],[43.52342166666667,-5.663600000000001],[43.523541666666674,-5.663588333333333],[43.52366,-5.66357],[43.52378166666667,-5.663544999999999],[43.52390666666667,-5.6635133333333325],[43.52403833333333,-5.663475],[43.524175,-5.663433333333334],[43.524316666666664,-5.66339],[43.52445833333333,-5.6633466666666665],[43.52459833333334,-5.6633033333333325],[43.52474,-5.663258333333332],[43.52488166666667,-5.663213333333334],[43.52502333333334,-5.663170000000001],[43.52516833333334,-5.663128333333333],[43.525310000000005,-5.6630899999999995],[43.52545333333334,-5.663056666666667],[43.52559666666666,-5.663023333333332],[43.52574,-5.662986666666666],[43.52588333333333,-5.662945000000001],[43.52602666666667,-5.6629033333333325],[43.52617,-5.66286],[43.526309999999995,-5.662816666666667],[43.52645166666666,-5.662774999999999],[43.526595,-5.662733333333334],[43.526738333333334,-5.66269],[43.52688,-5.6626433333333335],[43.52702166666666,-5.662598333333333],[43.52716166666667,-5.662554999999999],[43.52729666666667,-5.662515],[43.52742,-5.662476666666667],[43.52753333333333,-5.662441666666666],[43.527635,-5.6624083333333335],[43.527725,-5.662374999999999],[43.52780500000001,-5.662344999999999],[43.52787,-5.662319999999999],[43.527925,-5.662303333333334],[43.52797166666666,-5.66229],[43.528009999999995,-5.662276666666666],[43.52804166666667,-5.662266666666667],[43.52806666666667,-5.662256666666667],[43.52807833333333,-5.66225],[43.52808166666667,-5.66225],[43.52808,-5.662248333333334],[43.52807833333333,-5.6622466666666655] |
19 | ]; |
20 | @@ -336,10 +338,9 @@ |
21 | },800); |
22 | return; |
23 | // TODO release END remove debug |
24 | - |
25 | - |
26 | - |
27 | - |
28 | + // *TESTING* End |
29 | + **/ |
30 | + |
31 | id_gps = navigator.geolocation.watchPosition( |
32 | function (pos) { |
33 | |
34 | @@ -451,7 +452,10 @@ |
35 | longpress = (endTime - startTime < 225) ? false : true; |
36 | }); |
37 | |
38 | - // TODO release BEGIN remove debug |
39 | + /** |
40 | + // *TESTING* This block is for testing purposes only. It simulates a route |
41 | + // and should be removed before every release. |
42 | + // TODO release BEGIN remove debug |
43 | $("#map").on('mousedown', function () { |
44 | startTime = new Date().getTime(); |
45 | }); |
46 | @@ -460,6 +464,8 @@ |
47 | longpress = (endTime - startTime < 225) ? false : true; |
48 | }); |
49 | // TODO release END remove debug |
50 | + //*TESTING* END |
51 | + **/ |
52 | |
53 | // Drag |
54 | map.on('pointerdrag', function(evt) { |
55 | |
56 | === modified file 'qml/AboutPage.qml' |
57 | --- qml/AboutPage.qml 2016-03-26 18:53:17 +0000 |
58 | +++ qml/AboutPage.qml 2016-03-28 19:01:41 +0000 |
59 | @@ -100,7 +100,7 @@ |
60 | anchors.fill: parent |
61 | section.property: "category" |
62 | section.criteria: ViewSection.FullString |
63 | - section.delegate: HeaderListItem { |
64 | + section.delegate: ListItemHeader { |
65 | title: section |
66 | } |
67 | |
68 | |
69 | === modified file 'qml/PoiListPage.qml' |
70 | --- qml/PoiListPage.qml 2016-03-26 21:16:49 +0000 |
71 | +++ qml/PoiListPage.qml 2016-03-28 19:01:41 +0000 |
72 | @@ -326,6 +326,7 @@ |
73 | subtitle.text: model.openinghours !== "" ? "%1 | %2".arg(getOpeningHours(model.openinghours)).arg(QmlJs.formatDistance(model.distance, navApp.settings.unit)) |
74 | : QmlJs.formatDistance(model.distance, navApp.settings.unit) |
75 | subtitle.maximumLineCount: 2 |
76 | + subtitle.wrapMode: Text.WordWrap |
77 | summary.text: model.description |
78 | summary.maximumLineCount: 3 |
79 | |
80 | |
81 | === modified file 'qml/PoiPage.qml' |
82 | --- qml/PoiPage.qml 2016-03-27 10:05:20 +0000 |
83 | +++ qml/PoiPage.qml 2016-03-28 19:01:41 +0000 |
84 | @@ -157,7 +157,7 @@ |
85 | width: parent.width |
86 | height: sectionHeader.height |
87 | |
88 | - HeaderListItem { |
89 | + ListItemHeader { |
90 | id: sectionHeader |
91 | title: section |
92 | } |
93 | @@ -191,4 +191,3 @@ |
94 | } |
95 | } |
96 | |
97 | - |
98 | |
99 | === modified file 'qml/RoutePage.qml' |
100 | --- qml/RoutePage.qml 2016-03-26 18:53:17 +0000 |
101 | +++ qml/RoutePage.qml 2016-03-28 19:01:41 +0000 |
102 | @@ -31,7 +31,7 @@ |
103 | Flickable { |
104 | id: flickable |
105 | |
106 | - anchors { fill: parent; margins: units.gu(2) } |
107 | + anchors { fill: parent; margins: units.gu(2); topMargin: units.gu(6) } |
108 | height: contentItem.childrenRect.height |
109 | boundsBehavior: (contentHeight > routePage.height) ? Flickable.DragAndOvershootBounds : Flickable.StopAtBounds |
110 | |
111 | |
112 | === modified file 'qml/SearchPage.qml' |
113 | --- qml/SearchPage.qml 2016-03-26 18:53:17 +0000 |
114 | +++ qml/SearchPage.qml 2016-03-28 19:01:41 +0000 |
115 | @@ -230,7 +230,7 @@ |
116 | |
117 | section.property: "title" |
118 | section.criteria: ViewSection.FullString |
119 | - section.delegate: HeaderListItem { |
120 | + section.delegate: ListItemHeader { |
121 | title: section |
122 | } |
123 | |
124 | |
125 | === modified file 'qml/SettingsPage.qml' |
126 | --- qml/SettingsPage.qml 2016-03-26 18:53:17 +0000 |
127 | +++ qml/SettingsPage.qml 2016-03-28 19:01:41 +0000 |
128 | @@ -40,7 +40,7 @@ |
129 | navigationModeModel.append({ "mode": i18n.tr("Walking"), "index": 1 }) |
130 | navigationModeModel.append({ "mode": i18n.tr("Bicycle"), "index": 2 }) |
131 | |
132 | - _modeList.subText.text = navigationModeModel.get(navApp.settings.routingMode).mode |
133 | + modeList.subText.text = navigationModeModel.get(navApp.settings.routingMode).mode |
134 | } |
135 | } |
136 | |
137 | @@ -52,7 +52,7 @@ |
138 | soundModel.append({ "sound": i18n.tr("A Notification"), "index": 1 }) |
139 | soundModel.append({ "sound": i18n.tr("None"), "index": 2 }) |
140 | |
141 | - _soundList.subText.text = soundModel.get(navApp.settings.soundIndications).sound |
142 | + soundList.subText.text = soundModel.get(navApp.settings.soundIndications).sound |
143 | } |
144 | } |
145 | |
146 | @@ -63,7 +63,7 @@ |
147 | unitModel.append({ "unit": i18n.tr("Kilometres"), "index": 0 }) |
148 | unitModel.append({ "unit": i18n.tr("Miles"), "index": 1 }) |
149 | |
150 | - _unitList.subText.text = unitModel.get(navApp.settings.unit).unit |
151 | + unitList.subText.text = unitModel.get(navApp.settings.unit).unit |
152 | } |
153 | } |
154 | |
155 | @@ -81,22 +81,30 @@ |
156 | right: parent.right |
157 | } |
158 | |
159 | + ListItemHeader { |
160 | + id: navigationListHeader |
161 | + title: i18n.tr("Navigation") |
162 | + } |
163 | + |
164 | ExpandableListItem { |
165 | - id: _modeList |
166 | + id: modeList |
167 | |
168 | - listViewHeight: units.gu(21) |
169 | + listViewHeight: units.gu(12+1) |
170 | titleText.text: i18n.tr("Navigation Mode") |
171 | |
172 | model: navigationModeModel |
173 | |
174 | delegate: ListItem { |
175 | + divider.visible: false |
176 | + height: navigationListItemLayout.height |
177 | ListItemLayout { |
178 | + id: navigationListItemLayout |
179 | title.text: model.mode |
180 | - |
181 | + title.color: "#5D5D5D" |
182 | + padding { top: units.gu(1); bottom: units.gu(1) } |
183 | Icon { |
184 | SlotsLayout.position: SlotsLayout.Trailing |
185 | width: units.gu(2) |
186 | - height: width |
187 | name: "tick" |
188 | visible: navApp.settings.routingMode === model.index |
189 | } |
190 | @@ -105,8 +113,8 @@ |
191 | onClicked: { |
192 | navApp.settings.routingMode = model.index |
193 | mainPageStack.executeJavaScript("if (nav.get_route_status() != 'no' && !nav.get_route_status().startsWith('simulate')){nav.set_route_status('waiting4signal')}; settings.set_routing_mode(" + model.index +");") |
194 | - _modeList.subText.text = navigationModeModel.get(navApp.settings.routingMode).mode |
195 | - _modeList.toggleExpansion() |
196 | + modeList.subText.text = navigationModeModel.get(navApp.settings.routingMode).mode |
197 | + modeList.toggleExpansion() |
198 | } |
199 | } |
200 | } |
201 | @@ -152,21 +160,24 @@ |
202 | } |
203 | |
204 | ExpandableListItem { |
205 | - id: _soundList |
206 | + id: soundList |
207 | |
208 | - listViewHeight: units.gu(21) |
209 | + listViewHeight: units.gu(12+1) |
210 | titleText.text: i18n.tr("Guidance") |
211 | |
212 | model: soundModel |
213 | |
214 | delegate: ListItem { |
215 | + divider.visible: false |
216 | + height: soundListItemLayout.height |
217 | ListItemLayout { |
218 | + id: soundListItemLayout |
219 | title.text: model.sound |
220 | - |
221 | + title.color: "#5D5D5D" |
222 | + padding { top: units.gu(1); bottom: units.gu(1) } |
223 | Icon { |
224 | SlotsLayout.position: SlotsLayout.Trailing |
225 | width: units.gu(2) |
226 | - height: width |
227 | name: "tick" |
228 | visible: navApp.settings.soundIndications === model.index |
229 | } |
230 | @@ -175,33 +186,36 @@ |
231 | onClicked: { |
232 | navApp.settings.soundIndications = model.index |
233 | mainPageStack.executeJavaScript("settings.set_sound(" + model.index + ");") |
234 | - _soundList.subText.text = soundModel.get(navApp.settings.soundIndications).sound |
235 | - _soundList.toggleExpansion() |
236 | + soundList.subText.text = soundModel.get(navApp.settings.soundIndications).sound |
237 | + soundList.toggleExpansion() |
238 | } |
239 | } |
240 | } |
241 | |
242 | - HeaderListItem { |
243 | + ListItemHeader { |
244 | id: mapListHeader |
245 | title: i18n.tr("Map") |
246 | } |
247 | |
248 | ExpandableListItem { |
249 | - id: _unitList |
250 | + id: unitList |
251 | |
252 | - listViewHeight: units.gu(14) |
253 | + listViewHeight: units.gu(8+1) |
254 | titleText.text: i18n.tr("Units") |
255 | |
256 | model: unitModel |
257 | |
258 | delegate: ListItem { |
259 | + divider.visible: false |
260 | + height: unitListItemLayout.height |
261 | ListItemLayout { |
262 | + id: unitListItemLayout |
263 | title.text: model.unit |
264 | - |
265 | + title.color: "#5D5D5D" |
266 | + padding { top: units.gu(1); bottom: units.gu(1) } |
267 | Icon { |
268 | SlotsLayout.position: SlotsLayout.Trailing |
269 | width: units.gu(2) |
270 | - height: width |
271 | name: "tick" |
272 | visible: navApp.settings.unit === model.index |
273 | } |
274 | @@ -211,13 +225,13 @@ |
275 | navApp.settings.unit = model.index |
276 | mainPageStack.executeJavaScript("settings.set_unit(\'" + ( model.index === 0 ? "km" : "mi" ) +"\');") |
277 | mainPageStack.executeJavaScript("ui.set_scale_unit(\'" + ( navApp.settings.unit === 0 ? "km" : "mi" ) +"\');") |
278 | - _unitList.subText.text = unitModel.get(navApp.settings.unit).unit |
279 | - _unitList.toggleExpansion() |
280 | + unitList.subText.text = unitModel.get(navApp.settings.unit).unit |
281 | + unitList.toggleExpansion() |
282 | } |
283 | } |
284 | } |
285 | |
286 | - HeaderListItem { |
287 | + ListItemHeader { |
288 | id: privacyListHeader |
289 | title: i18n.tr("History") |
290 | } |
291 | @@ -237,9 +251,8 @@ |
292 | } |
293 | |
294 | ListItem { |
295 | - Label { |
296 | - anchors { verticalCenter: parent.verticalCenter; left: parent.left; leftMargin: units.gu(2) } |
297 | - text: i18n.tr("Clear History") |
298 | + ListItemLayout { |
299 | + title.text: i18n.tr("Clear History") |
300 | } |
301 | onClicked: PopupUtils.open(confirmEraseHistory) |
302 | } |
303 | @@ -251,6 +264,7 @@ |
304 | id: dialogueErase |
305 | title: i18n.tr("Clear History") |
306 | text: i18n.tr("You'll delete the current history") |
307 | + |
308 | Button { |
309 | text: i18n.tr("Delete") |
310 | color: UbuntuColors.red |
311 | @@ -259,6 +273,7 @@ |
312 | PopupUtils.close(dialogueErase); |
313 | } |
314 | } |
315 | + |
316 | Button { |
317 | text: i18n.tr("Cancel") |
318 | onClicked: PopupUtils.close(dialogueErase) |
319 | @@ -279,7 +294,5 @@ |
320 | } |
321 | } |
322 | } |
323 | - |
324 | } |
325 | } |
326 | - |
327 | |
328 | === modified file 'qml/components/ExpandableListItem.qml' |
329 | --- qml/components/ExpandableListItem.qml 2016-03-26 18:53:17 +0000 |
330 | +++ qml/components/ExpandableListItem.qml 2016-03-28 19:01:41 +0000 |
331 | @@ -45,12 +45,12 @@ |
332 | ListItem { |
333 | id: headerListItem |
334 | height: expandableHeader.height + divider.height |
335 | - |
336 | + divider.visible: false |
337 | ListItemLayout { |
338 | id: expandableHeader |
339 | |
340 | subtitle.textSize: Label.Medium |
341 | - |
342 | + subtitle.visible: !expansion.expanded |
343 | Icon { |
344 | id: arrow |
345 | |
346 | |
347 | === removed file 'qml/components/HeaderListItem.qml' |
348 | --- qml/components/HeaderListItem.qml 2016-03-26 18:53:17 +0000 |
349 | +++ qml/components/HeaderListItem.qml 1970-01-01 00:00:00 +0000 |
350 | @@ -1,30 +0,0 @@ |
351 | -/* |
352 | - * uNav http://launchpad.net/unav |
353 | - * Copyright (C) 2016 Nekhelesh Ramananthan https://launchpad.net/~nik90 |
354 | - * |
355 | - * uNav is free software; you can redistribute it and/or modify |
356 | - * it under the terms of the GNU General Public License as published by |
357 | - * the Free Software Foundation; either version 3 of the License, or |
358 | - * (at your option) any later version. |
359 | - * |
360 | - * uNav is distributed in the hope that it will be useful, |
361 | - * but WITHOUT ANY WARRANTY; without even the implied warranty of |
362 | - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
363 | - * GNU General Public License for more details. |
364 | - */ |
365 | - |
366 | -import QtQuick 2.4 |
367 | -import Ubuntu.Components 1.3 |
368 | - |
369 | -ListItem { |
370 | - id: headerListItem |
371 | - |
372 | - property string title |
373 | - |
374 | - height: units.gu(4) |
375 | - Label { |
376 | - anchors { verticalCenter: parent.verticalCenter; left: parent.left; leftMargin: units.gu(2) } |
377 | - text: title |
378 | - font.bold: true |
379 | - } |
380 | -} |
381 | |
382 | === added file 'qml/components/ListItemHeader.qml' |
383 | --- qml/components/ListItemHeader.qml 1970-01-01 00:00:00 +0000 |
384 | +++ qml/components/ListItemHeader.qml 2016-03-28 19:01:41 +0000 |
385 | @@ -0,0 +1,30 @@ |
386 | +/* |
387 | + * uNav http://launchpad.net/unav |
388 | + * Copyright (C) 2016 Nekhelesh Ramananthan https://launchpad.net/~nik90 |
389 | + * |
390 | + * uNav is free software; you can redistribute it and/or modify |
391 | + * it under the terms of the GNU General Public License as published by |
392 | + * the Free Software Foundation; either version 3 of the License, or |
393 | + * (at your option) any later version. |
394 | + * |
395 | + * uNav is distributed in the hope that it will be useful, |
396 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
397 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
398 | + * GNU General Public License for more details. |
399 | + */ |
400 | + |
401 | +import QtQuick 2.4 |
402 | +import Ubuntu.Components 1.3 |
403 | + |
404 | +ListItem { |
405 | + id: headerListItem |
406 | + |
407 | + property string title |
408 | + |
409 | + height: units.gu(4) |
410 | + Label { |
411 | + anchors { verticalCenter: parent.verticalCenter; left: parent.left; leftMargin: units.gu(2) } |
412 | + text: title |
413 | + font.bold: false |
414 | + } |
415 | +} |
Hi Joerg, thanks for the fixes. They're mostly good except for some minor issues that I have listed below,
1. The listitem text is not vertically centered in the settings page. See http:// imgur.com/ 2ccNEsq. The tick padding is also incorrect. I also think we shouldn't change the left padding of the text since the rest of the core apps don't do that. I did some testing and this looks good -> http:// imgur.com/ AzrnFAE
Here is the code diff, http:// paste.ubuntu. com/15540311/
2. The delete icon on the list item is also weird. I think the text "Clear History" makes it clear what the listitem action does.
An example from the core app "Clock", we have a settings option "Change Timezone"..the text alone does the job.
3. I already agreed to the name change of HeaderListItem. Just one minor thing, the next time you move/rename a file, please do "bzr mv oldfile newfile". That is more cleaner. For this MP it is good.
4. I don't have any opinion on the index.html file change. I leave that to Marcos.