Merge lp:~ahayzen/ubuntu-weather-app/reboot-uc1.3-bump into lp:ubuntu-weather-app
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Approved by: | Victor Thompson on 2015-11-26 | ||||
| Approved revision: | 166 | ||||
| Merged at revision: | 180 | ||||
| Proposed branch: | lp:~ahayzen/ubuntu-weather-app/reboot-uc1.3-bump | ||||
| Merge into: | lp:ubuntu-weather-app | ||||
| Diff against target: |
776 lines (+93/-67) 42 files modified
app/components/CurrentLocation.qml (+1/-1) app/components/DayDelegate.qml (+1/-1) app/components/DayDelegateExtraInfo.qml (+1/-1) app/components/ExpandableListItem.qml (+1/-1) app/components/FakeHeader.qml (+4/-4) app/components/FastScroll.qml (+1/-1) app/components/ForecastDetailsDelegate.qml (+1/-1) app/components/HeaderRow.qml (+1/-1) app/components/HomeGraphic.qml (+1/-1) app/components/HomeHourly.qml (+1/-1) app/components/HomePageEmptyStateComponent.qml (+1/-1) app/components/HomeTempInfo.qml (+1/-1) app/components/ListItemActions/CheckBox.qml (+1/-1) app/components/ListItemActions/Remove.qml (+1/-1) app/components/ListItemReorderComponent.qml (+1/-1) app/components/ListItemWithActions.qml (+1/-1) app/components/LoadingIndicator.qml (+1/-1) app/components/LocationsPageEmptyStateComponent.qml (+1/-1) app/components/MultiSelectHeadState.qml (+1/-1) app/components/MultiSelectListView.qml (+1/-1) app/components/NetworkErrorStateComponent.qml (+1/-1) app/components/NoAPIKeyErrorStateComponent.qml (+1/-1) app/components/PageWithBottomEdge.qml (+21/-10) app/components/SettingsButton.qml (+1/-1) app/components/StandardListItem.qml (+1/-1) app/components/WeatherListItem.qml (+1/-1) app/components/WeatherListView.qml (+1/-1) app/ubuntu-weather-app.qml (+1/-1) app/ui/AddLocationPage.qml (+8/-6) app/ui/HomePage.qml (+7/-1) app/ui/LocationPane.qml (+1/-1) app/ui/LocationsPage.qml (+2/-2) app/ui/SettingsPage.qml (+1/-1) app/ui/settings/DataProviderPage.qml (+1/-1) app/ui/settings/LocationPage.qml (+1/-1) app/ui/settings/RefreshIntervalPage.qml (+1/-1) app/ui/settings/UnitsPage.qml (+1/-1) debian/changelog (+1/-0) manifest.json.in (+1/-1) po/com.ubuntu.weather.pot (+6/-6) tests/autopilot/ubuntu_weather_app/__init__.py (+10/-5) ubuntu-weather-app.desktop.in.in (+1/-0) |
||||
| To merge this branch: | bzr merge lp:~ahayzen/ubuntu-weather-app/reboot-uc1.3-bump | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Victor Thompson | Approve on 2015-11-26 | ||
| Jenkins Bot | continuous-integration | Approve on 2015-11-26 | |
| Bartosz Kosiorek | 2015-11-06 | Approve on 2015-11-11 | |
| Ubuntu Phone Apps Jenkins Bot | continuous-integration | Approve on 2015-11-03 | |
| Andrew Hayzen | Abstain on 2015-11-03 | ||
|
Review via email:
|
|||
Commit Message
* Update app to use the Ubuntu Components version 1.3
Description of the Change
* Update app to use the Ubuntu Components version 1.3
I think I've managed to tweak the FakeHeader and PageWithBottomEdge to match the new header height and styling requirements :-)
Please test AP locally as well.
FAILED: Continuous integration, rev:154
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
- 155. By Andrew Hayzen on 2015-11-02
-
* Merge of trunk
- 156. By Andrew Hayzen on 2015-11-02
-
* Update changelog
PASSED: Continuous integration, rev:156
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:156
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild:
http://
PASSED: Continuous integration, rev:156
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Andrew Hayzen (ahayzen) wrote : | # |
AP tests failing alot due to bottomEdge .isReady being incorrect and other issues.
- 157. By Andrew Hayzen on 2015-11-03
-
* Tweaks to bottomEdge for autopilot
PASSED: Continuous integration, rev:157
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
PASSED: Continuous integration, rev:157
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild:
http://
PASSED: Continuous integration, rev:157
http://
Executed test runs:
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Victor Thompson (vthompson) wrote : | # |
IMO the startup animation where the header hides and the animation of the header when swiping from the bottom edge are a bit annoying.
It would at least seem like the animation of closing the fake header has the wrong height (height of the old header) initially.
Other than that this seems mostly fine, as I can't find any actual regressions.
| Bartosz Kosiorek (gang65) wrote : | # |
I would like to test weather app, but unfortunately I have an errors:
qml: Sent request URL: http://
qml: retry of http://
qml: Sent request URL: http://
qml: {"msg":"wrong response http code, got 401","request"
qml: wrong response http code, got 401 / http://
qml: {"msg":"wrong response http code, got 401","request"
According to documentation:
http://
Starting from 9 October 2015 our API requires a valid APPID for access. Note that this does not mean that our API is subscription-only now - please take a minute to register a free account to receive a key.
We are sorry for inconvenience but this is a necessary measure that will help us deliver our services to you faster and more reliably.
For FOSS developers: we welcome free and open source software and are willing to help you. If you want to use OWM data in your free software application please register an API key and file a ticket describing your application and API key registered. OWM will review your request lift access limits for your key if used in open source application.
@Victor Do you know how to resolve this issue?
| Andrew Hayzen (ahayzen) wrote : | # |
@Bartosz, we now have a MP [0] which adds instructions of how to add an OWM key :-)
0 - https:/
| Bartosz Kosiorek (gang65) wrote : | # |
Thanks Andrew.
I made some testing of this branch and here is what I found:
1. Missing title for Desktop. With new SDK, the title is working correctly if you hide header
title: i18n.tr("Weather")
2. There are obsolete warning:
file://
3. There are undefined types errors in Location Pane:
file://
file://
file://
file://
4. There are undefined types errors in HomeTempInfo.qml:
file://
file://
file://
file://
file://
file://
file://
file://
file://
file://
file://
file://
file://
file://
file://
file://
| Bartosz Kosiorek (gang65) wrote : | # |
There are also following deprecate warnings:
file://
file://
file://
file://
- 158. By Andrew Hayzen on 2015-11-11
-
* Fix deprecate warnings and title for desktop
- 159. By Andrew Hayzen on 2015-11-11
-
* Merge of trunk
| Andrew Hayzen (ahayzen) wrote : | # |
1) Added title to the HomePage.qml
2) Fixed the bottomEdge issues
3) Not been able to reproduce this one yet, could you provide exact steps
4) Same as 3)
5) These look like issues due to the old listitems (note /Ubuntu/
Meanwhile there appear to be offset issues in the LocationPage when switching on/off the auto detect location, which I believe partially existed before. I have minimised the issues when actually on the LocationPage, but it looks like I need to force the bottomEdge hint to reload when the location detection is toggled. As the offset is incorrect after switching until the app is restarted. I'll investigate this :-)
PASSED: Continuous integration, rev:159
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
| Bartosz Kosiorek (gang65) wrote : | # |
See my inline comments.
Also after clicking on city (from city list), I have following error:
file://
| Bartosz Kosiorek (gang65) wrote : | # |
@Andrew @Victor
Could you please also take a look at:
https:/
:-)
Thanks
| Andrew Hayzen (ahayzen) wrote : | # |
@Bartosz, as the PageWithBottomEdge is an 'upstream' component to us, any modifications we make to the code we try to add a // CUSTOM comment next to them, and any code we remove we try to comment out rather than remove. This is to ease upgrading and help merge conflicts when changes are made upstream, so in this case we would comment out that block and add // CUSTOM as the block is upstream here [2].
Also IIRC the PageWrapperUtil
| Victor Thompson (vthompson) wrote : | # |
Please also bump the new NoAPIKeyErrorSt
I still see all the same odd header behaviors.
I'm not sure I agree with using "Weather" as the title. The title on the desktop did not change with this MP, so fixing it here doesn't seem necessary. I don't think the user should see "Weather" as the header is hidden/shown.
I think you need to rebuild the pot file? It seems your mp is removing some stuff.
- 160. By Andrew Hayzen on 2015-11-12
-
* Bump merged component
PASSED: Continuous integration, rev:160
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
- 161. By Andrew Hayzen on 2015-11-12
-
* Update .pot file
PASSED: Continuous integration, rev:161
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
- 162. By Andrew Hayzen on 2015-11-26
-
* Show header in splash to minimise slide up effect
PASSED: Continuous integration, rev:162
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
- 163. By Andrew Hayzen on 2015-11-26
-
* Remove Weather from title for now due to incorrect header animation
PASSED: Continuous integration, rev:163
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
- 164. By Andrew Hayzen on 2015-11-26
-
* Reduce bounce animation of content when pushing a page
PASSED: Continuous integration, rev:164
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
- 165. By Andrew Hayzen on 2015-11-26
-
* Fix the bottom edge hint topMargin being incorrect on the second pull
PASSED: Continuous integration, rev:165
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
- 166. By Andrew Hayzen on 2015-11-26
-
* Add extra code commentary
PASSED: Continuous integration, rev:166
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
| Victor Thompson (vthompson) wrote : | # |
Thanks for the extra fixes and code commentary. I think we still should work on resolving the issues with the FakeHeader, but for now this is great... as our header/BottomEdge transition wasn't consistent with all other apps previously, so I don't mind it being somewhat "different" for the time being.


FAILED: Continuous integration, rev:154 91.189. 93.70:8080/ job/ubuntu- weather- app-reboot- ci/289/ 91.189. 93.70:8080/ job/ubuntu- weather- app-reboot- vivid-amd64- ci/289/ console
http://
Executed test runs:
FAILURE: http://
Click here to trigger a rebuild: 91.189. 93.70:8080/ job/ubuntu- weather- app-reboot- ci/289/ rebuild
http://