← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

[Merge] lp:~ahayzen/ubuntu-weather-app/fix-1496422-1478255-today-extra-info into lp:ubuntu-weather-app/reboot

 

Andrew Hayzen has proposed merging lp:~ahayzen/ubuntu-weather-app/fix-1496422-1478255-today-extra-info into lp:ubuntu-weather-app/reboot.

Commit message:
* Implement extra info expandable for today info
* Add autopilot test for expanding the today info

Requested reviews:
  Ubuntu Weather Developers (ubuntu-weather-dev)
Related bugs:
  Bug #1478255 in Ubuntu Weather App: "[weather] Details such as chance of rain and humidity are available for all days but today"
  https://bugs.launchpad.net/ubuntu-weather-app/+bug/1478255
  Bug #1496422 in Ubuntu Weather App: "[reboot] Info for today is not full enough"
  https://bugs.launchpad.net/ubuntu-weather-app/+bug/1496422

For more details, see:
https://code.launchpad.net/~ahayzen/ubuntu-weather-app/fix-1496422-1478255-today-extra-info/+merge/272790

* Implement extra info expandable for today info
* Add autopilot test for expanding the today info
-- 
Your team Ubuntu Weather Developers is requested to review the proposed merge of lp:~ahayzen/ubuntu-weather-app/fix-1496422-1478255-today-extra-info into lp:ubuntu-weather-app/reboot.
=== modified file 'app/components/DayDelegateExtraInfo.qml'
--- app/components/DayDelegateExtraInfo.qml	2015-09-04 21:36:50 +0000
+++ app/components/DayDelegateExtraInfo.qml	2015-09-29 15:23:31 +0000
@@ -44,6 +44,7 @@
         horizontalAlignment: Text.AlignHCenter
         text: modelData.condition
         width: parent.width
+        visible: text !== ""
     }
 
     ForecastDetailsDelegate {

=== modified file 'app/components/HomeTempInfo.qml'
--- app/components/HomeTempInfo.qml	2015-08-23 18:03:23 +0000
+++ app/components/HomeTempInfo.qml	2015-09-29 15:23:31 +0000
@@ -19,56 +19,157 @@
 import QtQuick 2.4
 import Ubuntu.Components 1.2
 
-
-Column {
+Item {
+    id: homeTempInfoItem
     anchors {
         left: parent.left
         right: parent.right
     }
-    spacing: units.gu(1)
-
-    property alias description: descriptionLabel.text
-    property alias high: highLabel.text
-    property alias low: lowLabel.text
+    clip: true
+    height: collapsedHeight
+    objectName: "homeTempInfo"
+    state: "normal"
+    states: [
+        State {
+            name: "normal"
+            PropertyChanges {
+                target: homeTempInfoItem
+                height: collapsedHeight
+            }
+            PropertyChanges {
+                target: expandedInfo
+                opacity: 0
+            }
+        },
+        State {
+            name: "expanded"
+            PropertyChanges {
+                target: homeTempInfoItem
+                height: expandedHeight
+            }
+            PropertyChanges {
+                target: expandedInfo
+                opacity: 1
+            }
+        }
+    ]
+    transitions: [
+        Transition {
+            from: "normal"
+            to: "expanded"
+            SequentialAnimation {
+                ScriptAction {
+                    script: expandedInfo.active = true
+                }
+                NumberAnimation {
+                    easing.type: Easing.InOutQuad
+                    properties: "height,opacity"
+                }
+            }
+        },
+        Transition {
+            from: "expanded"
+            to: "normal"
+            SequentialAnimation {
+                NumberAnimation {
+                    easing.type: Easing.InOutQuad
+                    properties: "height,opacity"
+                }
+                ScriptAction {
+                    script: expandedInfo.active = false
+                }
+            }
+        }
+    ]
+
+    property int collapsedHeight: units.gu(12)
+    property int expandedHeight: collapsedHeight + units.gu(4) + (expandedInfo.item ? expandedInfo.item.height : 0)
+
+    property var modelData
+
     property alias now: nowLabel.text
 
-    Label {
-        font.weight: Font.Light
-        fontSize: "small"
-        text: i18n.tr("Today")
-    }
-
-    Label {
-        id: descriptionLabel
-        font.capitalization: Font.Capitalize
-        font.weight: Font.Normal
-        fontSize: "large"
-    }
-
-    Row {
-        spacing: units.gu(2)
+    Column {
+        id: labelColumn
+        anchors {
+            left: parent.left
+            right: parent.right
+        }
+        spacing: units.gu(1)
 
         Label {
-            id: nowLabel
-            color: UbuntuColors.orange
-            font.pixelSize: units.gu(8)
             font.weight: Font.Light
-            height: units.gu(8)
-            verticalAlignment: Text.AlignBottom  // AlignBottom seems to put it at the top?
-        }
-
-        Column {
-            Label {
-                id: lowLabel
-                font.weight: Font.Light
-                fontSize: "medium"
-            }
-
-            Label {
-                id: highLabel
-                font.weight: Font.Light
-                fontSize: "medium"
-            }
+            fontSize: "small"
+            text: i18n.tr("Today")
+        }
+
+        Label {
+            id: descriptionLabel
+            font.capitalization: Font.Capitalize
+            font.weight: Font.Normal
+            fontSize: "large"
+            text: modelData.condition
+        }
+
+        Row {
+            spacing: units.gu(2)
+
+            Label {
+                id: nowLabel
+                color: UbuntuColors.orange
+                font.pixelSize: units.gu(8)
+                font.weight: Font.Light
+                height: units.gu(8)
+                verticalAlignment: Text.AlignBottom  // AlignBottom seems to put it at the top?
+            }
+
+            Column {
+                Label {
+                    id: lowLabel
+                    font.weight: Font.Light
+                    fontSize: "medium"
+                    text: modelData.low
+                }
+
+                Label {
+                    id: highLabel
+                    font.weight: Font.Light
+                    fontSize: "medium"
+                    text: modelData.high
+                }
+            }
+        }
+    }
+
+    Loader {
+        id: expandedInfo
+        active: false
+        anchors {
+            left: parent.left
+            leftMargin: units.gu(2)
+            right: parent.right
+            rightMargin: units.gu(2)
+            top: labelColumn.bottom
+            topMargin: units.gu(2)
+        }
+        asynchronous: true
+        opacity: 0
+        source: "DayDelegateExtraInfo.qml"
+
+        property var modelData: todayData || ({})
+    }
+
+    MouseArea {
+        anchors {
+            fill: parent
+        }
+        onClicked: parent.state = parent.state === "normal" ? "expanded" : "normal"
+    }
+
+    Behavior on height {
+        NumberAnimation {
+            easing.type: Easing.InOutQuad
         }
     }
 }
+

=== modified file 'app/ui/LocationPane.qml'
--- app/ui/LocationPane.qml	2015-09-05 00:03:33 +0000
+++ app/ui/LocationPane.qml	2015-09-29 15:23:31 +0000
@@ -36,16 +36,15 @@
       Data properties
     */
     property string name
-    property string conditionText
     property string currentTemp
-    property string todayMaxTemp
-    property string todayMinTemp
     property string icon
     property string iconName
 
     property var hourlyForecastsData
     property string hourlyTempUnits
 
+    property var todayData
+
     delegate: DayDelegate {
         day: model.day
         high: model.high
@@ -98,9 +97,7 @@
 
         HomeTempInfo {
             id: homeTempInfo
-            description: conditionText
-            high: mainPageWeekdayListView.todayMaxTemp
-            low: mainPageWeekdayListView.todayMinTemp
+            modelData: todayData
             now: mainPageWeekdayListView.currentTemp
         }
 
@@ -143,6 +140,25 @@
         }
     }
 
+    function getDayData(data) {
+        var tempUnits = settings.tempScale === "°C" ? "metric" : "imperial"
+
+        return {
+            day: formatTimestamp(data.date, 'dddd'),
+            low: Math.round(data[tempUnits].tempMin).toString() + settings.tempScale,
+            high: (data[tempUnits].tempMax !== undefined) ? Math.round(data[tempUnits].tempMax).toString() + settings.tempScale : "",
+            image: (data.icon !== undefined && iconMap[data.icon] !== undefined) ? iconMap[data.icon] : "",
+            condition: emptyIfUndefined(data.condition),
+            chanceOfRain: emptyIfUndefined(data.propPrecip, "%"),
+            humidity: emptyIfUndefined(data.humidity, "%"),
+            sunrise: data.sunrise || SunCalc.SunCalc.getTimes(getDate(data.date), data.location.coord.lat, data.location.coord.lon).sunrise.toLocaleTimeString(),
+            sunset: data.sunset || SunCalc.SunCalc.getTimes(getDate(data.date), data.location.coord.lat, data.location.coord.lon).sunset.toLocaleTimeString(),
+            uvIndex: emptyIfUndefined(data.uv),
+            wind: data[tempUnits].windSpeed === undefined || data.windDir === undefined
+                        ? "" : Math.round(data[tempUnits].windSpeed) + settings.windUnits + " " + data.windDir
+        };
+    }
+
     function emptyIfUndefined(variable, append) {
         if (append === undefined) {
             append = ""
@@ -163,7 +179,6 @@
                 current = data.data[0].current,
                 forecasts = data.data,
                 forecastsLength = forecasts.length,
-                today = forecasts[0],
                 hourlyForecasts = [];
 
         var tempUnits = settings.tempScale === "°C" ? "metric" : "imperial"
@@ -174,12 +189,10 @@
         // set current temps and condition
         iconName = (current.icon) ? current.icon : "";
         icon = (imageMap[iconName] !== undefined) ? imageMap[iconName] : "";
-        conditionText = (current.condition !== undefined) ? current.condition : "";
-        todayMaxTemp = (today[tempUnits].tempMax !== undefined) ? Math.round(today[tempUnits].tempMax).toString() + settings.tempScale: "";
-        todayMinTemp = Math.round(today[tempUnits].tempMin).toString() + settings.tempScale;
         currentTemp = Math.round(current[tempUnits].temp).toString() + String("°");
 
         // reset days list
+        // TODO: overwrite and trim to make the refresh smoother?
         mainPageWeekdayListView.model.clear()
 
         // set daily forecasts
@@ -189,27 +202,25 @@
                 if(forecasts[x].hourly !== undefined && forecasts[x].hourly.length > 0) {
                     hourlyForecasts = hourlyForecasts.concat(forecasts[x].hourly)
                 }
-                if(x === 0) {
-                    // skip todays daydata
-                    continue;
-                }
-
-                // set daydata
-                var dayData = {
-                    day: formatTimestamp(forecasts[x].date, 'dddd'),
-                    low: Math.round(forecasts[x][tempUnits].tempMin).toString() + settings.tempScale,
-                    high: (forecasts[x][tempUnits].tempMax !== undefined) ? Math.round(forecasts[x][tempUnits].tempMax).toString() + settings.tempScale : "",
-                    image: (forecasts[x].icon !== undefined && iconMap[forecasts[x].icon] !== undefined) ? iconMap[forecasts[x].icon] : "",
-                    condition: emptyIfUndefined(forecasts[x].condition),
-                    chanceOfRain: emptyIfUndefined(forecasts[x].propPrecip, "%"),
-                    humidity: emptyIfUndefined(forecasts[x].humidity, "%"),
-                    sunrise: forecasts[x].sunrise || SunCalc.SunCalc.getTimes(getDate(forecasts[x].date), data.location.coord.lat, data.location.coord.lon).sunrise.toLocaleTimeString(),
-                    sunset: forecasts[x].sunset || SunCalc.SunCalc.getTimes(getDate(forecasts[x].date), data.location.coord.lat, data.location.coord.lon).sunset.toLocaleTimeString(),
-                    uvIndex: emptyIfUndefined(forecasts[x].uv),
-                    wind: forecasts[x][tempUnits].windSpeed === undefined || forecasts[x].windDir === undefined
-                                ? "" : Math.round(forecasts[x][tempUnits].windSpeed) + settings.windUnits + " " + forecasts[x].windDir
-                }
-                mainPageWeekdayListView.model.append(dayData);
+
+                // Copy the coords of the location
+                // so that sun{rise,set} work with OWM
+                forecasts[x].location = {
+                    coord: data.location.coord,
+                };
+
+                if (x === 0) {
+                    // Store as tmp var so we can remove the condition
+                    // therefore not showing it in the expandable
+                    var tmpData = getDayData(forecasts[x]);
+                    tmpData.condition = "";
+
+                    // store today's data for later use
+                    todayData = tmpData;
+                } else {
+                    // set daydata
+                    mainPageWeekdayListView.model.append(getDayData(forecasts[x]));
+                }
             }
         }
 

=== modified file 'debian/changelog'
--- debian/changelog	2015-09-14 19:59:26 +0000
+++ debian/changelog	2015-09-29 15:23:31 +0000
@@ -47,6 +47,8 @@
   * Fix for wind test failing and some selects not waiting for visible=True (LP: #1492321)
   * Add LocationPane helper to the autopilot tests
   * Various tidy ups to improve readability and code commentary of autopilot code
+  * Implement extra info expandable for today info (LP: #1496422, LP: #1478255)
+  * Add autopilot test for expanding the today info
 
   [ Carla Sella ]
   * Create autopilot test which shows the day delegate (LP: #1452491)

=== modified file 'po/com.ubuntu.weather.pot'
--- po/com.ubuntu.weather.pot	2015-08-28 21:54:22 +0000
+++ po/com.ubuntu.weather.pot	2015-09-29 15:23:31 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: ubuntu-weather-app\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2015-08-28 16:51-0500\n"
+"POT-Creation-Date: 2015-09-29 15:30+0100\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@xxxxxx>\n"
@@ -18,31 +18,31 @@
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n"
 
-#: ../app/components/DayDelegateExtraInfo.qml:45
+#: ../app/components/DayDelegateExtraInfo.qml:52
 msgid "Chance of rain"
 msgstr ""
 
-#: ../app/components/DayDelegateExtraInfo.qml:52
+#: ../app/components/DayDelegateExtraInfo.qml:59
 msgid "Winds"
 msgstr ""
 
-#: ../app/components/DayDelegateExtraInfo.qml:60
+#: ../app/components/DayDelegateExtraInfo.qml:68
 msgid "UV Index"
 msgstr ""
 
-#: ../app/components/DayDelegateExtraInfo.qml:66
+#: ../app/components/DayDelegateExtraInfo.qml:74
 msgid "Pollen"
 msgstr ""
 
-#: ../app/components/DayDelegateExtraInfo.qml:73
+#: ../app/components/DayDelegateExtraInfo.qml:81
 msgid "Humidity"
 msgstr ""
 
-#: ../app/components/DayDelegateExtraInfo.qml:80
+#: ../app/components/DayDelegateExtraInfo.qml:88
 msgid "Sunrise"
 msgstr ""
 
-#: ../app/components/DayDelegateExtraInfo.qml:87
+#: ../app/components/DayDelegateExtraInfo.qml:95
 msgid "Sunset"
 msgstr ""
 
@@ -58,7 +58,7 @@
 msgid "Manually add a location by swiping up from the bottom of the display"
 msgstr ""
 
-#: ../app/components/HomeTempInfo.qml:38
+#: ../app/components/HomeTempInfo.qml:102
 msgid "Today"
 msgstr ""
 
@@ -110,23 +110,23 @@
 msgid "Search city"
 msgstr ""
 
-#: ../app/ui/AddLocationPage.qml:295
+#: ../app/ui/AddLocationPage.qml:297
 msgid "No city found"
 msgstr ""
 
-#: ../app/ui/AddLocationPage.qml:308
+#: ../app/ui/AddLocationPage.qml:310
 msgid "Couldn't load weather data, please try later again!"
 msgstr ""
 
-#: ../app/ui/AddLocationPage.qml:318
+#: ../app/ui/AddLocationPage.qml:320
 msgid "Location already added."
 msgstr ""
 
-#: ../app/ui/AddLocationPage.qml:321
+#: ../app/ui/AddLocationPage.qml:323
 msgid "OK"
 msgstr ""
 
-#: ../app/ui/HomePage.qml:31 ../app/ui/LocationsPage.qml:31
+#: ../app/ui/HomePage.qml:31 ../app/ui/LocationsPage.qml:29
 msgid "Locations"
 msgstr ""
 

=== modified file 'tests/autopilot/ubuntu_weather_app/__init__.py'
--- tests/autopilot/ubuntu_weather_app/__init__.py	2015-09-10 16:46:42 +0000
+++ tests/autopilot/ubuntu_weather_app/__init__.py	2015-09-29 15:23:31 +0000
@@ -196,12 +196,20 @@
         return self.get_location_pane(self.get_selected_location_index())
 
 
+class HomeTempInfo(UbuntuUIToolkitCustomProxyObjectBase):
+    pass
+
+
 class LocationPane(UbuntuUIToolkitCustomProxyObjectBase):
     @click_object
     def click_day_delegate(self, day):
         return self.get_day_delegate(day)
 
     @click_object
+    def click_home_temp_info(self):
+        return self.get_home_temp_info()
+
+    @click_object
     def click_settings_button(self):
         return self.select_single(
             "AbstractButton", objectName="settingsButton")
@@ -210,6 +218,10 @@
         return self.wait_select_single(
             "DayDelegate", objectName="dayDelegate" + str(day))
 
+    def get_home_temp_info(self):
+        return self.wait_select_single(
+            "HomeTempInfo", objectName="homeTempInfo")
+
 
 class LocationsPage(Page):
     """Autopilot helper for LocationsPage."""

=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_home_page.py'
--- tests/autopilot/ubuntu_weather_app/tests/test_home_page.py	2015-09-05 00:03:33 +0000
+++ tests/autopilot/ubuntu_weather_app/tests/test_home_page.py	2015-09-29 15:23:31 +0000
@@ -37,7 +37,7 @@
 
         day = 0
 
-        # Get the first day delegate in the selection pane
+        # Get the first day delegate in the selected pane
         location_pane = self.home_page.get_selected_location_pane()
         day_delegate = location_pane.get_day_delegate(day)
 
@@ -56,3 +56,26 @@
         # Check that the state and height of the delegate have changed
         self.assertThat(day_delegate.state, Eventually(Equals("expanded")))
         self.assertEqual(day_delegate.height, day_delegate.expandedHeight)
+
+    def test_show_today_details(self):
+        """tests clicking on the today info to expand and contract it"""
+
+        # Get the HomeTempInfo component from the selected pane
+        location_pane = self.home_page.get_selected_location_pane()
+        home_temp_info = location_pane.get_home_temp_info()
+
+        # Check that the extra info is not shown
+        self.assertThat(home_temp_info.state, Eventually(Equals("normal")))
+
+        # Click the HomeTempInfo to show the extra info
+        location_pane.click_home_temp_info()
+
+        # Re-get HomeTempInfo as change in loaders changes tree
+        home_temp_info = location_pane.get_home_temp_info()
+
+        # Wait for the height of the HomeTempInfo to grow
+        home_temp_info.height.wait_for(home_temp_info.expandedHeight)
+
+        # Check that the state and height of the HomeTempInfo have changed
+        self.assertThat(home_temp_info.state, Eventually(Equals("expanded")))
+        self.assertEqual(home_temp_info.height, home_temp_info.expandedHeight)


Follow ups