Merge lp:~joergberroth/unav/format_fixes_PoiListpage into lp:unav

[JkB]

*from trailing to last

small format fixes for icon in PoiListpage

[Nekhelesh Ramananthan (nik90)]

I think the icon size fix is a good idea. I noticed that issue when I was casually testing. Nice catch Joerg!. One thing however, I would keep SlotsLayout.position: SlotsLayout.Last since the icon is meant to be shown last in the listitem.

https://developer.ubuntu.com/api/apps/qml/sdk-15.04.1/Ubuntu.Components.SlotsLayout/

review: Needs Fixing

[JkB (joergberroth)]

Yes, but than the icon seems not fixed in position for last and will vary !
So this at least fixes the position and i don't see why trailling should
not do the trick

Best Joerg

Am 2016-04-03 um 11:56 schrieb Nekhelesh Ramananthan:
> Review: Needs Fixing
>
> I think the icon size fix is a good idea. I noticed that issue when I was casually testing. Nice catch Joerg!. One thing however, I would keep SlotsLayout.position: SlotsLayout.Last since the icon is meant to be shown last in the listitem.
>
> https://developer.ubuntu.com/api/apps/qml/sdk-15.04.1/Ubuntu.Components.SlotsLayout/
>

[Nekhelesh Ramananthan (nik90)]

Well for starters it should be SlotsLayout.Trailing (with one L). And secondly according to the documentation, SlotsLayout.Last ensures that the icon doesn't move and is always the last item on the listitem. While SlotsLayout.Trailing allows the icon to just be on the trailing slot and move if more elements are added.

In trunk, the icon *appears* to move because of the variable width that I set..not because of the slots positioning.

review: Needs Fixing

[JkB (joergberroth)]

So, as long as nothing is added, trailling is fine ;-) and if something
will be added it has to be reevaluated still.
m Sonntag, 3. April 2016 12:14:39 CEST schrieb Nekhelesh Ramananthan
<email address hidden>:
> Review: Needs Fixing
>
> Well for starters it should be SlotsLayout.Trailing (with one
> L). And secondly according to the documentation,
> SlotsLayout.Last ensures that the icon doesn't move and is
> always the last item on the listitem. While SlotsLayout.Trailing
> allows the icon to just be on the trailing slot and move if more
> elements are added.
>
> In trunk, the icon *appears* to move because of the variable
> width that I set..not because of the slots positioning.

--
Versandt, mit Dekko von meinem Ubuntu-Gerät

[Nekhelesh Ramananthan (nik90)]

> So, as long as nothing is added, trailling is fine ;-) and if something
> will be added it has to be reevaluated still.
> m Sonntag, 3. April 2016 12:14:39 CEST schrieb Nekhelesh Ramananthan
> <email address hidden>:
> > Review: Needs Fixing
> >
> > Well for starters it should be SlotsLayout.Trailing (with one
> > L). And secondly according to the documentation,
> > SlotsLayout.Last ensures that the icon doesn't move and is
> > always the last item on the listitem. While SlotsLayout.Trailing
> > allows the icon to just be on the trailing slot and move if more
> > elements are added.
> >
> > In trunk, the icon *appears* to move because of the variable
> > width that I set..not because of the slots positioning.
>

Just curious, but what do you have against using SlotsLayouts.Last? By using it, we can ensure the icon is always the last position regardless of any code changes in the future. We do want the icon to be always the last position, no?
>
>
> --
> Versandt, mit Dekko von meinem Ubuntu-Gerät

[JkB (joergberroth)]

Am 2016-04-03 um 12:39 schrieb Nekhelesh Ramananthan:
>> So, as long as nothing is added, trailling is fine ;-) and if something
>> will be added it has to be reevaluated still.
>> m Sonntag, 3. April 2016 12:14:39 CEST schrieb Nekhelesh Ramananthan
>> <email address hidden>:
>>> Review: Needs Fixing
>>>
>>> Well for starters it should be SlotsLayout.Trailing (with one
>>> L). And secondly according to the documentation,
>>> SlotsLayout.Last ensures that the icon doesn't move and is
>>> always the last item on the listitem. While SlotsLayout.Trailing
>>> allows the icon to just be on the trailing slot and move if more
>>> elements are added.
>>>
>>> In trunk, the icon *appears* to move because of the variable
>>> width that I set..not because of the slots positioning.
>>
>
> Just curious, but what do you have against using SlotsLayouts.Last? By using it, we can ensure the icon is always the last position regardless of any code changes in the future. We do want the icon to be always the last position, no?
Do you know this, yet? ;-)

For me it looks like this is clear code without having to tweak
anything. if I put last, I have to tweak the main slot to get a fixed
position. Why ever...
and if there will be additional trailing features, you will have to
decide than which trailing will be last as you can not foresee this now.
>>
>>
>> --
>> Versandt, mit Dekko von meinem Ubuntu-Gerät

[Nekhelesh Ramananthan (nik90)]

Fine whatever.. Could you atleast then fix the spelling mistake of SlotsLayout.Trailling.

[Nekhelesh Ramananthan (nik90)]

FYI you wouldn't have to tweak the main slot positioning. And for 0.58 we know the icon is going to be last always. So yes I know it :)

[JkB (joergberroth)]

just changed it to last.
Seems to work "now". Maybe some eye sand in the morning. ;-)

Best Joerg

Am 2016-04-03 um 12:53 schrieb Nekhelesh Ramananthan:
> FYI you wouldn't have to tweak the main slot positioning. And for 0.58 we know the icon is going to be last always. So yes I know it :)
>

Unmerged revisions

=== modified file 'qml/Coordinate.qml'
--- qml/Coordinate.qml	2016-03-26 18:53:17 +0000
+++ qml/Coordinate.qml	2016-04-03 11:23:12 +0000
@@ -14,7 +14,6 @@
  */
 
 import QtQuick 2.4
-import QtQuick.Layouts 1.1
 import Ubuntu.Components 1.3
 import Ubuntu.Components.Pickers 1.3
 import Ubuntu.Components.Popups 1.3
@@ -22,36 +21,31 @@
 Page {
     id: coordPage
 
-    title: i18n.tr("Coordinates")
-    anchors.fill: parent
-
-    head {
-        id: headPage
-        backAction: Action {
-            iconName: "back"
-            text: i18n.tr("Back")
-            onTriggered: {
-                mainPageStack.pop(coordPage)
-                mainPageStack.push(Qt.resolvedUrl("RoutePage.qml"))
+    header: PageHeader {
+        title: i18n.tr("Coordinates")
+        extension: Sections {
+            id: typeSections
+            anchors {
+                left: parent.left
+                bottom: parent.bottom
             }
-        }
-        sections {
+
             model: [i18n.tr("Decimal"), i18n.tr("Sexagesimal")]
             selectedIndex: 0
         }
     }
-
+    anchors.fill: parent
 
     Column {
         id: colCoordDec
-        visible: !coordPage.head.sections.selectedIndex
+        visible: !typeSections.selectedIndex
         anchors.horizontalCenter: parent.horizontalCenter
         anchors.verticalCenter: parent.verticalCenter
         width: units.gu(28)
         height: units.gu(10)
-     
+
         spacing: units.gu(2)
-     
+
         Row {
             spacing: units.gu(1)
             Label {
@@ -59,7 +53,7 @@
                 text: i18n.tr("Lat:")
                 width: units.gu(5)
             }
-     
+
             TextField {
                 id: lat1
                 anchors.verticalCenter: parent.verticalCenter
@@ -69,7 +63,7 @@
                 placeholderText: '51.506177'
             }
         }
-     
+
         Row {
             spacing: units.gu(1)
             Label {
@@ -77,7 +71,7 @@
                 text: i18n.tr("Long:")
                 width: units.gu(5)
             }
-     
+
             TextField {
                 id: lng1
                 anchors.verticalCenter: parent.verticalCenter
@@ -87,7 +81,7 @@
                 placeholderText: '-0.100236'
             }
         }
-     
+
         Button {
             id: showCoordDec
             text: i18n.tr("Show on Map")
@@ -96,6 +90,9 @@
             anchors.horizontalCenter: parent.horizontalCenter
             color: UbuntuColors.green
             onClicked: {
+                lng1.text === "" ? lng1.text = lng1.placeholderText : undefined
+                lat1.text === "" ? lat1.text = lat1.placeholderText : undefined
+
                 try {
                     var aux_lat = lat1.text;
                     var aux_lng = lng1.text;
@@ -106,8 +103,8 @@
                             mainPageStack.center_onpos = 1;
                         mainPageStack.pop(coordPage);
                         mainPageStack.executeJavaScript(
-                            "ui.markers_POI_set([{title: '', lat: " + mainPageStack.clickedLat + ", lng: " + mainPageStack.clickedLng + "}]);" 
-                        );
+                                    "ui.markers_POI_set([{title: '', lat: " + mainPageStack.clickedLat + ", lng: " + mainPageStack.clickedLng + "}]);"
+                                    );
                         mainPageStack.favPopup = false;
                         goThereActionPopover.show();
                     }
@@ -128,8 +125,8 @@
                 title: i18n.tr("Coordinates are not valid")
                 text: i18n.tr("Enter valid decimal coordinates\n\nExpected format is:") + "\n51.506177\n-0.100236"
                 Button {
-                     text: i18n.tr("Close")
-                     onClicked: PopupUtils.close(dialogue)
+                    text: i18n.tr("Close")
+                    onClicked: PopupUtils.close(dialogue)
                 }
             }
         }
@@ -138,14 +135,14 @@
     
     Column {
         id: colCoordPolar
-        visible: coordPage.head.sections.selectedIndex
+        visible: typeSections.selectedIndex
         anchors.horizontalCenter: parent.horizontalCenter
         anchors.verticalCenter: parent.verticalCenter
         width: units.gu(35)
         height: units.gu(10)
-    
+
         spacing: units.gu(2)
-    
+
         Row {
             Label {
                 anchors.verticalCenter: parent.verticalCenter
@@ -205,7 +202,7 @@
                 width: units.gu(2)
             }
         }
-     
+
         Row {
             Label {
                 anchors.verticalCenter: parent.verticalCenter
@@ -274,6 +271,15 @@
             anchors.horizontalCenter: parent.horizontalCenter
             color: UbuntuColors.green
             onClicked: {
+                lng2a.text === "" ? lng2a.text = lng2a.placeholderText : undefined
+                lat2a.text === "" ? lat2a.text = lat2a.placeholderText : undefined
+                lng2b.text === "" ? lng2b.text = lng2b.placeholderText : undefined
+                lat2b.text === "" ? lat2b.text = lat2b.placeholderText : undefined
+                lng2c.text === "" ? lng2c.text = lng2c.placeholderText : undefined
+                lat2c.text === "" ? lat2c.text = lat2c.placeholderText : undefined
+                lng2d.text === "" ? lng2d.text = lng2d.placeholderText : undefined
+                lat2d.text === "" ? lat2d.text = lat2d.placeholderText : undefined
+
                 try {
                     var aux_lat_day = parseInt(lat2a.text);
                     var aux_lat_min = parseFloat(lat2b.text);
@@ -285,7 +291,7 @@
                     var aux_lng_dir = lng2d.text.toUpperCase();
                     
                     if ((!isNaN(aux_lat_day) && !isNaN(aux_lat_min) && !isNaN(aux_lat_sec) && (aux_lat_dir === 'S' || aux_lat_dir === 'N')) &&
-                        (!isNaN(aux_lng_day) && !isNaN(aux_lng_min) && !isNaN(aux_lng_sec) && (aux_lng_dir === 'W' || aux_lng_dir === 'E'))) {
+                            (!isNaN(aux_lng_day) && !isNaN(aux_lng_min) && !isNaN(aux_lng_sec) && (aux_lng_dir === 'W' || aux_lng_dir === 'E'))) {
                         var aux_lat = aux_lat_day + aux_lat_min/60 + aux_lat_sec/(60*60);
                         if (aux_lat_dir === "S" || aux_lat_dir === "W")
                             aux_lat = aux_lat * -1;
@@ -301,8 +307,8 @@
                                 mainPageStack.center_onpos = 1;
                             mainPageStack.pop(coordPage);
                             mainPageStack.executeJavaScript(
-                                "ui.markers_POI_set([{title: '', lat: " + mainPageStack.clickedLat + ", lng: " + mainPageStack.clickedLng + "}]);" 
-                            );
+                                        "ui.markers_POI_set([{title: '', lat: " + mainPageStack.clickedLat + ", lng: " + mainPageStack.clickedLng + "}]);"
+                                        );
                             mainPageStack.favPopup = false;
                             goThereActionPopover.show();
                         }
@@ -327,8 +333,8 @@
                 title: i18n.tr("Coordinates are not valid")
                 text: i18n.tr("Enter valid sexagesimal coordinates\n\nExpected format is:") + "\n51° 30' 22.23'' N\n0° 6' 0.84'' W"
                 Button {
-                     text: i18n.tr("Close")
-                     onClicked: PopupUtils.close(dialogue)
+                    text: i18n.tr("Close")
+                    onClicked: PopupUtils.close(dialogue)
                 }
             }
         }

=== modified file 'qml/PoiListPage.qml'
--- qml/PoiListPage.qml	2016-03-28 19:36:52 +0000
+++ qml/PoiListPage.qml	2016-04-03 11:23:12 +0000
@@ -334,7 +334,7 @@
                     id: acessibilityIcon
                     name: "preferences-desktop-accessibility-symbolic"
                     visible: model.wheelchair === "yes" || model.wheelchair === "limited" // is limited enough as criteria?
-                    width: parent.height / 2
+                    width: units.gu(2)
                     SlotsLayout.position: SlotsLayout.Last
                 }
             }