Skip to content

Commit bdb5f4e

Browse files
fcollonvalscomea
authored andcommitted
Fix toolbar stealing focus (#6888)
* Fix toolbar stealing focus * Implement suggestion by @scomea * Add test * Use 'getRootActiveElement' * Change files * Apply review comment --------- Co-authored-by: Frédéric Collonval <[email protected]> Co-authored-by: Stephane Comeau <[email protected]>
1 parent b43ca3d commit bdb5f4e

File tree

3 files changed

+78
-1
lines changed

3 files changed

+78
-1
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "prerelease",
3+
"comment": "Fix toolbar stealing focus",
4+
"packageName": "@microsoft/fast-foundation",
5+
"email": "[email protected]",
6+
"dependentChangeType": "prerelease"
7+
}

packages/web-components/fast-foundation/src/toolbar/toolbar.pw.spec.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,4 +479,66 @@ test.describe("Toolbar", () => {
479479
await button3.click();
480480
await expect(button3).toBeFocused();
481481
});
482+
483+
test("should not focus clicked item when focus is moved outside of the toolbar by an event handler", async () => {
484+
await root.evaluate(node => {
485+
node.innerHTML = /* html */ `
486+
<fast-toolbar>
487+
<button slot="start">Start Slot Button</button>
488+
<button id="toolbar-button-1">Button 1</button>
489+
<button id="toolbar-button-1">Button 2</button>
490+
<button>Button 3</button>
491+
<button>Button 4</button>
492+
<button>Button 5</button>
493+
<button slot="end">End Slot Button</button>
494+
</fast-toolbar>
495+
<button id="outside-button">Button Outside Toolbar</button>
496+
`;
497+
});
498+
499+
const buttonOutsideToolbar = page.locator("button", { hasText: "Button Outside Toolbar"});
500+
const button1 = element.locator("button", { hasText: "Button 1" });
501+
const button2 = element.locator("button", { hasText: "Button 2" });
502+
const button3 = element.locator("button", { hasText: "Button 3" });
503+
504+
const wasButton1Clicked = await button1.evaluate(node => new Promise(resolve => {
505+
node.addEventListener("mousedown", (e: MouseEvent) => {
506+
document.querySelector<HTMLButtonElement>('#outside-button')?.focus();
507+
resolve(true)
508+
})
509+
510+
node.dispatchEvent(new MouseEvent("mousedown", {bubbles: true, cancelable: true}))
511+
}))
512+
513+
expect.soft(wasButton1Clicked).toEqual(true)
514+
await expect.soft(buttonOutsideToolbar).toBeFocused();
515+
buttonOutsideToolbar.evaluate(node => {node.blur()})
516+
517+
const [wasButton2Clicked] = await Promise.all([
518+
button2.evaluate(node => new Promise(resolve => {
519+
node.addEventListener("click", () => {
520+
document.querySelector<HTMLButtonElement>('#outside-button')?.focus();
521+
resolve(true)
522+
})
523+
})),
524+
button2.click()
525+
])
526+
527+
expect.soft(wasButton2Clicked).toEqual(true)
528+
await expect.soft(buttonOutsideToolbar).toBeFocused();
529+
buttonOutsideToolbar.evaluate(node => {node.blur()})
530+
531+
const [wasButton3Clicked] = await Promise.all([
532+
button3.evaluate(node => new Promise(resolve => {
533+
node.addEventListener("click", () => {
534+
resolve(true)
535+
})
536+
})),
537+
button3.click()
538+
])
539+
540+
expect.soft(wasButton3Clicked).toEqual(true)
541+
await expect.soft(buttonOutsideToolbar).not.toBeFocused();
542+
await expect(button3).toBeFocused();
543+
});
482544
});

packages/web-components/fast-foundation/src/toolbar/toolbar.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { isFocusable } from "tabbable";
44
import { ARIAGlobalStatesAndProperties, StartEnd } from "../patterns/index.js";
55
import { applyMixins } from "../utilities/apply-mixins.js";
66
import { getDirection } from "../utilities/direction.js";
7+
import { getRootActiveElement } from "../utilities/root-active-element.js";
78
import { ToolbarOrientation } from "./toolbar.options.js";
89

910
/**
@@ -246,7 +247,14 @@ export class FASTToolbar extends FASTElement {
246247
private setFocusedElement(activeIndex: number = this.activeIndex): void {
247248
this.activeIndex = activeIndex;
248249
this.setFocusableElements();
249-
this.focusableElements[this.activeIndex]?.focus();
250+
if (
251+
this.focusableElements[this.activeIndex] &&
252+
// Don't focus the toolbar element if some event handlers moved
253+
// the focus on another element in the page.
254+
this.contains(getRootActiveElement(this))
255+
) {
256+
this.focusableElements[this.activeIndex].focus();
257+
}
250258
}
251259

252260
/**

0 commit comments

Comments
 (0)