Code review comment for lp:~artmello/webbrowser-app/webbrowser-app-bookmarks_view

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

Instead of setting 'enabled' to 'chrome.visible && !bookmarksViewLoader.active' on most of the keyboard shortcuts, the visible property of the top-level FocusScope that contains tabContainer should be updated to be false when bookmarksViewLoader.active.
That way you wouldn’t have to update the enabled property of every single keyboard shortcut, and that would make for much easier maintenance.

Once done, please test all the keyboard shortcuts and ensure that they function only where intended. Please also add/update tests in tests/autopilot/webbrowser_app/tests/test_keyboard.py where relevant.

The Ctrl+T shortcut should be enabled while in the bookmarks view, to be consistent with the fact that the view has a "new tab" toolbar button. Of course opening a new tab should close the view.

review: Needs Fixing

« Back to merge proposal