ubuntu-touch-coreapps-reviewers team mailing list archive
-
ubuntu-touch-coreapps-reviewers team
-
Mailing list archive
-
Message #04359
[Merge] lp:~ahayzen/ubuntu-weather-app/reboot-profile-homepage-001 into lp:ubuntu-weather-app/reboot
Andrew Hayzen has proposed merging lp:~ahayzen/ubuntu-weather-app/reboot-profile-homepage-001 into lp:ubuntu-weather-app/reboot.
Commit message:
* Split day delegate extra info into a separate component that is loaded on demand
* Use ListView per Location Pane so that scrolling vertically only affect the currently focused pane
Requested reviews:
Ubuntu Weather Developers (ubuntu-weather-dev)
For more details, see:
https://code.launchpad.net/~ahayzen/ubuntu-weather-app/reboot-profile-homepage-001/+merge/268635
* Split day delegate extra info into a separate component that is loaded on demand
* Use ListView per Location Pane so that scrolling vertically only affect the currently focused pane
This improves the performance of scrolling up/down the homepage as it doesn't need to load the extra info for each delegate and it only scrolls the location pane in-view.
--
Your team Ubuntu Weather Developers is requested to review the proposed merge of lp:~ahayzen/ubuntu-weather-app/reboot-profile-homepage-001 into lp:ubuntu-weather-app/reboot.
=== modified file 'app/components/DayDelegate.qml'
--- app/components/DayDelegate.qml 2015-08-09 12:20:27 +0000
+++ app/components/DayDelegate.qml 2015-08-20 19:50:47 +0000
@@ -25,21 +25,14 @@
height: collapsedHeight
property int collapsedHeight: units.gu(8)
- property int expandedHeight: collapsedHeight + units.gu(4) + extraInfoColumn.height
+ property int expandedHeight: collapsedHeight + units.gu(4) + (expandedInfo.item ? expandedInfo.item.height : 0)
property alias day: dayLabel.text
property alias image: weatherImage.name
property alias high: highLabel.text
property alias low: lowLabel.text
- property alias condition: conditionForecast.text
- property alias chanceOfRain: chanceOfRainForecast.value
- property alias humidity: humidityForecast.value
- property alias pollen: pollenForecast.value
- property alias sunrise: sunriseForecast.value
- property alias sunset: sunsetForecast.value
- property alias wind: windForecast.value
- property alias uvIndex: uvIndexForecast.value
+ property alias modelData: expandedInfo.modelData
state: "normal"
states: [
@@ -49,6 +42,10 @@
target: dayDelegate
height: collapsedHeight
}
+ PropertyChanges {
+ target: expandedInfo
+ opacity: 0
+ }
},
State {
name: "expanded"
@@ -68,9 +65,8 @@
from: "normal"
to: "expanded"
SequentialAnimation {
- NumberAnimation {
- easing.type: Easing.InOutQuad
- properties: "height"
+ ScriptAction {
+ script: expandedInfo.active = true
}
NumberAnimation {
easing.type: Easing.InOutQuad
@@ -86,9 +82,8 @@
easing.type: Easing.InOutQuad
properties: "opacity"
}
- NumberAnimation {
- easing.type: Easing.InOutQuad
- properties: "height"
+ ScriptAction {
+ script: expandedInfo.active = false
}
}
}
@@ -165,77 +160,27 @@
}
}
- Item {
+ Loader {
id: expandedInfo
+ active: false
anchors {
- bottom: parent.bottom
+ bottomMargin: units.gu(2)
left: parent.left
+ leftMargin: units.gu(2)
right: parent.right
+ rightMargin: units.gu(2)
top: mainInfo.bottom
- bottomMargin: units.gu(2)
}
+ asynchronous: true
opacity: 0
- visible: opacity !== 0
-
- Column {
- id: extraInfoColumn
- anchors {
- centerIn: parent
- }
- spacing: units.gu(2)
-
- // FIXME: extended-infomation_* aren't actually on device
-
- // Overview text
- Label {
- id: conditionForecast
- color: UbuntuColors.coolGrey
- fontSize: "x-large"
- horizontalAlignment: Text.AlignHCenter
- width: parent.width
- }
-
- ForecastDetailsDelegate {
- id: chanceOfRainForecast
- forecast: i18n.tr("Chance of rain")
- imageSource: "../graphics/extended-information_chance-of-rain.svg"
- }
-
- ForecastDetailsDelegate {
- id: windForecast
- forecast: i18n.tr("Winds")
- imageSource: "../graphics/extended-information_wind.svg"
- }
-
- ForecastDetailsDelegate {
- id: uvIndexForecast
- imageSource: "../graphics/extended-information_uv-level.svg"
- forecast: i18n.tr("UV Index")
- }
-
- ForecastDetailsDelegate {
- id: pollenForecast
- forecast: i18n.tr("Pollen")
- // FIXME: need icon
- }
-
- ForecastDetailsDelegate {
- id: humidityForecast
- forecast: i18n.tr("Humidity")
- imageSource: "../graphics/extended-information_humidity.svg"
- }
-
- ForecastDetailsDelegate {
- id: sunriseForecast
- forecast: i18n.tr("Sunrise")
- imageSource: "../graphics/extended-information_sunrise.svg"
- }
-
- ForecastDetailsDelegate {
- id: sunsetForecast
- forecast: i18n.tr("Sunset")
- imageSource: "../graphics/extended-information_sunset.svg"
- }
+ source: "DayDelegateExtraInfo.qml"
+
+ property var modelData
+ }
+
+ Behavior on height {
+ NumberAnimation {
+ easing.type: Easing.InOutQuad
}
}
=== added file 'app/components/DayDelegateExtraInfo.qml'
--- app/components/DayDelegateExtraInfo.qml 1970-01-01 00:00:00 +0000
+++ app/components/DayDelegateExtraInfo.qml 2015-08-20 19:50:47 +0000
@@ -0,0 +1,90 @@
+/*
+ * Copyright (C) 2015 Canonical Ltd
+ *
+ * This file is part of Ubuntu Weather App
+ *
+ * Ubuntu Weather App is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Ubuntu Weather App is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import QtQuick 2.4
+import Ubuntu.Components 1.2
+
+
+Column {
+ id: extraInfoColumn
+ anchors {
+ centerIn: parent
+ }
+ spacing: units.gu(2)
+
+ // FIXME: extended-infomation_* aren't actually on device
+
+ // Overview text
+ Label {
+ id: conditionForecast
+ color: UbuntuColors.coolGrey
+ fontSize: "x-large"
+ horizontalAlignment: Text.AlignHCenter
+ text: modelData.condition
+ width: parent.width
+ }
+
+ ForecastDetailsDelegate {
+ id: chanceOfRainForecast
+ forecast: i18n.tr("Chance of rain")
+ imageSource: "../graphics/extended-information_chance-of-rain.svg"
+ value: modelData.chanceOfRain
+ }
+
+ ForecastDetailsDelegate {
+ id: windForecast
+ forecast: i18n.tr("Winds")
+ imageSource: "../graphics/extended-information_wind.svg"
+ value: modelData.wind
+ }
+
+ ForecastDetailsDelegate {
+ id: uvIndexForecast
+ imageSource: "../graphics/extended-information_uv-level.svg"
+ forecast: i18n.tr("UV Index")
+ value: modelData.uvIndex
+ }
+
+ ForecastDetailsDelegate {
+ id: pollenForecast
+ forecast: i18n.tr("Pollen")
+ // FIXME: need icon
+ //value: modelData.pollen // TODO: extra from API
+ }
+
+ ForecastDetailsDelegate {
+ id: humidityForecast
+ forecast: i18n.tr("Humidity")
+ imageSource: "../graphics/extended-information_humidity.svg"
+ value: modelData.humidity
+ }
+
+ ForecastDetailsDelegate {
+ id: sunriseForecast
+ forecast: i18n.tr("Sunrise")
+ imageSource: "../graphics/extended-information_sunrise.svg"
+ value: modelData.sunrise
+ }
+
+ ForecastDetailsDelegate {
+ id: sunsetForecast
+ forecast: i18n.tr("Sunset")
+ imageSource: "../graphics/extended-information_sunset.svg"
+ value: modelData.sunset
+ }
+}
=== modified file 'app/components/HomeHourly.qml'
--- app/components/HomeHourly.qml 2015-06-21 18:35:36 +0000
+++ app/components/HomeHourly.qml 2015-08-20 19:50:47 +0000
@@ -25,14 +25,14 @@
clip:true
height: parent ? parent.height : undefined
width: parent ? parent.width : undefined
- model: forecasts.length
+ model: hourlyForecastsData.length
orientation: ListView.Horizontal
property string currentDate: Qt.formatTime(new Date())
onVisibleChanged: {
if(visible) {
- ListView.model = forecasts.length
+ ListView.model = hourlyForecastsData.length
}
}
@@ -46,7 +46,7 @@
delegate: Item {
id: delegate
- property var hourData: forecasts[index]
+ property var hourData: hourlyForecastsData[index]
height: parent.height
width: childrenRect.width
@@ -84,7 +84,7 @@
anchors.horizontalCenter: parent.horizontalCenter
font.pixelSize: units.gu(3)
font.weight: Font.Light
- text: Math.round(hourData[tempUnits].temp).toString()+settings.tempScale
+ text: Math.round(hourData[hourlyTempUnits].temp).toString()+settings.tempScale
}
}
=== modified file 'app/ui/HomePage.qml'
--- app/ui/HomePage.qml 2015-07-30 04:10:44 +0000
+++ app/ui/HomePage.qml 2015-08-20 19:50:47 +0000
@@ -93,91 +93,58 @@
}
/*
- Flickable to scroll the location vertically.
- The respective contentHeight gets calculated after the Location is filled with data.
+ ListView for locations with snap-scrolling horizontally.
*/
- Flickable {
- id: locationFlickable
- width: parent.width
- height: parent.height
-
- // FIXME: not sure where the 3GU comes from, PullToRefresh or something in HomePage?
- contentHeight: locationPages.currentItem ? locationPages.currentItem.childrenRect.height + units.gu(3) : 0
- contentWidth: parent.width
-
- Behavior on contentHeight {
- NumberAnimation {
-
- }
- }
-
- PullToRefresh {
- id: pullToRefresh
- parent: locationFlickable
- refreshing: false
- onRefresh: {
- locationPages.loaded = false
- refreshing = true
- refreshData(false, true)
- }
- }
-
- /*
- ListView for locations with snap-scrolling horizontally.
- */
- ListView {
- id: locationPages
- objectName: "locationPages"
- anchors.fill: parent
- currentIndex: settings.current
- delegate: LocationPane {}
- height: childrenRect.height
- highlightRangeMode: ListView.StrictlyEnforceRange
- model: weatherApp.locationsList.length
- orientation: ListView.Horizontal
- // TODO with snapMode, currentIndex is not properly set and setting currentIndex fails
- //snapMode: ListView.SnapOneItem
- width: parent.width
-
- property bool loaded: false
-
- signal collapseOtherDelegates(int index)
-
- onCurrentIndexChanged: {
- if (loaded) {
- // FIXME: when a model is reloaded this causes the currentIndex to be lost
- settings.current = currentIndex
-
- collapseOtherDelegates(-1) // collapse all
- }
- }
- onModelChanged: {
- currentIndex = settings.current
-
- if (model > 0) {
- pullToRefresh.refreshing = false
- loading = false
- loaded = true
- }
- }
- onVisibleChanged: {
- if (!visible && loaded) {
- collapseOtherDelegates(-1) // collapse all
- }
- }
-
- // TODO: workaround for not being able to use snapMode property
- Component.onCompleted: {
- var scaleFactor = units.gridUnit * 10;
- maximumFlickVelocity = maximumFlickVelocity * scaleFactor;
- flickDeceleration = flickDeceleration * scaleFactor;
- }
-
- Connections {
- target: settings
- onCurrentChanged: {
- locationPages.currentIndex = settings.current
- }
+ ListView {
+ id: locationPages
+ objectName: "locationPages"
+ anchors.fill: parent
+ contentHeight: parent.height
+ currentIndex: settings.current
+ delegate: LocationPane {}
+ highlightRangeMode: ListView.StrictlyEnforceRange
+ model: weatherApp.locationsList.length
+ orientation: ListView.Horizontal
+ // TODO with snapMode, currentIndex is not properly set and setting currentIndex fails
+ //snapMode: ListView.SnapOneItem
+
+ property bool loaded: false
+
+ signal collapseOtherDelegates(int index)
+
+ onCurrentIndexChanged: {
+ if (loaded) {
+ // FIXME: when a model is reloaded this causes the currentIndex to be lost
+ settings.current = currentIndex
+
+ collapseOtherDelegates(-1) // collapse all
+ }
+ }
+ onModelChanged: {
+ currentIndex = settings.current
+
+ if (model > 0) {
+ loading = false
+ loaded = true
+ }
+ }
+ onVisibleChanged: {
+ if (!visible && loaded) {
+ collapseOtherDelegates(-1) // collapse all
+ }
+ }
+
+ // TODO: workaround for not being able to use snapMode property
+ Component.onCompleted: {
+ var scaleFactor = units.gridUnit * 10;
+ maximumFlickVelocity = maximumFlickVelocity * scaleFactor;
+ flickDeceleration = flickDeceleration * scaleFactor;
+ }
+
+ Connections {
+ target: settings
+ onCurrentChanged: {
+ locationPages.currentIndex = settings.current
}
}
}
=== modified file 'app/ui/LocationPane.qml'
--- app/ui/LocationPane.qml 2015-08-09 12:20:27 +0000
+++ app/ui/LocationPane.qml 2015-08-20 19:50:47 +0000
@@ -22,8 +22,16 @@
import "../components"
import "../data/suncalc.js" as SunCalc
-Item {
- id: locationItem
+
+ListView {
+ id: mainPageWeekdayListView
+ height: parent.height
+ model: ListModel {
+
+ }
+ objectName: "locationListView" + index
+ width: weatherApp.width
+
/*
Data properties
*/
@@ -35,11 +43,85 @@
property string icon
property string iconName
- Component.onCompleted: renderData(index)
-
- width: locationPage.width
- height: childrenRect.height
- anchors.fill: parent.fill
+ property var hourlyForecastsData
+ property string hourlyTempUnits
+
+ delegate: DayDelegate {
+ day: model.day
+ high: model.high
+ image: model.image
+ low: model.low
+
+ modelData: model
+ }
+ header: Column {
+ id: locationTop
+
+ anchors {
+ left: parent.left
+ right: parent.right
+ margins: units.gu(2)
+ }
+ spacing: units.gu(1)
+ onHeightChanged: mainPageWeekdayListView.contentY = -height
+
+ Row { // spacing at top
+ height: units.gu(1)
+ width: parent.width
+ }
+
+ HeaderRow {
+ id: headerRow
+ locationName: mainPageWeekdayListView.name
+ }
+
+ HomeGraphic {
+ id: homeGraphic
+ icon: mainPageWeekdayListView.icon
+ MouseArea {
+ anchors.fill: parent
+ onClicked: {
+ homeGraphic.visible = false;
+ }
+ }
+ }
+
+ Loader {
+ id: homeHourlyLoader
+ active: !homeGraphic.visible
+ asynchronous: true
+ height: units.gu(32)
+ source: "../components/HomeHourly.qml"
+ visible: active
+ width: parent.width
+ }
+
+ HomeTempInfo {
+ id: homeTempInfo
+ description: conditionText
+ high: mainPageWeekdayListView.todayMaxTemp
+ low: mainPageWeekdayListView.todayMinTemp
+ now: mainPageWeekdayListView.currentTemp
+ }
+
+ // TODO: Migrate this to using the new SDK list item when possible.
+ ListItem.ThinDivider {
+ anchors {
+ leftMargin: units.gu(-2);
+ rightMargin: units.gu(-2)
+ }
+ }
+ }
+
+ PullToRefresh {
+ id: pullToRefresh
+ refreshing: false
+ onRefresh: {
+ locationPages.loaded = false
+ refreshing = true
+ refreshData(false, true)
+ }
+ }
function emptyIfUndefined(variable, append) {
if (append === undefined) {
@@ -113,89 +195,10 @@
// set data for hourly forecasts
if(hourlyForecasts.length > 0) {
- homeHourlyLoader.forecasts = hourlyForecasts;
- homeHourlyLoader.tempUnits = tempUnits;
- }
- }
-
- Column {
- id: locationTop
-
- anchors {
- top: parent.top
- left: parent.left
- right: parent.right
- margins: units.gu(2)
- }
- spacing: units.gu(1)
-
- HeaderRow {
- id: headerRow
- locationName: locationItem.name
- }
-
- HomeGraphic {
- id: homeGraphic
- icon: locationItem.icon
- MouseArea {
- anchors.fill: parent
- onClicked: {
- homeGraphic.visible = false;
- }
- }
- }
-
- Loader {
- id: homeHourlyLoader
- active: !homeGraphic.visible
- asynchronous: true
- height: units.gu(32)
- source: "../components/HomeHourly.qml"
- visible: active
- width: parent.width
-
- property var forecasts: []
- property string tempUnits: ""
- }
-
- HomeTempInfo {
- id: homeTempInfo
- description: conditionText
- high: locationItem.todayMaxTemp
- low: locationItem.todayMinTemp
- now: locationItem.currentTemp
- }
-
- // TODO: Migrate this to using the new SDK list item when possible.
- ListItem.ThinDivider { anchors { leftMargin: units.gu(-2); rightMargin: units.gu(-2) } }
- }
- Column {
- id: weekdayColumn
- objectName: "weekdayColumn" + index
-
- anchors.top: locationTop.bottom
- height: childrenRect.height
- width: parent.width
-
- Repeater {
- id: mainPageWeekdayListView
- model: ListModel{}
- delegate: DayDelegate {
- day: model.day
- high: model.high
- image: model.image
- low: model.low
-
- condition: model.condition
- chanceOfRain: model.chanceOfRain
- humidity: model.humidity
- // TODO: extra from API
- //pollen: model.pollen
- sunrise: model.sunrise
- sunset: model.sunset
- wind: model.wind
- uvIndex: model.uvIndex
- }
- }
- }
+ hourlyForecastsData = hourlyForecasts;
+ hourlyTempUnits = tempUnits;
+ }
+ }
+
+ Component.onCompleted: renderData(index)
}
=== modified file 'debian/changelog'
--- debian/changelog 2015-08-20 14:53:54 +0000
+++ debian/changelog 2015-08-20 19:50:47 +0000
@@ -36,9 +36,11 @@
* Create autopilot test which adds a location via cached results (LP: #1452497)
* Fix for racy search bar causing incorrect search query when typed quickly
* Fix for flaky test as get_location does not wait for location to be loaded (LP: #1485657)
+ * Split day delegate extra info into a separate component that is loaded on demand
+ * Use ListView per Location Pane so that scrolling vertically only affect the currently focused pane
[ Carla Sella ]
- * Create autopilot test which shows the day delegate (LP: #1452491)
+ * Create autopilot test which shows the day delegate (LP: #1452491)
-- Victor Thompson <victor.thompson@xxxxxxxxx> Mon, 01 Jun 2015 20:11:23 -0500
=== modified file 'po/com.ubuntu.weather.pot'
--- po/com.ubuntu.weather.pot 2015-08-17 15:52:27 +0000
+++ po/com.ubuntu.weather.pot 2015-08-20 19:50:47 +0000
@@ -8,7 +8,7 @@
msgstr ""
"Project-Id-Version: ubuntu-weather-app\n"
"Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2015-08-12 00:11+0100\n"
+"POT-Creation-Date: 2015-08-20 20:46+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/DayDelegate.qml:200
+#: ../app/components/DayDelegateExtraInfo.qml:44
msgid "Chance of rain"
msgstr ""
-#: ../app/components/DayDelegate.qml:206
+#: ../app/components/DayDelegateExtraInfo.qml:51
msgid "Winds"
msgstr ""
-#: ../app/components/DayDelegate.qml:213
+#: ../app/components/DayDelegateExtraInfo.qml:59
msgid "UV Index"
msgstr ""
-#: ../app/components/DayDelegate.qml:218
+#: ../app/components/DayDelegateExtraInfo.qml:65
msgid "Pollen"
msgstr ""
-#: ../app/components/DayDelegate.qml:224
+#: ../app/components/DayDelegateExtraInfo.qml:72
msgid "Humidity"
msgstr ""
-#: ../app/components/DayDelegate.qml:230
+#: ../app/components/DayDelegateExtraInfo.qml:79
msgid "Sunrise"
msgstr ""
-#: ../app/components/DayDelegate.qml:236
+#: ../app/components/DayDelegateExtraInfo.qml:86
msgid "Sunset"
msgstr ""
@@ -98,19 +98,19 @@
msgid "Search city"
msgstr ""
-#: ../app/ui/AddLocationPage.qml:293
+#: ../app/ui/AddLocationPage.qml:295
msgid "No city found"
msgstr ""
-#: ../app/ui/AddLocationPage.qml:306
+#: ../app/ui/AddLocationPage.qml:308
msgid "Couldn't load weather data, please try later again!"
msgstr ""
-#: ../app/ui/AddLocationPage.qml:316
+#: ../app/ui/AddLocationPage.qml:318
msgid "Location already added."
msgstr ""
-#: ../app/ui/AddLocationPage.qml:319
+#: ../app/ui/AddLocationPage.qml:321
msgid "OK"
msgstr ""
=== modified file 'tests/autopilot/ubuntu_weather_app/__init__.py'
--- tests/autopilot/ubuntu_weather_app/__init__.py 2015-08-20 14:53:54 +0000
+++ tests/autopilot/ubuntu_weather_app/__init__.py 2015-08-20 19:50:47 +0000
@@ -141,10 +141,10 @@
def get_selected_location_index(self):
return self.get_location_pages().currentIndex
- def get_daydelegate(self, weekdaycolumn, day):
- weekdaycolumn = self.wait_select_single(
- "QQuickColumn", objectName="weekdayColumn" + str(weekdaycolumn))
- return weekdaycolumn.wait_select_single(
+ def get_daydelegate(self, location, day):
+ listview = self.wait_select_single(
+ "LocationPane", objectName="locationListView" + str(location))
+ return listview.wait_select_single(
"DayDelegate", objectName="dayDelegate" + str(day))
@click_object
=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_home_page.py'
--- tests/autopilot/ubuntu_weather_app/tests/test_home_page.py 2015-08-09 16:28:57 +0000
+++ tests/autopilot/ubuntu_weather_app/tests/test_home_page.py 2015-08-20 19:50:47 +0000
@@ -42,6 +42,10 @@
self.assertThat(day_delegate.state, Eventually(Equals("normal")))
home_page.click_daydelegate(day_delegate)
+
+ # Re-get daydelegate as change in loaders changes tree
+ day_delegate = home_page.get_daydelegate(weekdaycolumn, day)
+
day_delegate.height.wait_for(day_delegate.expandedHeight)
self.assertThat(day_delegate.state, Eventually(Equals("expanded")))
self.assertEqual(day_delegate.height, day_delegate.expandedHeight)
Follow ups
