From 89864e75a5d5d68742e262b5de1e73889136379e Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 5 Sep 2023 11:37:06 -0500 Subject: [PATCH 1/6] Fix bug with nested elements in toolbar click handler --- .../src/toolbar/toolbar.pw.spec.ts | 66 +++++++++++++++++++ .../fast-foundation/src/toolbar/toolbar.ts | 2 +- 2 files changed, 67 insertions(+), 1 deletion(-) 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 2090ec378ae..c097513f22d 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 @@ -349,4 +349,70 @@ 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", { hasText: "more button content" }); + + 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); + }); }); diff --git a/packages/web-components/fast-foundation/src/toolbar/toolbar.ts b/packages/web-components/fast-foundation/src/toolbar/toolbar.ts index 6701e26cd89..2f17e04876f 100644 --- a/packages/web-components/fast-foundation/src/toolbar/toolbar.ts +++ b/packages/web-components/fast-foundation/src/toolbar/toolbar.ts @@ -122,7 +122,7 @@ 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); } From 43e73935d008d696342abdfca99742dd78b8dc38 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 5 Sep 2023 11:54:52 -0500 Subject: [PATCH 2/6] Change files --- ...st-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json diff --git a/change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json b/change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json new file mode 100644 index 00000000000..ac640e4e33a --- /dev/null +++ b/change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json @@ -0,0 +1,7 @@ +{ + "type": "prerelease", + "comment": "Fix bug with nested elements in toolbar click handler", + "packageName": "@microsoft/fast-foundation", + "email": "20542556+mollykreis@users.noreply.github.com", + "dependentChangeType": "patch" +} From 23236647a5abbb065f62a80c7492d63dc1e1ed34 Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 5 Sep 2023 11:58:38 -0500 Subject: [PATCH 3/6] lint --- .../web-components/fast-foundation/src/toolbar/toolbar.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/web-components/fast-foundation/src/toolbar/toolbar.ts b/packages/web-components/fast-foundation/src/toolbar/toolbar.ts index 2f17e04876f..98fcf9fcff5 100644 --- a/packages/web-components/fast-foundation/src/toolbar/toolbar.ts +++ b/packages/web-components/fast-foundation/src/toolbar/toolbar.ts @@ -122,7 +122,9 @@ export class FASTToolbar extends FASTElement { * @internal */ public clickHandler(e: MouseEvent): boolean | void { - const activeIndex = this.focusableElements?.findIndex(x => x.contains(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); } From 41fa53528e8af57f4cb3b9d6eb6119103c97c11b Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 5 Sep 2023 12:02:29 -0500 Subject: [PATCH 4/6] update change description --- ...ft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json b/change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json index ac640e4e33a..1da3d92439d 100644 --- a/change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json +++ b/change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json @@ -1,6 +1,6 @@ { "type": "prerelease", - "comment": "Fix bug with nested elements in toolbar click handler", + "comment": "Fix bug in toolbar click handler when a slotted element has child elements", "packageName": "@microsoft/fast-foundation", "email": "20542556+mollykreis@users.noreply.github.com", "dependentChangeType": "patch" From 044c6d999ed2b411c885578a4f28e60c800aee1b Mon Sep 17 00:00:00 2001 From: mollykreis <20542556+mollykreis@users.noreply.github.com> Date: Tue, 5 Sep 2023 12:49:21 -0500 Subject: [PATCH 5/6] slight test improvement --- .../fast-foundation/src/toolbar/toolbar.pw.spec.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) 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 c097513f22d..9416924b333 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 @@ -388,7 +388,10 @@ test.describe("Toolbar", () => { <fast-toolbar> <button slot="start">Start Slot Button</button> <button>Button 1</button> - <button>Button 2 <div>more button content</div></button> + <button> + Button 2 + <div>more button content</div> + </button> <button>Button 3</button> <button slot="end">End Slot Button</button> </fast-toolbar> @@ -397,7 +400,7 @@ test.describe("Toolbar", () => { const button2 = element.locator("button", { hasText: "Button 2" }); - const button2InnerContent = button2.locator("div", { hasText: "more button content" }); + const button2InnerContent = button2.locator("div"); const startSlotButton = element.locator("button", { hasText: "Start Slot Button", From c4cd46f3d0183a7b8e5557ce3c9ff9024cba6e03 Mon Sep 17 00:00:00 2001 From: Chris Holt <chhol@microsoft.com> Date: Mon, 25 Sep 2023 13:32:11 -0700 Subject: [PATCH 6/6] Update change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json --- ...ft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json b/change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json index 1da3d92439d..b88d36dc993 100644 --- a/change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json +++ b/change/@microsoft-fast-foundation-2ada25c3-3fb4-48ba-893d-e64d11f7f095.json @@ -3,5 +3,5 @@ "comment": "Fix bug in toolbar click handler when a slotted element has child elements", "packageName": "@microsoft/fast-foundation", "email": "20542556+mollykreis@users.noreply.github.com", - "dependentChangeType": "patch" + "dependentChangeType": "prerelease" }