← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

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

 

Four functional comments before I review the code.

1. The existing list items contract when the view changes, we should do the same for the today delegate.
2. Currently you can expand today and a future day. You should only be able to expand one.
3. This goes against the initial design, so perhaps we need to get clarification... but I'd rather we put the descriptive weather text for this view as well.
4. Are the added conditions the current conditions when applicable? For instance is the wind speed the current wind speed? Part of me wants to ask Design to change "Today" to "Now" because these conditions should all be the current conditions, IMO.
-- 
https://code.launchpad.net/~ahayzen/ubuntu-weather-app/fix-1496422-1478255-today-extra-info/+merge/272790
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.


References