Skip to content

Commit

Permalink
Fix toolbar focus bug (#6836)
Browse files Browse the repository at this point in the history
* Fix focus issue when clicking on an element in the toolbar

* remove describe.only

* Change files

* Update api-report.md

* Update change/@microsoft-fast-foundation-903a969b-3de3-4090-974a-3423344c571c.json

---------

Co-authored-by: Chris Holt <[email protected]>
  • Loading branch information
mollykreis and chrisdholt authored Oct 11, 2023
1 parent b78c921 commit a7144c6
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Fix focus issue when clicking on an element in the toolbar",
"packageName": "@microsoft/fast-foundation",
"email": "[email protected]",
"dependentChangeType": "prerelease"
}
4 changes: 2 additions & 2 deletions packages/web-components/fast-foundation/docs/api-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -2093,8 +2093,6 @@ export class FASTToolbar extends FASTElement {
childItems: Element[];
// (undocumented)
protected childItemsChanged(prev: undefined | Element[], next: Element[]): void;
// @internal
clickHandler(e: MouseEvent): boolean | void;
// @internal (undocumented)
connectedCallback(): void;
// @internal
Expand All @@ -2103,6 +2101,8 @@ export class FASTToolbar extends FASTElement {
focusinHandler(e: FocusEvent): boolean | void;
// @internal
keydownHandler(e: KeyboardEvent): boolean | void;
// @internal
mouseDownHandler(e: MouseEvent): boolean | void;
orientation: ToolbarOrientation;
// @internal
protected reduceFocusableElements(): void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,4 +418,65 @@ test.describe("Toolbar", () => {

await expect(element).toHaveJSProperty("activeIndex", 2);
});

test("should focus most recently focused item when toolbar receives focus", async () => {
await root.evaluate(node => {
node.innerHTML = /* html */ `
<fast-toolbar>
<button slot="start">Start Slot Button</button>
<button>Button 1</button>
<button>Button 2</button>
<button>Button 3</button>
<button>Button 4</button>
<button>Button 5</button>
<button slot="end">End Slot Button</button>
</fast-toolbar>
<button>Button Outside Toolbar</button>
`;
});
const button2 = element.locator("button", { hasText: "Button 2" });

const buttonOutsideToolbar = page.locator("button", { hasText: "Button Outside Toolbar"});

await button2.click();
await expect(button2).toBeFocused();

await buttonOutsideToolbar.click();
await expect(buttonOutsideToolbar).toBeFocused();

await element.focus();

await expect(button2).toBeFocused();
});

test("should focus clicked item when focus enters toolbar by clicking an item that is not the most recently focused item", async () => {
await root.evaluate(node => {
node.innerHTML = /* html */ `
<fast-toolbar>
<button slot="start">Start Slot Button</button>
<button>Button 1</button>
<button>Button 2</button>
<button>Button 3</button>
<button>Button 4</button>
<button>Button 5</button>
<button slot="end">End Slot Button</button>
</fast-toolbar>
<button>Button Outside Toolbar</button>
`;
});
const button2 = element.locator("button", { hasText: "Button 2" });

const button3 = element.locator("button", { hasText: "Button 3" });

const buttonOutsideToolbar = page.locator("button", { hasText: "Button Outside Toolbar"});

await button2.click();
await expect(button2).toBeFocused();

await buttonOutsideToolbar.click();
await expect(buttonOutsideToolbar).toBeFocused();

await button3.click();
await expect(button3).toBeFocused();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export function toolbarTemplate<T extends FASTToolbar>(
aria-orientation="${x => x.orientation}"
orientation="${x => x.orientation}"
role="toolbar"
@click="${(x, c) => x.clickHandler(c.event as MouseEvent)}"
@mousedown="${(x, c) => x.mouseDownHandler(c.event as MouseEvent)}"
@focusin="${(x, c) => x.focusinHandler(c.event as FocusEvent)}"
@keydown="${(x, c) => x.keydownHandler(c.event as KeyboardEvent)}"
${children({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ export class FASTToolbar extends FASTElement {
*
* @internal
*/
public clickHandler(e: MouseEvent): boolean | void {
public mouseDownHandler(e: MouseEvent): boolean | void {
const activeIndex = this.focusableElements?.findIndex(x =>
x.contains(e.target as HTMLElement)
);
Expand Down

0 comments on commit a7144c6

Please sign in to comment.