Code review comment for lp:~rpadovani/webbrowser-app/settings-page

Revision history for this message
Olivier Tilloy (osomon) wrote :

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.

review: Needs Fixing

« Back to merge proposal