[Merge] lp:~uriboni/webbrowser-app/keyboard-navigation into lp:webbrowser-app

Olivier Tilloy olivier.tilloy at canonical.com
Tue Jun 9 10:34:54 UTC 2015


Review: Needs Fixing

ESC in the tabs view (recentView) and in the settings page should exit the view (I think it’s ok if more advanced keyboard navigation is not implemented in those views yet, but at the very least it should be possible to leave them using the keyboard).

It might be interesting to have Ctrl+T, Ctrl+W and Ctrl+Tab work while recentView is visible.

The current implementation of Ctrl+Tab doesn’t switch to the next tab, it switches to the last one, is that intended?

I’m not fond of the name of the 'textLocked' property: it kind of suggests that the text cannot be modified, which isn’t true, because keyboard input will modify the text. Can we think of a more appropriate name?

In internal.switchToTab(), you don’t want to focus the address bar when the tab is empty on devices (you only want that behaviour on desktop, because on mobile the OSK would get in the way of the new tab view).

In the definition of LimitProxyModel::get(), there is an extra blank line that should be removed.

In Browser.press_key(), is it right to create a Keyboard instance everytime we press a key? Shouldn’t the instance be created in Browser.__init__() instead? And why is there a try… except block? When can we get a RuntimeError for pressing a key? Silently swallowing exceptions is usually not a good practice, except in very specific cases.

Can PrepopulatedDatabaseTestCaseBase be factored out in a single base class that all tests can re-use?

According to https://docs.python.org/3.3/library/unittest.html#skipping-tests-and-expected-failures, "Classes can be skipped just like methods", so no need to add a skipIf decorator on every single test method in the TestKeyboard class, the entire class should be skipped.
-- 
https://code.launchpad.net/~uriboni/webbrowser-app/keyboard-navigation/+merge/260183
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list