[Merge] lp:~uriboni/webbrowser-app/keyboard-navigation into lp:webbrowser-app
Ugo Riboni
ugo.riboni at canonical.com
Thu Jun 11 17:12:22 UTC 2015
All done except where commented below:
> - I’m not sure I understand why the test on 'preventSimplifyText' is not
> inside updateUrlFromFocus(). When the address bar has active focus, we always
> want the text to be expanded, right? So the check would be better placed in
> the 'else if' branch of the function, no?
Address bar and suggestion list should be logically considered one single unit in terms of what should happen when they have active focus. However they are two different elements in two different locations in the object tree.
The canSimplifyText exists for this reason: when it is true it means that neither the address bar nor the suggestions list have active focus.
I moved the check inside the updateUrlFromFocus anyway, as it makes the code simpler, but it should not be in the else branch as you suggest.
> As pointed out by Bill in his e-mail review (adding it here for future
> reference), the following need implementing (per design spec):
> - F11 to toggle fullscreen mode
This will be addressed later as we first need to create a proper design that takes into account the difference between application fullscreen and page fullscreen. Tracked by: https://pad.lv/1464333
--
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