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

[Riccardo Padovani]

Hi Olivier,
thanks for the fast review.

I think I addressed all your requests. I split all fixes in single commits so you can review them one by one, if you prefer

> CI builds are failing because urlManagement.js is missing a copyright header.

Added

> Visually, this looks good except for the separation between the header and the list items. I think you should add a Divider there (https://developer.ubuntu.com/api/qml/sdk-14.10/Ubuntu.Components.ListItems.Divider/).

Indeed, I added it, now looks like a classic page header

> In SettingsPage.qml there are a bunch of references to 'browser', which breaks encapsulation: the SettingsPage component isn’t aware of its parent context. To avoid this, you should add top-level properties to the component for each setting, and in Browser.qml where the component is instantiated, listen to changes on those properties and update the corresponding properties on the Browser instance.

I'm not sure I understood this, I hope I fixed well

> Instead of a historyRemoved() signal, I would add a top-level 'historyModel' property, and have the corresponding action call clearAll() on the model. Also, the opacity shouldn’t be set to 0.5 after clearing. Instead, we need to add a 'count' property to the history model, and bind the opacity to its value being ≠ 0.

Done. I also added a test, as you asked on IRC - it's the first unit test I write, so let me know if could be improved. Also, let me know if we need other tests

> The back button should probably be implemented using an AbstractButton (and it should be highlighted when pressed, to match the standard header).

The higlighted when pressed was a bit tricky to implement, mainly to reproduce the shadow as it's in other apps, with a leftMargin, but I think I achieved that

> "Restore old session on startup" should probably be "Restore previous session at startup"
> "Open new tab in background" should probably be "Allow opening new tabs in background"

My bad english strikes again :-)

> The Column needs to be inside a Flickable, in case all entries don’t fit on the screen

Indeed, I added it also to the privacy page, because I think it will expande in the future

> browser.allowOpenInBackgroundTab is a string, not a boolean

Right, fixed
-- 
https://code.launchpad.net/~rpadovani/webbrowser-app/settings-page/+merge/253975
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.