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

[Riccardo Padovani]

> If I have a number of bookmarks (more than 5), then open a new tab and start
> removing all bookmarks by swiping them to the right and confirming, they are
> correctly removed (and following bookmarks in the list show up), but the
> height of the bookmarks section is not updated, it doesn’t shrink when the
> total number of bookmarks gets < 5. Eventually, when only one bookmark is
> left, there is a big, almost empty bookmarks section, which looks akward. If I
> then open another new tab, the section’s height is correct.

Actually, I wasn't able to reproduce this. Are you on vivid?

But I reproduced a similar issue: if I've 2 new tab view open, and I remove bookmarks in the 2nd one, the height in the 1st isn't updated. I fixed that, so I hope it fixes also the one you found.
Also, I found a similar issue with 'See more' - expand in one tab, remove bookmarks in the other one, go back - but I think I fixed also this one

> There should be some spacing above the message that says that the user hasn’t
> visited any sites yet, it’s currently stuck to the section separator, which
> looks ugly. And the text color probably shouldn’t be plain black.

Done, and now the text it's of the same color of 'See more' button


> numberOfBookmarks and numberOfTopSites should check that bookmarksModel and
> historyModel are not null before trying to read their 'count' properties. This
> is because those models are instantiated asynchronously at startup. If a blank
> tab is restored at startup and for some reason the models take a long time to
> load, you will see (harmless) warnings in the console. Those are easy enough
> to fix that it’s worth doing it.

Done

> There should be some spacing between the bottom of the top sites list and the
> bottom of the window, to account for the bottom edge hint on mobile (I guess
> it’s ok to have that spacing unconditionally, even on desktop where it’s not
> strictly needed).

Are you sure? Having a gray line at bottom is ugly, and the effect is very strange.
Take a look to Dekko or Reminders, they don't have any space at bottom, and looks good anyway

> In the UrlList for top sites, why do you increase the limit in the
> onUrlRemoved handler?

Legacy code from the first implementation, thanks, removed

> Can you update the unit tests for the bookmarks model to check that the
> rowCountChanged signal is correctly emitted when expected, and that the value
> of the count is always as expected?

Done
-- 
https://code.launchpad.net/~rpadovani/webbrowser-app/newTabRefactoring/+merge/247498
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.