search

ibm-js / delite

HTML Custom Element / Widget infrastructure
http://ibm-js.github.io/delite/
Other
68 stars 28 forks source link

KeyNav: focus jumps when user clicks in an accordion panel #462

Closed wkeese closed 8 years ago

wkeese commented 8 years ago

There's a bug with deliteful/Accordion where clicking a focusable element inside an Accordion panel can make focus jump to the most recently focused Accordion header. There also appear to be some less serious issues where TAB/SHIFT-TAB will unwantedly stop on the Accordion root node, or the root of an Accordion header.

Why does this happen? Accordion extends KeyNav. It's a complicated case because half of the Accordion children are [role=tab] headers, which match descendantSelector and are navigable via KeyNav up/down arrows, but the other half of the Accordion children are panels, which are not navigable through KeyNav.

This is the current KeyNav focusin handling code:

this.on("focusin", function (evt) {
    if (this.focusDescendants) {
        var target = this._getTargetElement(evt);
        if (target === this) {
            this._keynavFocusHandler(evt);
        } else {
            this._descendantNavigateHandler(target, evt);
        }
    }
}.bind(this), this.keyNavContainerNode);

The main issue is that when an Accordion panel's descendant gets focus, KeyNav calls this._keynavFocusHandler(evt). But _keynavFocusHandler() should only be called when the KeyNav (root node) itself gets focus, because it shifts focus to one of the descendants. Also, its API doc says "Handler for when the container itself gets focus.".

However, whenever a descendant element of the KeyNav is focused, KeyNav should remove its own tabindex, so that Shift-TAB works as expected, i.e. leaving the KeyNav completely without stopping at the KeyNav root node. The code to remove the KeyNav's root node tabIndex is currently in _keynavFocusHandler() and _descendantNavigateHandler().

A third possible issue is that _descendantNavigateHandler() changes the Accordion header from tabindex=-1 to tabindex=0 even when a node inside of the header got focused. That could lead to unwanted tab stops. http://www.oaa-accessibility.org/examplep/accordian1/ shares the same problem: clicking one of the radio buttons and then pressing SHIFT-TAB focuses the first accordion header. It probably shouldn't. But have to read https://www.w3.org/TR/wai-aria-practices-1.1/#accordion carefully as I'm not sure what makes sense here. It probably doesn't matter much since users don't usually mix mouse and keyboard.

Actually, it would be better to temporarily clear the tabIndex on the Accordion header altogether, as Firefox and Safari have issues with focusable elements inside of other focusable elements. On those browsers, clicking the <button> in this test case actually focuses the <div>:

<div tabindex="0" style="width: 300px;">
     i'm a div, including a button:
    <button>click me</button>
</div>

(That's explained on Safari because it doesn't focus <button> by default but Firefox is just weird.)

wkeese commented 5 years ago

Note that on Firefox (at least Firefox 69 on Mac), although you can focus a button by tabbing to it, clicking a button never focuses it. Even in the simple case when the container doesn't have a tabindex, focus will simply go to the <body>:

<div style="width: 300px;">
     i'm a div, including a button:
    <button>click me</button>
</div>

A <span role=button tabindex=0> will work exactly how we want it, both on Firefox and Safari.