From 5b2b801208a86c1d342feff49961f5c490ad247e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Collonval?= Date: Wed, 20 Mar 2024 05:14:57 +0100 Subject: [PATCH] Fix toolbar stealing focus (#6888) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix toolbar stealing focus * Implement suggestion by @scomea * Add test * Use 'getRootActiveElement' * Change files * Apply review comment --------- Co-authored-by: Frédéric Collonval Co-authored-by: Stephane Comeau --- ...-65238abc-a169-4169-a830-9dcdb4e9af99.json | 7 +++ .../src/toolbar/toolbar.pw.spec.ts | 62 +++++++++++++++++++ .../fast-foundation/src/toolbar/toolbar.ts | 10 ++- 3 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 change/@microsoft-fast-foundation-65238abc-a169-4169-a830-9dcdb4e9af99.json diff --git a/change/@microsoft-fast-foundation-65238abc-a169-4169-a830-9dcdb4e9af99.json b/change/@microsoft-fast-foundation-65238abc-a169-4169-a830-9dcdb4e9af99.json new file mode 100644 index 00000000000..f9acba00247 --- /dev/null +++ b/change/@microsoft-fast-foundation-65238abc-a169-4169-a830-9dcdb4e9af99.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Fix toolbar stealing focus", + "packageName": "@microsoft/fast-foundation", + "email": "fcollonval@users.noreply.github.com", + "dependentChangeType": "prerelease" +} diff --git a/packages/web-components/fast-foundation/src/toolbar/toolbar.pw.spec.ts b/packages/web-components/fast-foundation/src/toolbar/toolbar.pw.spec.ts index e450ad5770d..85548f8e0fb 100644 --- a/packages/web-components/fast-foundation/src/toolbar/toolbar.pw.spec.ts +++ b/packages/web-components/fast-foundation/src/toolbar/toolbar.pw.spec.ts @@ -479,4 +479,66 @@ test.describe("Toolbar", () => { await button3.click(); await expect(button3).toBeFocused(); }); + + test("should not focus clicked item when focus is moved outside of the toolbar by an event handler", async () => { + await root.evaluate(node => { + node.innerHTML = /* html */ ` + + + + + + + + + + + `; + }); + + const buttonOutsideToolbar = page.locator("button", { hasText: "Button Outside Toolbar"}); + const button1 = element.locator("button", { hasText: "Button 1" }); + const button2 = element.locator("button", { hasText: "Button 2" }); + const button3 = element.locator("button", { hasText: "Button 3" }); + + const wasButton1Clicked = await button1.evaluate(node => new Promise(resolve => { + node.addEventListener("mousedown", (e: MouseEvent) => { + document.querySelector('#outside-button')?.focus(); + resolve(true) + }) + + node.dispatchEvent(new MouseEvent("mousedown", {bubbles: true, cancelable: true})) + })) + + expect.soft(wasButton1Clicked).toEqual(true) + await expect.soft(buttonOutsideToolbar).toBeFocused(); + buttonOutsideToolbar.evaluate(node => {node.blur()}) + + const [wasButton2Clicked] = await Promise.all([ + button2.evaluate(node => new Promise(resolve => { + node.addEventListener("click", () => { + document.querySelector('#outside-button')?.focus(); + resolve(true) + }) + })), + button2.click() + ]) + + expect.soft(wasButton2Clicked).toEqual(true) + await expect.soft(buttonOutsideToolbar).toBeFocused(); + buttonOutsideToolbar.evaluate(node => {node.blur()}) + + const [wasButton3Clicked] = await Promise.all([ + button3.evaluate(node => new Promise(resolve => { + node.addEventListener("click", () => { + resolve(true) + }) + })), + button3.click() + ]) + + expect.soft(wasButton3Clicked).toEqual(true) + await expect.soft(buttonOutsideToolbar).not.toBeFocused(); + await expect(button3).toBeFocused(); + }); }); diff --git a/packages/web-components/fast-foundation/src/toolbar/toolbar.ts b/packages/web-components/fast-foundation/src/toolbar/toolbar.ts index df230a91063..2bdcc16d95a 100644 --- a/packages/web-components/fast-foundation/src/toolbar/toolbar.ts +++ b/packages/web-components/fast-foundation/src/toolbar/toolbar.ts @@ -4,6 +4,7 @@ import { isFocusable } from "tabbable"; import { ARIAGlobalStatesAndProperties, StartEnd } from "../patterns/index.js"; import { applyMixins } from "../utilities/apply-mixins.js"; import { getDirection } from "../utilities/direction.js"; +import { getRootActiveElement } from "../utilities/root-active-element.js"; import { ToolbarOrientation } from "./toolbar.options.js"; /** @@ -246,7 +247,14 @@ export class FASTToolbar extends FASTElement { private setFocusedElement(activeIndex: number = this.activeIndex): void { this.activeIndex = activeIndex; this.setFocusableElements(); - this.focusableElements[this.activeIndex]?.focus(); + if ( + this.focusableElements[this.activeIndex] && + // Don't focus the toolbar element if some event handlers moved + // the focus on another element in the page. + this.contains(getRootActiveElement(this)) + ) { + this.focusableElements[this.activeIndex].focus(); + } } /**