From 192e80c03e18387f94edd171a1f11477c91e9a80 Mon Sep 17 00:00:00 2001 From: Bruno Boi Date: Thu, 10 Nov 2022 10:27:22 +0000 Subject: [PATCH] [FIX] web: dropdown unusable by screen readers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dropdown menus/items were refactored and moved to OWL when migrating to 15.0. Now, the focus is always on the dropdown button, which captures and handles all keyboard events, to be able to move over the menus. Currently-selected item is highlighted by toggling classes. However, since the focus is always on the dropdown button, screen reader users are no longer able to know what menu item is being selected, as it's only a visual change (in previous versions, the focus actually moved to the selected element). The above is fixed by making the activeElement follow the visual change. Part-of: odoo/odoo#106277 Co-authored-by: Luis González --- .../web/static/src/core/dropdown/dropdown.js | 7 ++ .../static/src/core/dropdown/dropdown.scss | 1 + .../src/core/dropdown/dropdown_item.xml | 1 + .../core/dropdown/dropdown_navigation_hook.js | 14 +++- .../web/static/tests/core/dropdown_tests.js | 80 ++++++++++++++++++- 5 files changed, 97 insertions(+), 6 deletions(-) diff --git a/addons/web/static/src/core/dropdown/dropdown.js b/addons/web/static/src/core/dropdown/dropdown.js index acc8ebcd3f7..e0aa6a1dc3e 100644 --- a/addons/web/static/src/core/dropdown/dropdown.js +++ b/addons/web/static/src/core/dropdown/dropdown.js @@ -133,6 +133,12 @@ export class Dropdown extends Component { } this.toggle(); }; + if (this.rootRef.el.parentElement.tabIndex === -1) { + // If the parent is not focusable, make it focusable programmatically. + // This code may look weird, but an element with a negative tabIndex is + // focusable programmatically ONLY if its tabIndex is explicitly set. + this.rootRef.el.parentElement.tabIndex = -1; + } this.rootRef.el.parentElement.addEventListener("click", onClick); return () => { this.rootRef.el.parentElement.removeEventListener("click", onClick); @@ -261,6 +267,7 @@ export class Dropdown extends Component { */ onTogglerMouseEnter() { if (this.state.groupIsOpen && !this.state.open) { + this.togglerRef.el.focus(); this.open(); } } diff --git a/addons/web/static/src/core/dropdown/dropdown.scss b/addons/web/static/src/core/dropdown/dropdown.scss index 19f68b203c9..a6d8ecca978 100644 --- a/addons/web/static/src/core/dropdown/dropdown.scss +++ b/addons/web/static/src/core/dropdown/dropdown.scss @@ -3,6 +3,7 @@ .dropdown-menu { .dropdown-toggle:focus, .dropdown-item:focus { background-color: transparent; + outline: none; } .dropdown-toggle.focus, .dropdown-item.focus { background-color: $dropdown-link-hover-bg; diff --git a/addons/web/static/src/core/dropdown/dropdown_item.xml b/addons/web/static/src/core/dropdown/dropdown_item.xml index 38d7c332393..bc2660593af 100644 --- a/addons/web/static/src/core/dropdown/dropdown_item.xml +++ b/addons/web/static/src/core/dropdown/dropdown_item.xml @@ -11,6 +11,7 @@ t-att-title="props.title" t-att-data-hotkey="props.hotkey" t-att="dataAttributes" + tabindex="0" > diff --git a/addons/web/static/src/core/dropdown/dropdown_navigation_hook.js b/addons/web/static/src/core/dropdown/dropdown_navigation_hook.js index 154251f0118..254420a0601 100644 --- a/addons/web/static/src/core/dropdown/dropdown_navigation_hook.js +++ b/addons/web/static/src/core/dropdown/dropdown_navigation_hook.js @@ -114,6 +114,7 @@ export function useDropdownNavigation() { } // Make myself active navTarget.classList.add(ACTIVE_MENU_ELEMENT_CLASS); + navTarget.focus(); }; /** @type {MenuElement} */ @@ -200,7 +201,16 @@ export function useDropdownNavigation() { // Set up keyboard navigation ---------------------------------------------- const hotkeyService = useService("hotkey"); - const closeSubDropdown = comp.parentDropdown ? comp.close : () => {}; + const closeAndRefocus = () => { + const toFocus = + comp.props.toggler === "parent" + ? comp.rootRef.el.parentElement + : comp.rootRef.el.querySelector(":scope > .dropdown-toggle"); + comp.close().then(() => { + toFocus.focus(); + }); + }; + const closeSubDropdown = comp.parentDropdown ? closeAndRefocus : () => {}; const openSubDropdown = () => { const menuElement = getActiveMenuElement(); // Active menu element is a sub dropdown @@ -229,7 +239,7 @@ export function useDropdownNavigation() { arrowleft: localization.direction === "rtl" ? openSubDropdown : closeSubDropdown, arrowright: localization.direction === "rtl" ? closeSubDropdown : openSubDropdown, enter: selectActiveMenuElement, - escape: comp.close, + escape: closeAndRefocus, }; useEffect( (open) => { diff --git a/addons/web/static/tests/core/dropdown_tests.js b/addons/web/static/tests/core/dropdown_tests.js index f4bc7efe4a4..8909654d3fd 100644 --- a/addons/web/static/tests/core/dropdown_tests.js +++ b/addons/web/static/tests/core/dropdown_tests.js @@ -64,7 +64,7 @@ QUnit.module("Components", ({ beforeEach }) => { await mount(Parent, target, { env }); assert.strictEqual( target.querySelector(".dropdown-item").outerHTML, - 'coucou' + 'coucou' ); }); @@ -76,7 +76,7 @@ QUnit.module("Components", ({ beforeEach }) => { await mount(Parent, target, { env }); assert.strictEqual( target.querySelector(".dropdown-item").outerHTML, - 'coucou' + 'coucou' ); }); @@ -589,8 +589,31 @@ QUnit.module("Components", ({ beforeEach }) => { assert.containsOnce(target, ".dropdown-menu"); }); + QUnit.test("siblings dropdowns: toggler focused on mouseenter", async (assert) => { + class Parent extends Component { } + Parent.template = xml` +
+ + +
+ `; + Parent.components = { Dropdown }; + env = await makeTestEnv(); + await mount(Parent, target, { env }); + // Click on one + target.querySelector(".one button").focus(); // mocks a real click flow + await click(target, ".one button"); + assert.strictEqual(document.activeElement, target.querySelector(".one button")); + assert.containsOnce(target, ".dropdown-menu"); + // Hover on two + const two = target.querySelector(".two"); + two.querySelector("button").dispatchEvent(new MouseEvent("mouseenter")); + await nextTick(); + assert.strictEqual(document.activeElement, two.querySelector("button")); + }); + QUnit.test("dropdowns keynav", async (assert) => { - assert.expect(26); + assert.expect(41); class Parent extends Component { onItemSelected(value) { assert.step(value.toString()); @@ -645,6 +668,10 @@ QUnit.module("Components", ({ beforeEach }) => { triggerHotkey(step.hotkey); await nextTick(); assert.hasClass(target.querySelector(".dropdown-menu > .focus"), step.expected); + assert.strictEqual( + document.activeElement, + target.querySelector(".dropdown-menu > .focus") + ); } // Select last one activated in previous scenario (item1) @@ -738,8 +765,39 @@ QUnit.module("Components", ({ beforeEach }) => { assert.containsN(target, ".dropdown .dropdown-menu .dropdown-item", 2); }); + QUnit.test("props toggler='parent': refocus toggler on close with keynav", async (assert) => { + class Parent extends Component {} + Parent.template = xml` +
+
+ Click Me + + Element 1 + Element 2 + +
+
`; + Parent.components = { Dropdown, DropdownItem }; + + env = await makeTestEnv(); + await mount(Parent, target, { env }); + assert.strictEqual(document.activeElement, document.body); + target.querySelector(".my_custom_toggler").focus(); // mocks a real click flow + await click(target, ".my_custom_toggler"); + assert.strictEqual(document.activeElement, target.querySelector(".my_custom_toggler")); + triggerHotkey("ArrowDown"); + await nextTick(); + assert.strictEqual( + document.activeElement, + target.querySelector(".dropdown-item:first-child") + ); + triggerHotkey("Escape"); + await nextTick(); + assert.strictEqual(document.activeElement, target.querySelector(".my_custom_toggler")); + }); + QUnit.test("multi-level dropdown: keynav", async (assert) => { - assert.expect(125); + assert.expect(213); class Parent extends Component { onItemSelected(value) { assert.step(value); @@ -830,6 +888,20 @@ QUnit.module("Components", ({ beforeEach }) => { for (const element of activeElements) { assert.hasClass(element, step.highlighted[index++]); } + const lastActiveElement = activeElements.slice(-1)[0]; + if (lastActiveElement) { + assert.hasClass(lastActiveElement, step.highlighted.slice(-1)[0]); + assert.strictEqual( + document.activeElement, + lastActiveElement.classList.contains("dropdown") + ? lastActiveElement.querySelector(":scope > .dropdown-toggle") + : lastActiveElement + ); + } else { + // no active element means that the main dropdown is closed + assert.hasClass(document.activeElement, "dropdown-toggle"); + assert.hasClass(document.activeElement.parentElement, "first"); + } } if (step.selected !== undefined) { const verify = step.selected === false ? [] : [step.selected];