[Merge] lp:~rpadovani/webbrowser-app/settings-page into lp:webbrowser-app

Olivier Tilloy olivier.tilloy at canonical.com
Tue Mar 31 09:35:24 UTC 2015


Review: Needs Fixing

In src/app/webbrowser/SettingsPageHeader.qml:

 - Does the actual header really need to be a ListItem.Empty ? Can’t it just be a Rectangle?

 - The 'text' property should be of type string, not var

 - Can you rename 'trigger' to 'back' ?

 - Does the ListItem.Divider really need a coloured Rectangle inside itself? Isn’t it themed correctly by default?

 - The top-level item should be a Column, its anchors shouldn’t be defined in the definition of the component itself, and its height should probably be set to childrenRect.height.
-- 
https://code.launchpad.net/~rpadovani/webbrowser-app/settings-page/+merge/253975
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list