From 84a9496e22465cbc21a569da07010fbc3af2db2a Mon Sep 17 00:00:00 2001 From: b2 Date: Wed, 8 Jan 2025 17:31:21 -0700 Subject: [PATCH 1/7] fix(#1511): fix click outside popover --- libs/web-components/src/common/utils.ts | 23 +++++ .../src/components/calendar/Calendar.svelte | 1 + .../src/components/dropdown/Dropdown.svelte | 8 +- .../src/components/popover/Popover.svelte | 27 +++--- .../popover/PopoverWrapper.test.svelte | 14 ++- .../src/components/popover/popover.spec.ts | 89 +++++++++++++------ 6 files changed, 112 insertions(+), 50 deletions(-) diff --git a/libs/web-components/src/common/utils.ts b/libs/web-components/src/common/utils.ts index 742b72040..fe7e66ef8 100644 --- a/libs/web-components/src/common/utils.ts +++ b/libs/web-components/src/common/utils.ts @@ -121,6 +121,29 @@ export function getSlottedChildren( } } +export function isElementContainedInSlotsRecursive( + rootEl: Element, + childEl: Element, + depth = 15 +): boolean { + if (rootEl.contains(childEl)) { + return true; + } + if (depth <= 0) { + return false; + } + const slots = rootEl.querySelectorAll("slot"); + for (const slot of Array.from(slots)) { + const assigned = slot.assignedElements(); + for (const el of assigned) { + if (isElementContainedInSlotsRecursive(el, childEl, depth - 1)) { + return true; + } + } + } + return false; +} + export function toBoolean(value: string): boolean { // this is how false will need to be represented if (value === "false") { diff --git a/libs/web-components/src/components/calendar/Calendar.svelte b/libs/web-components/src/components/calendar/Calendar.svelte index 1d57c45c4..efe3f9cb4 100644 --- a/libs/web-components/src/components/calendar/Calendar.svelte +++ b/libs/web-components/src/components/calendar/Calendar.svelte @@ -304,6 +304,7 @@ style={calculateMargin(mt, mr, mb, ml)} class:bordered={bordered === "true"} data-testid={testid} + tabindex="-1" > diff --git a/libs/web-components/src/components/dropdown/Dropdown.svelte b/libs/web-components/src/components/dropdown/Dropdown.svelte index a7d0d8aea..454f91317 100644 --- a/libs/web-components/src/components/dropdown/Dropdown.svelte +++ b/libs/web-components/src/components/dropdown/Dropdown.svelte @@ -311,7 +311,7 @@ setTimeout(async () => { syncFilteredOptions(); _isMenuVisible = true; - // _inputEl?.focus(); + _inputEl?.focus(); }, 0); } @@ -406,9 +406,9 @@ function onClearIconKeyDown(e: KeyboardEvent) { if (e.key === "Enter" || e.key === " ") { - e.stopPropagation(); reset(); showMenu(); + e.stopPropagation(); } } @@ -439,9 +439,7 @@ } async function onChevronClick(e: Event) { - await tick(); showMenu(); - e.stopPropagation(); } function onFocus(e: Event) { @@ -830,7 +828,7 @@ .dropdown-icon--arrow, .dropdown-icon--clear { - margin-right: var(--goa-space-s); + padding-right: var(--goa-space-s); } /* TODO: add indicator to when the reset button has focus state */ diff --git a/libs/web-components/src/components/popover/Popover.svelte b/libs/web-components/src/components/popover/Popover.svelte index 9cf80cbbc..6f567ee5f 100644 --- a/libs/web-components/src/components/popover/Popover.svelte +++ b/libs/web-components/src/components/popover/Popover.svelte @@ -16,6 +16,7 @@ getSlottedChildren, styles, toBoolean, + isElementContainedInSlotsRecursive, } from "../../common/utils"; import type { Spacing } from "../../common/styling"; @@ -104,6 +105,18 @@ ) as HTMLElement) || _targetEl; }); + function handleFocusOut(e: FocusEvent) { + if (!_focusTrapEl) return; + + const activeElement = e.relatedTarget; + const isFocusInPopover = + activeElement instanceof Element && + isElementContainedInSlotsRecursive(_rootEl, activeElement); + if (!isFocusInPopover) { + closePopover(); + } + } + // Functions // Called on window popstate changes. This allows for clicking links within @@ -225,6 +238,7 @@
-
diff --git a/libs/web-components/src/components/popover/PopoverWrapper.test.svelte b/libs/web-components/src/components/popover/PopoverWrapper.test.svelte index 6f840f2df..008576d41 100644 --- a/libs/web-components/src/components/popover/PopoverWrapper.test.svelte +++ b/libs/web-components/src/components/popover/PopoverWrapper.test.svelte @@ -2,16 +2,14 @@ - - {#if content} -
{content}
- {/if} - {#if targetTrigger} -
{targetTrigger}
- {/if} -
+ +
{@html targetTrigger}
+ {@html content} +
diff --git a/libs/web-components/src/components/popover/popover.spec.ts b/libs/web-components/src/components/popover/popover.spec.ts index e0ee09ba9..14bdf53c7 100644 --- a/libs/web-components/src/components/popover/popover.spec.ts +++ b/libs/web-components/src/components/popover/popover.spec.ts @@ -1,35 +1,32 @@ -import { render, fireEvent } from "@testing-library/svelte"; +import { render } from "@testing-library/svelte"; +import userEvent, { UserEvent } from "@testing-library/user-event"; import PopoverWrapper from "./PopoverWrapper.test.svelte"; import Popover from "./Popover.svelte"; import { it } from "vitest"; -it("should render", async () => { - const { container } = render(PopoverWrapper, { - content: "This is content", - targetTrigger: "Clik Action", - }); +let user: UserEvent; - const slotContent = container.querySelector("[slot=content]"); - const slotTarget = container.querySelector("[slot=target]"); +beforeEach(() => { + user = userEvent.setup(); +}); - expect(slotContent).toBeTruthy(); - expect(slotTarget).toBeTruthy(); +it("should render without content by default", async () => { + const result = render(Popover); + + const slotContent = result.queryByTestId("popover-content"); + const slotTarget = result.queryByTestId("popover-target"); - expect(slotContent?.innerHTML).toContain( - "This is content", - ); - expect(slotTarget?.innerHTML).toContain( - "Clik Action", - ); + expect(slotContent).toBeFalsy(); + expect(slotTarget).toBeTruthy(); }); it("should open content when target is clicked", async () => { - const result = render(Popover, {minwidth: "8rem", maxwidth: "16rem"}); + const result = render(Popover, { minwidth: "8rem", maxwidth: "16rem" }); const target = result.queryByTestId("popover-target"); expect(target).toBeTruthy(); expect(result.queryByTestId("popover-content")).toBeNull(); - target && await fireEvent.click(target); + target && (await user.click(target)); const popOverContent = result.queryByTestId("popover-content"); expect(popOverContent).toBeTruthy(); const style = popOverContent?.getAttribute("style"); @@ -40,16 +37,58 @@ it("should open content when target is clicked", async () => { it("should close content when clicked outside the content container", async () => { const result = render(Popover); - const target = result.queryByTestId("popover-target"); + const target = result.queryByTestId("popover-target"); expect(target).toBeTruthy(); - target && await fireEvent.click(target); + expect(result.queryByTestId("popover-content")).toBeFalsy(); - const background = result.queryByTestId("popover-background"); - expect(background).toBeTruthy(); + // click on popover target + target && (await user.click(target)); expect(result.queryByTestId("popover-content")).toBeTruthy(); - expect(result.queryByTestId("popover-background")).toBeTruthy(); - background && await fireEvent.click(background); - expect(result.queryByTestId("popover-content")).toBeNull(); + // click outside of popover + await user.click(document.body); + expect(result.queryByTestId("popover-content")).toBeFalsy(); +}); + +it("should not close content when clicked focusable target then click focusable content inside the content container", async () => { + // arrange to test that focusout event bubbles up to popover component + const result = render(PopoverWrapper, { + content: `
`, + targetTrigger: `
`, + }); + + const target = result.queryByTestId("clickable-target"); + expect(target).toBeTruthy(); + expect(result.queryByTestId("clickable-content")).toBeFalsy(); + + // click on focusable element within popover target + target && (await user.click(target)); + expect(result.queryByTestId("clickable-content")).toBeTruthy(); + + // click on focusable content inside popover content + const clickButton = result.queryByTestId("clickable-content"); + clickButton && (await user.click(clickButton)); + expect(result.queryByTestId("clickable-content")).toBeTruthy(); +}); + +it("should not close content when clicked non-focusable target then click non-focusable content inside the content container", async () => { + // arrange to test that focusout event bubbles up to popover component + const result = render(PopoverWrapper, { + content: `Click me`, + targetTrigger: `Click Action`, + }); + + const target = result.queryByTestId("non-focusable-target"); + expect(target).toBeTruthy(); + expect(result.queryByTestId("popover-content")).toBeFalsy(); + + // click on non-focusable element within popover target + target && (await user.click(target)); + expect(await result.findByTestId("popover-content")).toBeTruthy(); + + // click on non-focusable content within popover content + const clickButton = result.queryByTestId("non-focusable-content"); + clickButton && (await user.click(clickButton)); + expect(result.queryByTestId("popover-content")).toBeTruthy(); }); From dff2253eba852dd93f127c7eb2fe45eb9a504ef5 Mon Sep 17 00:00:00 2001 From: b2 Date: Thu, 9 Jan 2025 08:46:54 -0700 Subject: [PATCH 2/7] cleanup --- libs/web-components/src/common/utils.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/libs/web-components/src/common/utils.ts b/libs/web-components/src/common/utils.ts index fe7e66ef8..99d8ec45f 100644 --- a/libs/web-components/src/common/utils.ts +++ b/libs/web-components/src/common/utils.ts @@ -36,7 +36,11 @@ export const msg = { export function receive( el: HTMLElement | Element | null | undefined, - handler: (action: string, data: Record, event: Event) => void, + handler: ( + action: string, + data: Record, + event: Event, + ) => void, ) { if (!el) { console.warn("receive() el is null | undefined"); @@ -113,8 +117,10 @@ export function getSlottedChildren( } else { // for unit tests only if (parentTestSelector) { - // @ts-expect-error testing - return [...rootEl.querySelector(parentTestSelector).children] as Element[]; + return [ + // @ts-expect-error testing + ...rootEl.querySelector(parentTestSelector).children, + ] as Element[]; } // @ts-expect-error testing return [...rootEl.children] as Element[]; @@ -124,7 +130,7 @@ export function getSlottedChildren( export function isElementContainedInSlotsRecursive( rootEl: Element, childEl: Element, - depth = 15 + depth = 15, ): boolean { if (rootEl.contains(childEl)) { return true; @@ -165,7 +171,10 @@ export function isValidDate(d: Date): boolean { return !isNaN(d.getDate()); } -export function validateRequired(componentName: string, props: Record) { +export function validateRequired( + componentName: string, + props: Record, +) { Object.entries(props).forEach((prop) => { if (!prop[1]) { console.warn(`${componentName}: ${prop[0]} is required`); From 10889b7a86de3f7d011770500776a09d0a10f07e Mon Sep 17 00:00:00 2001 From: Vanessa Tran Date: Wed, 8 Jan 2025 17:30:57 -0700 Subject: [PATCH 3/7] fix: build error roperty 'mt' does not exist on type 'IntrinsicAttributes & component props' on react --- libs/react-components/vite.config.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/react-components/vite.config.ts b/libs/react-components/vite.config.ts index 64cfe09bc..3c3323445 100644 --- a/libs/react-components/vite.config.ts +++ b/libs/react-components/vite.config.ts @@ -15,6 +15,7 @@ export default defineConfig({ dts({ entryRoot: "src", tsconfigPath: path.join(__dirname, "tsconfig.lib.json"), + aliasesExclude: [/^@abgov\/ui-components-common$/], }), ], From 5016438276b92fe9562f7d1e85b34d1144672a05 Mon Sep 17 00:00:00 2001 From: b2 Date: Thu, 9 Jan 2025 13:05:51 -0700 Subject: [PATCH 4/7] fix(#1511): fix click outside popover --- _templates/web/src/pages/PopoverPage.svelte | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/_templates/web/src/pages/PopoverPage.svelte b/_templates/web/src/pages/PopoverPage.svelte index 7fff9cdca..f1d7efcd8 100644 --- a/_templates/web/src/pages/PopoverPage.svelte +++ b/_templates/web/src/pages/PopoverPage.svelte @@ -3,8 +3,10 @@ -

This is a popover

- It can be used for a number of different contexts. +
+

This is a popover

+ It can be used for a number of different contexts. +
Click me
From e891160035a706c548747ee417afea0e7853f8a6 Mon Sep 17 00:00:00 2001 From: b2 Date: Thu, 9 Jan 2025 13:05:51 -0700 Subject: [PATCH 5/7] fix(#1511): fix click outside popover fix(#1511): fix click outside popover --- _templates/web/src/pages/PopoverPage.svelte | 6 +- libs/web-components/src/common/utils.ts | 23 +++++ .../src/components/calendar/Calendar.svelte | 1 + .../src/components/dropdown/Dropdown.svelte | 8 +- .../src/components/popover/Popover.svelte | 27 +++--- .../popover/PopoverWrapper.test.svelte | 14 ++- .../src/components/popover/popover.spec.ts | 89 +++++++++++++------ 7 files changed, 116 insertions(+), 52 deletions(-) diff --git a/_templates/web/src/pages/PopoverPage.svelte b/_templates/web/src/pages/PopoverPage.svelte index 7fff9cdca..f1d7efcd8 100644 --- a/_templates/web/src/pages/PopoverPage.svelte +++ b/_templates/web/src/pages/PopoverPage.svelte @@ -3,8 +3,10 @@ -

This is a popover

- It can be used for a number of different contexts. +
+

This is a popover

+ It can be used for a number of different contexts. +
Click me
diff --git a/libs/web-components/src/common/utils.ts b/libs/web-components/src/common/utils.ts index 742b72040..fe7e66ef8 100644 --- a/libs/web-components/src/common/utils.ts +++ b/libs/web-components/src/common/utils.ts @@ -121,6 +121,29 @@ export function getSlottedChildren( } } +export function isElementContainedInSlotsRecursive( + rootEl: Element, + childEl: Element, + depth = 15 +): boolean { + if (rootEl.contains(childEl)) { + return true; + } + if (depth <= 0) { + return false; + } + const slots = rootEl.querySelectorAll("slot"); + for (const slot of Array.from(slots)) { + const assigned = slot.assignedElements(); + for (const el of assigned) { + if (isElementContainedInSlotsRecursive(el, childEl, depth - 1)) { + return true; + } + } + } + return false; +} + export function toBoolean(value: string): boolean { // this is how false will need to be represented if (value === "false") { diff --git a/libs/web-components/src/components/calendar/Calendar.svelte b/libs/web-components/src/components/calendar/Calendar.svelte index 1d57c45c4..efe3f9cb4 100644 --- a/libs/web-components/src/components/calendar/Calendar.svelte +++ b/libs/web-components/src/components/calendar/Calendar.svelte @@ -304,6 +304,7 @@ style={calculateMargin(mt, mr, mb, ml)} class:bordered={bordered === "true"} data-testid={testid} + tabindex="-1" > diff --git a/libs/web-components/src/components/dropdown/Dropdown.svelte b/libs/web-components/src/components/dropdown/Dropdown.svelte index a7d0d8aea..454f91317 100644 --- a/libs/web-components/src/components/dropdown/Dropdown.svelte +++ b/libs/web-components/src/components/dropdown/Dropdown.svelte @@ -311,7 +311,7 @@ setTimeout(async () => { syncFilteredOptions(); _isMenuVisible = true; - // _inputEl?.focus(); + _inputEl?.focus(); }, 0); } @@ -406,9 +406,9 @@ function onClearIconKeyDown(e: KeyboardEvent) { if (e.key === "Enter" || e.key === " ") { - e.stopPropagation(); reset(); showMenu(); + e.stopPropagation(); } } @@ -439,9 +439,7 @@ } async function onChevronClick(e: Event) { - await tick(); showMenu(); - e.stopPropagation(); } function onFocus(e: Event) { @@ -830,7 +828,7 @@ .dropdown-icon--arrow, .dropdown-icon--clear { - margin-right: var(--goa-space-s); + padding-right: var(--goa-space-s); } /* TODO: add indicator to when the reset button has focus state */ diff --git a/libs/web-components/src/components/popover/Popover.svelte b/libs/web-components/src/components/popover/Popover.svelte index 9cf80cbbc..6f567ee5f 100644 --- a/libs/web-components/src/components/popover/Popover.svelte +++ b/libs/web-components/src/components/popover/Popover.svelte @@ -16,6 +16,7 @@ getSlottedChildren, styles, toBoolean, + isElementContainedInSlotsRecursive, } from "../../common/utils"; import type { Spacing } from "../../common/styling"; @@ -104,6 +105,18 @@ ) as HTMLElement) || _targetEl; }); + function handleFocusOut(e: FocusEvent) { + if (!_focusTrapEl) return; + + const activeElement = e.relatedTarget; + const isFocusInPopover = + activeElement instanceof Element && + isElementContainedInSlotsRecursive(_rootEl, activeElement); + if (!isFocusInPopover) { + closePopover(); + } + } + // Functions // Called on window popstate changes. This allows for clicking links within @@ -225,6 +238,7 @@
-
diff --git a/libs/web-components/src/components/popover/PopoverWrapper.test.svelte b/libs/web-components/src/components/popover/PopoverWrapper.test.svelte index 6f840f2df..008576d41 100644 --- a/libs/web-components/src/components/popover/PopoverWrapper.test.svelte +++ b/libs/web-components/src/components/popover/PopoverWrapper.test.svelte @@ -2,16 +2,14 @@ - - {#if content} -
{content}
- {/if} - {#if targetTrigger} -
{targetTrigger}
- {/if} -
+ +
{@html targetTrigger}
+ {@html content} +
diff --git a/libs/web-components/src/components/popover/popover.spec.ts b/libs/web-components/src/components/popover/popover.spec.ts index e0ee09ba9..14bdf53c7 100644 --- a/libs/web-components/src/components/popover/popover.spec.ts +++ b/libs/web-components/src/components/popover/popover.spec.ts @@ -1,35 +1,32 @@ -import { render, fireEvent } from "@testing-library/svelte"; +import { render } from "@testing-library/svelte"; +import userEvent, { UserEvent } from "@testing-library/user-event"; import PopoverWrapper from "./PopoverWrapper.test.svelte"; import Popover from "./Popover.svelte"; import { it } from "vitest"; -it("should render", async () => { - const { container } = render(PopoverWrapper, { - content: "This is content", - targetTrigger: "Clik Action", - }); +let user: UserEvent; - const slotContent = container.querySelector("[slot=content]"); - const slotTarget = container.querySelector("[slot=target]"); +beforeEach(() => { + user = userEvent.setup(); +}); - expect(slotContent).toBeTruthy(); - expect(slotTarget).toBeTruthy(); +it("should render without content by default", async () => { + const result = render(Popover); + + const slotContent = result.queryByTestId("popover-content"); + const slotTarget = result.queryByTestId("popover-target"); - expect(slotContent?.innerHTML).toContain( - "This is content", - ); - expect(slotTarget?.innerHTML).toContain( - "Clik Action", - ); + expect(slotContent).toBeFalsy(); + expect(slotTarget).toBeTruthy(); }); it("should open content when target is clicked", async () => { - const result = render(Popover, {minwidth: "8rem", maxwidth: "16rem"}); + const result = render(Popover, { minwidth: "8rem", maxwidth: "16rem" }); const target = result.queryByTestId("popover-target"); expect(target).toBeTruthy(); expect(result.queryByTestId("popover-content")).toBeNull(); - target && await fireEvent.click(target); + target && (await user.click(target)); const popOverContent = result.queryByTestId("popover-content"); expect(popOverContent).toBeTruthy(); const style = popOverContent?.getAttribute("style"); @@ -40,16 +37,58 @@ it("should open content when target is clicked", async () => { it("should close content when clicked outside the content container", async () => { const result = render(Popover); - const target = result.queryByTestId("popover-target"); + const target = result.queryByTestId("popover-target"); expect(target).toBeTruthy(); - target && await fireEvent.click(target); + expect(result.queryByTestId("popover-content")).toBeFalsy(); - const background = result.queryByTestId("popover-background"); - expect(background).toBeTruthy(); + // click on popover target + target && (await user.click(target)); expect(result.queryByTestId("popover-content")).toBeTruthy(); - expect(result.queryByTestId("popover-background")).toBeTruthy(); - background && await fireEvent.click(background); - expect(result.queryByTestId("popover-content")).toBeNull(); + // click outside of popover + await user.click(document.body); + expect(result.queryByTestId("popover-content")).toBeFalsy(); +}); + +it("should not close content when clicked focusable target then click focusable content inside the content container", async () => { + // arrange to test that focusout event bubbles up to popover component + const result = render(PopoverWrapper, { + content: `
`, + targetTrigger: `
`, + }); + + const target = result.queryByTestId("clickable-target"); + expect(target).toBeTruthy(); + expect(result.queryByTestId("clickable-content")).toBeFalsy(); + + // click on focusable element within popover target + target && (await user.click(target)); + expect(result.queryByTestId("clickable-content")).toBeTruthy(); + + // click on focusable content inside popover content + const clickButton = result.queryByTestId("clickable-content"); + clickButton && (await user.click(clickButton)); + expect(result.queryByTestId("clickable-content")).toBeTruthy(); +}); + +it("should not close content when clicked non-focusable target then click non-focusable content inside the content container", async () => { + // arrange to test that focusout event bubbles up to popover component + const result = render(PopoverWrapper, { + content: `Click me`, + targetTrigger: `Click Action`, + }); + + const target = result.queryByTestId("non-focusable-target"); + expect(target).toBeTruthy(); + expect(result.queryByTestId("popover-content")).toBeFalsy(); + + // click on non-focusable element within popover target + target && (await user.click(target)); + expect(await result.findByTestId("popover-content")).toBeTruthy(); + + // click on non-focusable content within popover content + const clickButton = result.queryByTestId("non-focusable-content"); + clickButton && (await user.click(clickButton)); + expect(result.queryByTestId("popover-content")).toBeTruthy(); }); From 0a3702129f8883e1bb201967b05f3087ef3c4c74 Mon Sep 17 00:00:00 2001 From: b2 Date: Thu, 9 Jan 2025 08:46:54 -0700 Subject: [PATCH 6/7] cleanup --- libs/web-components/src/common/utils.ts | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/libs/web-components/src/common/utils.ts b/libs/web-components/src/common/utils.ts index fe7e66ef8..99d8ec45f 100644 --- a/libs/web-components/src/common/utils.ts +++ b/libs/web-components/src/common/utils.ts @@ -36,7 +36,11 @@ export const msg = { export function receive( el: HTMLElement | Element | null | undefined, - handler: (action: string, data: Record, event: Event) => void, + handler: ( + action: string, + data: Record, + event: Event, + ) => void, ) { if (!el) { console.warn("receive() el is null | undefined"); @@ -113,8 +117,10 @@ export function getSlottedChildren( } else { // for unit tests only if (parentTestSelector) { - // @ts-expect-error testing - return [...rootEl.querySelector(parentTestSelector).children] as Element[]; + return [ + // @ts-expect-error testing + ...rootEl.querySelector(parentTestSelector).children, + ] as Element[]; } // @ts-expect-error testing return [...rootEl.children] as Element[]; @@ -124,7 +130,7 @@ export function getSlottedChildren( export function isElementContainedInSlotsRecursive( rootEl: Element, childEl: Element, - depth = 15 + depth = 15, ): boolean { if (rootEl.contains(childEl)) { return true; @@ -165,7 +171,10 @@ export function isValidDate(d: Date): boolean { return !isNaN(d.getDate()); } -export function validateRequired(componentName: string, props: Record) { +export function validateRequired( + componentName: string, + props: Record, +) { Object.entries(props).forEach((prop) => { if (!prop[1]) { console.warn(`${componentName}: ${prop[0]} is required`); From 62a6b0c79fe197e96becc58c0518636786bc47d6 Mon Sep 17 00:00:00 2001 From: b2 Date: Fri, 10 Jan 2025 15:45:46 -0700 Subject: [PATCH 7/7] fix(#1511): fix click outside popover --- .../src/components/popover/Popover.svelte | 19 +++++++++++++++++-- .../src/components/popover/popover.spec.ts | 12 +++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/libs/web-components/src/components/popover/Popover.svelte b/libs/web-components/src/components/popover/Popover.svelte index 6f567ee5f..30cdd12e9 100644 --- a/libs/web-components/src/components/popover/Popover.svelte +++ b/libs/web-components/src/components/popover/Popover.svelte @@ -152,11 +152,14 @@ // Opens the popover and adds the required binding to the new slot element function openPopover() { if (_disabled) return; + + _initFocusedEl.focus(); (async () => { _open = true; await tick(); _focusTrapEl.addEventListener("keydown", onFocusTrapEvent, true); _rootEl.dispatchEvent(new CustomEvent("_open", { composed: true })); + makeContentFocusable(); })(); } @@ -170,6 +173,20 @@ _rootEl.dispatchEvent(new CustomEvent("_close", { composed: true })); } + // Ensures that all immediate children of the popover content are included + // in event.relatedTarget that bubbles up to popover 'focusout' event handler + function makeContentFocusable() { + const content = $$slots.default; + if (content && _focusTrapEl) { + const immediateChildren = getSlottedChildren(_focusTrapEl); + immediateChildren.forEach((child) => { + if ((child as HTMLElement).tabIndex < 0) { + (child as HTMLElement).tabIndex = -1; + } + }); + } + } + function getBoundingClientRectWithMargins( el: Element, ): Omit { @@ -266,8 +283,6 @@
{#if _open} - -
{ // arrange to test that focusout event bubbles up to popover component const result = render(PopoverWrapper, { - content: `
`, + content: ``, targetTrigger: `
`, }); @@ -67,8 +67,8 @@ it("should not close content when clicked focusable target then click focusable expect(result.queryByTestId("clickable-content")).toBeTruthy(); // click on focusable content inside popover content - const clickButton = result.queryByTestId("clickable-content"); - clickButton && (await user.click(clickButton)); + const button = result.queryByTestId("clickable-content"); + button && (await user.click(button)); expect(result.queryByTestId("clickable-content")).toBeTruthy(); }); @@ -88,7 +88,9 @@ it("should not close content when clicked non-focusable target then click non-fo expect(await result.findByTestId("popover-content")).toBeTruthy(); // click on non-focusable content within popover content - const clickButton = result.queryByTestId("non-focusable-content"); - clickButton && (await user.click(clickButton)); + const contentNonfocusableEl = result.queryByTestId("non-focusable-content"); + expect(contentNonfocusableEl?.tabIndex).toBe(-1); + + contentNonfocusableEl && (await user.click(contentNonfocusableEl)); expect(result.queryByTestId("popover-content")).toBeTruthy(); });