Skip to content

Commit

Permalink
Fix bug in toolbar click handler when a slotted element has child ele…
Browse files Browse the repository at this point in the history
…ments (#6830)

* Fix bug with nested elements in toolbar click handler

* Change files

* lint

* update change description

* slight test improvement

* Update change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json

---------

Co-authored-by: Chris Holt <[email protected]>
  • Loading branch information
2 people authored and janechu committed Jun 10, 2024
1 parent e8b2593 commit 02c862f
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Fix bug in toolbar click handler when a slotted element has child elements",
"packageName": "@microsoft/fast-foundation",
"email": "[email protected]",
"dependentChangeType": "prerelease"
}
Original file line number Diff line number Diff line change
Expand Up @@ -349,4 +349,73 @@ test.describe("Toolbar", () => {

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

test("should update activeIndex when an element within the toolbar is clicked", 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 slot="end">End Slot Button</button>
</fast-toolbar>
`;
});

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

const startSlotButton = element.locator("button", {
hasText: "Start Slot Button",
});

await element.focus();

await expect(startSlotButton).toBeFocused();

await expect(element).toHaveJSProperty("activeIndex", 0);

await button2.click();

await expect(button2).toBeFocused();

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

test("should update activeIndex when a nested element within the toolbar is clicked", async () => {
await root.evaluate(node => {
node.innerHTML = /* html */ `
<fast-toolbar>
<button slot="start">Start Slot Button</button>
<button>Button 1</button>
<button>
Button 2
<div>more button content</div>
</button>
<button>Button 3</button>
<button slot="end">End Slot Button</button>
</fast-toolbar>
`;
});

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

const button2InnerContent = button2.locator("div");

const startSlotButton = element.locator("button", {
hasText: "Start Slot Button",
});

await element.focus();

await expect(startSlotButton).toBeFocused();

await expect(element).toHaveJSProperty("activeIndex", 0);

await button2InnerContent.click();

await expect(button2).toBeFocused();

await expect(element).toHaveJSProperty("activeIndex", 2);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ export class FASTToolbar extends FASTElement {
* @internal
*/
public clickHandler(e: MouseEvent): boolean | void {
const activeIndex = this.focusableElements?.indexOf(e.target as HTMLElement);
const activeIndex = this.focusableElements?.findIndex(x =>
x.contains(e.target as HTMLElement)
);
if (activeIndex > -1 && this.activeIndex !== activeIndex) {
this.setFocusedElement(activeIndex);
}
Expand Down

0 comments on commit 02c862f

Please sign in to comment.