Merge lp://staging/~sinzui/launchpad/bugs-subscriptons-wcag-love into lp://staging/launchpad

Proposed by Curtis Hovey
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 16373
Proposed branch: lp://staging/~sinzui/launchpad/bugs-subscriptons-wcag-love
Merge into: lp://staging/launchpad
Diff against target: 299 lines (+113/-19)
9 files modified
lib/lp/app/javascript/confirmationoverlay/assets/confirmationoverlay-core.css (+0/-4)
lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css (+0/-4)
lib/lp/app/javascript/formoverlay/formoverlay.js (+29/-4)
lib/lp/app/javascript/formoverlay/tests/test_formoverlay.js (+33/-0)
lib/lp/app/javascript/overlay/overlay.js (+6/-3)
lib/lp/app/javascript/overlay/tests/test_overlay.js (+3/-0)
lib/lp/registry/javascript/structural-subscription.js (+4/-1)
lib/lp/registry/javascript/tests/test_structural_subscription.html (+2/-0)
lib/lp/registry/javascript/tests/test_structural_subscription.js (+36/-3)
To merge this branch: bzr merge lp://staging/~sinzui/launchpad/bugs-subscriptons-wcag-love
Reviewer Review Type Date Requested Status
j.c.sackett (community) Approve
Review via email: mp+139812@code.staging.launchpad.net

Commit message

Fix the bugs subscription overlay's keyboard navigation.

Description of the change

The javascript controls for bug subscription are hard to use with screen
readers. The overlay does not take focus, the user can tab out of the
overlay, the accordion widget does not expand/contract as expected.

RULES

    Pre-implementation: wgrant, stevenk
    * Make the bug subscription widget uses the features provided by
      its base classes.
      * The first element is not being focused.
      * The user can tab out of the overlay because the list of tabable
        elements disagrees with what the browser supports...it's my
        arch nemesis visibility=hidden.
      * The problem is compounded by the overflow=hidden rules for
        the accordion. The item is not visible to the sighted user,
        but it is officially visible and was given space.

QA

    * Visit https://qastaging.launchpad.net/pocket-lint/trunk
    * View the Create milestone form and immediately press esc.
    * Verify the overlay closes, maybe you saw the close button in the
      upper right.
    * View the Create milestone form and press tab.
    * Verify the name field is focused.
    * View the Subscribe to bug mail link
    * press tab and verify the focus moves to the team select, but the
      team radio is NOT selected.
    * Tab a dozen times and confirm that the tabs cycle though the widget.
    * Tab to "are added or changed in any way", press enter, then
      tab a dozen times to cycle though the links again.
    * Tab to "Bugs must match this filter" and press enter.
    * Tab many times and verify that you tab into unseen links :(,
      but you can persevere and tab to the Create button.

LINT

    lib/lp/app/javascript/confirmationoverlay/assets/confirmationoverlay-core.css
    lib/lp/app/javascript/formoverlay/formoverlay.js
    lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css
    lib/lp/app/javascript/formoverlay/tests/test_formoverlay.js
    lib/lp/app/javascript/overlay/overlay.js
    lib/lp/app/javascript/overlay/tests/test_overlay.js
    lib/lp/registry/javascript/structural-subscription.js
    lib/lp/registry/javascript/tests/test_structural_subscription.html
    lib/lp/registry/javascript/tests/test_structural_subscription.js

LoC

    I have more than 10,000 lines of credit this week.

TEST

    ./bin/test -vvc --layer=YUITest -t subscription -t overlay lp

IMPLEMENTATION

I updated the overlay's tabbing rules to also exclude items that the
browser computes to be hidden. This fixed most of the tab problems within
the overlay if a field ever got focus. When the accordion is visible,
it is still possible to tab into a hidden link/field :(. Blind users are
fine, users with pointers are fine, users not filtering are fine, sighted
keyboard users doing filtering will be annoyed if they try to skip a
section. At least they can use the keyboard.
    lib/lp/app/javascript/overlay/overlay.js
    lib/lp/app/javascript/overlay/tests/test_overlay.js

OMG. The code tries to focus non-visible inputs like <input type=hidden
/>. Even when the proper elements were focuses. There was a corner-case
though in the formoverlay/overlay widget. It is possible to show the
overlay, but there is no elements that can be focused, so I removed the
default rule that hide the close button. The close button is needed is
made the focus in most overlay cases, but a few overlays hid it -- which
has been noted as a inconsistency in the past. The CSS fix ensures all
overlays will get focus and can be closed with esc before any content it
loaded.
    lib/lp/app/javascript/formoverlay/formoverlay.js
    lib/lp/app/javascript/formoverlay/tests/test_formoverlay.js
    lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css
    lib/lp/app/javascript/confirmationoverlay/assets/confirmationoverlay-core.css

The SS widget was hacking content into the overlay. The SS code replaces
the supposed download form with its own. The simple fix was to require
the focus rules which I extracted to a method in the formoverlay. There
was a nasty issue where the user tabs from the personal email radio button
to the team select box, which inadvertently then set the user's choice
to team email. The documentation said the code was doing the radio change
on the select box change, so it updated the event to work with onchange
per the docs.
    lib/lp/registry/javascript/structural-subscription.js
    lib/lp/registry/javascript/tests/test_structural_subscription.html
    lib/lp/registry/javascript/tests/test_structural_subscription.js

To post a comment you must log in.
Revision history for this message
j.c.sackett (jcsackett) wrote :

This looks good, Curtis. Thanks.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.