launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Curtis Hovey has proposed merging lp:~sinzui/launchpad/choicesource-tab into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1019064 in Launchpad itself: "Users cannot use keyboard to navigate the choicesource picker"
  https://bugs.launchpad.net/launchpad/+bug/1019064

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/choicesource-tab/+merge/112881

We discovered that we cannot use the keyboard to tab or use arrow keys
to move between the links in the choicesource pickers added the to
+filebug. There is nothing special about the pickers...keyboard
navigation was broken on every form a select menu was replaced with.

--------------------------------------------------------------------

RULES

    Pre-implementation: no one
    * Discover what is missing or what was added that broken tabbing.

    Addendum
    * It is possible to tab out of the so-called modal overview. This
      causes confusion because you can tab to elements underneath the
      overlay.
    * Prevent users from tabbing out of the overlay. The user must
      dismiss it using the escape key or the close button.


QA

    You can see a demo of the broken and fixes cases at
    http://people.canonical.com/~curtis/broken-and-fix-tabs.ogv

    On qastaging:
    * Visit https://bugs.qastaging.launchpad.net/gdp/+bug/932086
    * Tab to the status action, press enter.
    * Used tab to cycle through the links.
    * Use shift+tab to cycle backwards though the links.
    * Press enter on the status you want.
    * Verify the bug's status was changed.
    * Tab to the assignee, press enter.
    * Search for curtis
    * Used tab to cycle through the links.
      Verifying that the hidden links in the picker do cause you to
      tab outside of the overlay.
    * Tab to sinzui's email address and press enter to expand.
    * Tab to the Select link and press enter.
    * Verify sinzui was selected.


LINT

    lib/lp/app/javascript/choiceedit/choiceedit.js
    lib/lp/app/javascript/overlay/overlay.js
    lib/lp/app/javascript/overlay/assets/skins/sam/pretty-overlay-skin.css
    lib/lp/app/javascript/overlay/tests/test_overlay.js


TEST

    ./bin/test -vv -t test_choiceedit -t test_overlay lp.app.tests.test_yui


IMPLEMENTATION

Removed bad code that caused the close button to steal focus.
    lib/lp/app/javascript/choiceedit/choiceedit.js

Fixed the CSS for the close button, which was empty, invalid, and unusable
by screen readers.
    lib/lp/app/javascript/overlay/assets/skins/sam/pretty-overlay-skin.css

Add a handler to watch the tab key. When you tab past the last node, focus
it moved to the first node. If you shift-tab before the first node, focus
is moved to the last node. This is trickier for person and target pickers
because they have hidden nodes that you can expand to see. I wrote a
method to locate and filter the links that you cannot see...those that
the browser did not allocate height to on the page. As you may note from
one of the tests, we cannot check the elements style of computed style
because the parent element may cause it to be hidden.
    lib/lp/app/javascript/overlay/overlay.js
    lib/lp/app/javascript/overlay/tests/test_overlay.js
-- 
https://code.launchpad.net/~sinzui/launchpad/choicesource-tab/+merge/112881
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/choicesource-tab into lp:launchpad.