[Merge] lp:~uriboni/webbrowser-app/topsite-previews into lp:webbrowser-app
Olivier Tilloy
olivier.tilloy at canonical.com
Wed Oct 7 16:21:53 UTC 2015
Review: Needs Fixing
In BrowserTab.qml, 'topSites' and 'cachedPreviewUpdated' appear to be unused, can they be removed?
In HistoryViewWide.qml, the diff introduces an extraneous blank line before lastVisitDateDelegate’s onClicked handler, can this be reverted?
In NewTabView.qml, the comment to explain why there is a negative right margin makes sense, but instead of hardcoding it, couldn’t it be bound to "-contentColumn.anchors.rightMargin" ?
In UrlPreviewGrid.qml, do you really need to implement keyboard navigation yourself? I thought GridView already implemented it.
PreviewManager.qml should import Ubuntu.Web 0.2, as it has a reference to cacheLocation.
src/app/webbrowser/qmldir needs to be installed by src/app/webbrowser/CMakeLists.txt
In tests/autopilot/webbrowser_app/tests/__init__.py, it doesn’t look like the result of launch_app() is used anywhere, so the return value could be suppressed.
In test_cleanup_previews_on_startup, is the sleep really necessary? If so, how can we be sure that 0.5 seconds is enough?
In tst_BrowserTab.qml, is the additional import of Ubuntu.Test really needed?
3 out of the 4 tests in webbrowser_app.tests.test_site_previews fail in narrow mode because they silently assume wide mode (in narrow mode, before calling self.open_new_tab(), the test needs to call self.open_tabs_view()).
--
https://code.launchpad.net/~uriboni/webbrowser-app/topsite-previews/+merge/269771
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.
More information about the Ubuntu-reviews
mailing list