From 80c4c95dfb6b58734db10b4490fc845bd0848ac0 Mon Sep 17 00:00:00 2001 From: Benji Franck Date: Tue, 5 May 2026 15:29:46 -0600 Subject: [PATCH 1/2] fix(#3892): stop popover from closing parent popover, drawer, or modal Co-Authored-By: Copilot --- .../routes/bugs/3982/bug3982.component.html | 187 ++++++++++++ .../src/routes/bugs/3982/bug3982.component.ts | 111 +++++++ .../src/routes/bugs/3982/bug3982.route.json | 6 + .../src/app/routes/bugs/bug3982.route.ts | 10 + apps/prs/react/src/routes/bugs/bug3982.tsx | 280 ++++++++++++++++++ .../src/lib/components/drawer/drawer.ts | 5 +- .../src/lib/components/modal/modal.ts | 5 +- .../lib/components/push-drawer/push-drawer.ts | 5 +- .../specs/popover.browser.spec.tsx | 63 ++++ .../src/lib/drawer/drawer.tsx | 11 +- libs/react-components/src/lib/modal/modal.tsx | 5 +- .../src/lib/push-drawer/push-drawer.tsx | 11 +- .../src/components/dropdown/Dropdown.svelte | 1 + .../src/components/popover/Popover.svelte | 43 ++- 14 files changed, 731 insertions(+), 12 deletions(-) create mode 100644 apps/prs/angular/src/routes/bugs/3982/bug3982.component.html create mode 100644 apps/prs/angular/src/routes/bugs/3982/bug3982.component.ts create mode 100644 apps/prs/angular/src/routes/bugs/3982/bug3982.route.json create mode 100644 apps/prs/react/src/app/routes/bugs/bug3982.route.ts create mode 100644 apps/prs/react/src/routes/bugs/bug3982.tsx diff --git a/apps/prs/angular/src/routes/bugs/3982/bug3982.component.html b/apps/prs/angular/src/routes/bugs/3982/bug3982.component.html new file mode 100644 index 0000000000..d4d76cd60e --- /dev/null +++ b/apps/prs/angular/src/routes/bugs/3982/bug3982.component.html @@ -0,0 +1,187 @@ +
+
+

3982 - Nested close propagation

+

+ This page tests the issue where popover close events are propegating to parent + drawers and modals. +

+
+ + +
+

Popover

+

Open the popover and interact with all controls.

+
+ + + Open popover + + + +
+

+ Expected: using the date picker, dropdown, and menu button updates values + and leaves the popover open. +

+ + + + + + + + + + + + + +
+ + + + +
+

Drawer

+

Open the drawer and interact with all controls.

+
+ + Open drawer + + +
+

+ Expected: using the date picker, dropdown, and menu button updates values + and leaves the drawer open. +

+ + + + + + + + + + + + + +
+ + + + +

Push drawer

+

Open the push drawer and interact with all controls.

+ +
+ Open push drawer + + +
+

+ Expected: using the date picker, dropdown, and menu button updates values + and leaves the push drawer open. +

+ + + + + + + + + + + + + +
+ +
+ + + +
+

Modal

+

Open the modal and interact with all controls.

+
+ + Open modal + + +
+

+ Expected: using the date picker, dropdown, and menu button updates values + and leaves the modal open. +

+ + + + + + + + + + + + + +
+ + +
\ No newline at end of file diff --git a/apps/prs/angular/src/routes/bugs/3982/bug3982.component.ts b/apps/prs/angular/src/routes/bugs/3982/bug3982.component.ts new file mode 100644 index 0000000000..34672e37f7 --- /dev/null +++ b/apps/prs/angular/src/routes/bugs/3982/bug3982.component.ts @@ -0,0 +1,111 @@ +import { CommonModule } from "@angular/common"; +import { Component } from "@angular/core"; +import { + GoabButton, + GoabContainer, + GoabDatePicker, + GoabDrawer, + GoabDropdown, + GoabDropdownItem, + GoabMenuAction, + GoabMenuButton, + GoabModal, + GoabPopover, + GoabPushDrawer, +} from "@abgov/angular-components"; +import type { + GoabDatePickerOnChangeDetail, + GoabDropdownOnChangeDetail, +} from "@abgov/ui-components-common"; + +@Component({ + standalone: true, + selector: "abgov-bug3982", + templateUrl: "./bug3982.component.html", + imports: [ + CommonModule, + GoabButton, + GoabContainer, + GoabDatePicker, + GoabDrawer, + GoabDropdown, + GoabDropdownItem, + GoabMenuAction, + GoabMenuButton, + GoabModal, + GoabPopover, + GoabPushDrawer, + ], +}) +export class Bug3982Component { + popoverDate = ""; + popoverProvince = ""; + + drawerOpen = false; + drawerDate = ""; + drawerProvince = ""; + + pushDrawerOpen = false; + pushDrawerDate = ""; + pushDrawerProvince = ""; + + modalOpen = false; + modalDate = ""; + modalProvince = ""; + + handlePopoverDateChange(detail: GoabDatePickerOnChangeDetail): void { + this.popoverDate = detail.valueStr || ""; + } + + handlePopoverProvinceChange(detail: GoabDropdownOnChangeDetail): void { + this.popoverProvince = detail.value || ""; + } + + openDrawer(): void { + this.drawerOpen = true; + } + + handleDrawerClose(): void { + this.drawerOpen = false; + } + + handleDrawerDateChange(detail: GoabDatePickerOnChangeDetail): void { + this.drawerDate = detail.valueStr || ""; + } + + handleDrawerProvinceChange(detail: GoabDropdownOnChangeDetail): void { + this.drawerProvince = detail.value || ""; + } + + openPushDrawer(): void { + this.pushDrawerOpen = true; + } + + handlePushDrawerClose(): void { + this.pushDrawerOpen = false; + } + + handlePushDrawerDateChange(detail: GoabDatePickerOnChangeDetail): void { + this.pushDrawerDate = detail.valueStr || ""; + } + + handlePushDrawerProvinceChange(detail: GoabDropdownOnChangeDetail): void { + this.pushDrawerProvince = detail.value || ""; + } + + openModal(): void { + this.modalOpen = true; + } + + handleModalClose(): void { + this.modalOpen = false; + } + + handleModalDateChange(detail: GoabDatePickerOnChangeDetail): void { + this.modalDate = detail.valueStr || ""; + } + + handleModalProvinceChange(detail: GoabDropdownOnChangeDetail): void { + this.modalProvince = detail.value || ""; + } +} \ No newline at end of file diff --git a/apps/prs/angular/src/routes/bugs/3982/bug3982.route.json b/apps/prs/angular/src/routes/bugs/3982/bug3982.route.json new file mode 100644 index 0000000000..54d7261768 --- /dev/null +++ b/apps/prs/angular/src/routes/bugs/3982/bug3982.route.json @@ -0,0 +1,6 @@ +{ + "type": "bug", + "id": "3982", + "path": "bugs/3982", + "title": "Nested close propagation" +} \ No newline at end of file diff --git a/apps/prs/react/src/app/routes/bugs/bug3982.route.ts b/apps/prs/react/src/app/routes/bugs/bug3982.route.ts new file mode 100644 index 0000000000..1f2a6148e1 --- /dev/null +++ b/apps/prs/react/src/app/routes/bugs/bug3982.route.ts @@ -0,0 +1,10 @@ +import { Bug3982Route } from "../../../routes/bugs/bug3982"; +import type { PrRouteDefinition } from "../../route-manifest"; + +export default { + type: "bug", + id: "3982", + path: "bugs/3982", + title: "Nested close propagation", + component: Bug3982Route, +} satisfies PrRouteDefinition; \ No newline at end of file diff --git a/apps/prs/react/src/routes/bugs/bug3982.tsx b/apps/prs/react/src/routes/bugs/bug3982.tsx new file mode 100644 index 0000000000..bb59f82d5d --- /dev/null +++ b/apps/prs/react/src/routes/bugs/bug3982.tsx @@ -0,0 +1,280 @@ +import { useState } from "react"; +import { + GoabButton, + GoabDatePicker, + GoabDrawer, + GoabDropdown, + GoabDropdownItem, + GoabMenuAction, + GoabMenuButton, + GoabModal, + GoabPopover, + GoabPushDrawer, + GoabText, + GoabContainer, +} from "@abgov/react-components"; +import type { + GoabDatePickerOnChangeDetail, + GoabDropdownOnChangeDetail, +} from "@abgov/ui-components-common"; + +const stackStyle = { + display: "grid", + gap: "1.5rem", +}; + +export function Bug3982Route() { + const [popoverDate, setPopoverDate] = useState(""); + const [popoverProvince, setPopoverProvince] = useState(""); + + const [drawerOpen, setDrawerOpen] = useState(false); + const [drawerDate, setDrawerDate] = useState(""); + const [drawerProvince, setDrawerProvince] = useState(""); + + const [pushDrawerOpen, setPushDrawerOpen] = useState(false); + const [pushDrawerDate, setPushDrawerDate] = useState(""); + const [pushDrawerProvince, setPushDrawerProvince] = useState(""); + + const [modalOpen, setModalOpen] = useState(false); + const [modalDate, setModalDate] = useState(""); + const [modalProvince, setModalProvince] = useState(""); + + const handlePopoverDateChange = (detail: GoabDatePickerOnChangeDetail) => { + setPopoverDate(detail.valueStr || ""); + }; + + const handlePopoverProvinceChange = (detail: GoabDropdownOnChangeDetail) => { + setPopoverProvince(detail.value || ""); + }; + + const handleDrawerDateChange = (detail: GoabDatePickerOnChangeDetail) => { + setDrawerDate(detail.valueStr || ""); + }; + + const handleDrawerClose = () => { + setDrawerOpen(false); + }; + + const handleDrawerProvinceChange = (detail: GoabDropdownOnChangeDetail) => { + setDrawerProvince(detail.value || ""); + }; + + const handlePushDrawerDateChange = (detail: GoabDatePickerOnChangeDetail) => { + setPushDrawerDate(detail.valueStr || ""); + }; + + const handlePushDrawerProvinceChange = (detail: GoabDropdownOnChangeDetail) => { + setPushDrawerProvince(detail.value || ""); + }; + + const handlePushDrawerClose = () => { + setPushDrawerOpen(false); + }; + + const handleModalDateChange = (detail: GoabDatePickerOnChangeDetail) => { + setModalDate(detail.valueStr || ""); + }; + + const handleModalProvinceChange = (detail: GoabDropdownOnChangeDetail) => { + setModalProvince(detail.value || ""); + }; + + const handleModalClose = () => { + setModalOpen(false); + }; + + return ( +
+
+ 3982 - Nested close propagation + + This page tests the issue where popover close events are propegating to parent + drawers and modals. + +
+ + +
+ + Popover + + Open the popover and interact with all controls. +
+ + Open popover}> +
+ + Expected: using the date picker, dropdown, and menu button updates values + and leaves the popover open. + + + + + + + + + + + + + + +
+ + + + +
+ + Drawer + + Open the drawer and interact with all controls. +
+ + setDrawerOpen(true)}>Open drawer + + +
+ + Expected: using the date picker, dropdown, and menu button updates values + and leaves the drawer open. + + + + + + + + + + + + + + +
+ + + + + + Push drawer + + Open the push drawer and interact with all controls. + +
+ setPushDrawerOpen(true)}> + Open push drawer + + + +
+ + Expected: using the date picker, dropdown, and menu button updates values + and leaves the push drawer open. + + + + + + + + + + + + + + +
+ +
+ + + +
+ + Modal + + Open the modal and interact with all controls. +
+ + setModalOpen(true)}>Open modal + + +
+ + Expected: using the date picker, dropdown, and menu button updates values + and leaves the modal open. + + + + + + + + + + + + + + +
+ + +
+ ); +} \ No newline at end of file diff --git a/libs/angular-components/src/lib/components/drawer/drawer.ts b/libs/angular-components/src/lib/components/drawer/drawer.ts index 29ff92aff0..c6f5ae6541 100644 --- a/libs/angular-components/src/lib/components/drawer/drawer.ts +++ b/libs/angular-components/src/lib/components/drawer/drawer.ts @@ -25,7 +25,7 @@ import { GoabDrawerPosition, GoabDrawerSize } from "@abgov/ui-components-common" [attr.maxsize]="maxSize" [attr.testid]="testId" [attr.version]="version" - (_close)="_onClose()" + (_close)="_onClose($event)" >
@@ -70,7 +70,8 @@ export class GoabDrawer implements OnInit { }, 0); } - _onClose() { + _onClose(event: Event) { + if (event.target !== event.currentTarget) return; this.onClose.emit(); } diff --git a/libs/angular-components/src/lib/components/modal/modal.ts b/libs/angular-components/src/lib/components/modal/modal.ts index 6b5906f782..50e3b8175b 100644 --- a/libs/angular-components/src/lib/components/modal/modal.ts +++ b/libs/angular-components/src/lib/components/modal/modal.ts @@ -32,7 +32,7 @@ import { NgTemplateOutlet } from "@angular/common"; [attr.closable]="closable" [attr.transition]="transition" [attr.heading]="getHeadingAsString()" - (_close)="_onClose()" + (_close)="_onClose($event)" >
@if (this.heading !== "" && getHeadingAsTemplate() !== null) { @@ -102,7 +102,8 @@ export class GoabModal implements OnInit { return this.heading instanceof TemplateRef ? this.heading : null; } - _onClose() { + _onClose(event: Event) { + if (event.target !== event.currentTarget) return; this.onClose.emit(); } } diff --git a/libs/angular-components/src/lib/components/push-drawer/push-drawer.ts b/libs/angular-components/src/lib/components/push-drawer/push-drawer.ts index 1adaeb09cf..d869859df3 100644 --- a/libs/angular-components/src/lib/components/push-drawer/push-drawer.ts +++ b/libs/angular-components/src/lib/components/push-drawer/push-drawer.ts @@ -23,7 +23,7 @@ import { [attr.testid]="testId" [attr.width]="width" [attr.version]="version" - (_close)="_onClose()" + (_close)="_onClose($event)" >
@@ -66,7 +66,8 @@ export class GoabPushDrawer implements OnInit { }, 0); } - _onClose() { + _onClose(event: Event) { + if (event.target !== event.currentTarget) return; this.onClose.emit(); } diff --git a/libs/react-components/specs/popover.browser.spec.tsx b/libs/react-components/specs/popover.browser.spec.tsx index 7db4ad54f2..83941ab205 100644 --- a/libs/react-components/specs/popover.browser.spec.tsx +++ b/libs/react-components/specs/popover.browser.spec.tsx @@ -202,6 +202,69 @@ describe("Popover", () => { expect(aboveWidth!).toBe(belowWidth!); }); + it("should not close parent popover when child popover closes", async () => { + const Component = () => { + return ( + + Open parent + + } + > +

Parent content

+ + Open child + + } + > +

Child content

+ + Close child + + + + ); + }; + + const result = render(); + const parentTarget = result.getByTestId("parent-target"); + const childTarget = result.getByTestId("child-target"); + const parentContent = result.getByTestId("parent-content"); + const childContent = result.getByTestId("child-content"); + const childClose = result.getByTestId("child-close"); + + // Open parent popover + await parentTarget.click(); + await vi.waitFor(() => { + expect(parentContent).toBeVisible(); + }); + + // Open child popover + await childTarget.click(); + await vi.waitFor(() => { + expect(childContent).toBeVisible(); + }); + + // Close child popover using close button + await childClose.click(); + + // Parent popover should still be open + await vi.waitFor(() => { + expect(childContent).not.toBeVisible(); + expect(parentContent).toBeVisible(); + }); + }); + it("should align popover to the right edge of the target when there is not enough space on the right", async () => { const Component = () => { return ( diff --git a/libs/react-components/src/lib/drawer/drawer.tsx b/libs/react-components/src/lib/drawer/drawer.tsx index c1eda6568d..9c0fca7e34 100644 --- a/libs/react-components/src/lib/drawer/drawer.tsx +++ b/libs/react-components/src/lib/drawer/drawer.tsx @@ -56,9 +56,16 @@ export function GoabDrawer({ if (!el?.current || !onClose) { return; } - el.current?.addEventListener("_close", onClose); + const current = el.current; + const listener = (e: Event) => { + if (e.target !== current) { + return; + } + onClose(); + }; + current.addEventListener("_close", listener); return () => { - el.current?.removeEventListener("_close", onClose); + current.removeEventListener("_close", listener); }; }, [el, onClose]); diff --git a/libs/react-components/src/lib/modal/modal.tsx b/libs/react-components/src/lib/modal/modal.tsx index 38604c5d83..4b97f47b13 100644 --- a/libs/react-components/src/lib/modal/modal.tsx +++ b/libs/react-components/src/lib/modal/modal.tsx @@ -76,7 +76,10 @@ export function GoabModal({ return; } const current = el.current; - const listener = () => { + const listener = (e: Event) => { + if (e.target !== current) { + return; + } onClose?.(); }; diff --git a/libs/react-components/src/lib/push-drawer/push-drawer.tsx b/libs/react-components/src/lib/push-drawer/push-drawer.tsx index 681add6e08..ec3d4e7b96 100644 --- a/libs/react-components/src/lib/push-drawer/push-drawer.tsx +++ b/libs/react-components/src/lib/push-drawer/push-drawer.tsx @@ -51,9 +51,16 @@ export function GoabPushDrawer({ if (!el?.current || !onClose) { return; } - el.current?.addEventListener("_close", onClose); + const current = el.current; + const listener = (e: Event) => { + if (e.target !== current) { + return; + } + onClose(); + }; + current.addEventListener("_close", listener); return () => { - el.current?.removeEventListener("_close", onClose); + current.removeEventListener("_close", listener); }; }, [el, onClose]); diff --git a/libs/web-components/src/components/dropdown/Dropdown.svelte b/libs/web-components/src/components/dropdown/Dropdown.svelte index e92422814c..5bcffc0813 100644 --- a/libs/web-components/src/components/dropdown/Dropdown.svelte +++ b/libs/web-components/src/components/dropdown/Dropdown.svelte @@ -210,6 +210,7 @@ }); _popoverEl?.addEventListener("_close", (e) => { + e.stopPropagation(); _isMenuVisible = false; }); } diff --git a/libs/web-components/src/components/popover/Popover.svelte b/libs/web-components/src/components/popover/Popover.svelte index 0b9fa6d436..a18e255d17 100644 --- a/libs/web-components/src/components/popover/Popover.svelte +++ b/libs/web-components/src/components/popover/Popover.svelte @@ -200,6 +200,44 @@ } } + function isInPopoverComposedTree(node: EventTarget | null): boolean { + if (!_popoverEl || !(node instanceof Node)) { + return false; + } + + if (_popoverEl.contains(node)) { + return true; + } + + // Walk across slot and shadow boundaries to detect nested/slotted descendants. + let current: Node | null = node; + while (current) { + if (current === _popoverEl) { + return true; + } + + if (current instanceof Element && current.assignedSlot) { + current = current.assignedSlot; + continue; + } + + if (current.parentNode) { + current = current.parentNode; + continue; + } + + const rootNode = current.getRootNode?.(); + if (rootNode instanceof ShadowRoot) { + current = rootNode.host; + continue; + } + + current = null; + } + + return false; + } + // When one popover is opened it dispatches a `goa:closePopover` to the document.body element, so adding a listener // here will allow any other popover that is currently open to be closed function addGlobalCloseListener() { @@ -213,7 +251,10 @@ // the popover that is being opened will, at that time have the an open state, so we need to prevent // that one that is being opened be immediately closed. if (target !== _targetEl) { - closePopover(); + // Don't close if the target is a child popover (descendant of this popover's content) + if (target && !isInPopoverComposedTree(target)) { + closePopover(); + } } }); } From b1fff72163825349b9cb9b4991c6bde4c75e44a6 Mon Sep 17 00:00:00 2001 From: Benji Franck Date: Fri, 8 May 2026 10:09:23 -0600 Subject: [PATCH 2/2] chore: re-name PR files and titles from "3982" to "3892" --- .../bug3892.component.html} | 18 ++++++++--------- .../bug3892.component.ts} | 6 +++--- .../bug3892.route.json} | 4 ++-- .../{bug3982.route.ts => bug3892.route.ts} | 8 ++++---- .../routes/bugs/{bug3982.tsx => bug3892.tsx} | 20 +++++++++---------- 5 files changed, 28 insertions(+), 28 deletions(-) rename apps/prs/angular/src/routes/bugs/{3982/bug3982.component.html => 3892/bug3892.component.html} (95%) rename apps/prs/angular/src/routes/bugs/{3982/bug3982.component.ts => 3892/bug3892.component.ts} (95%) rename apps/prs/angular/src/routes/bugs/{3982/bug3982.route.json => 3892/bug3892.route.json} (59%) rename apps/prs/react/src/app/routes/bugs/{bug3982.route.ts => bug3892.route.ts} (56%) rename apps/prs/react/src/routes/bugs/{bug3982.tsx => bug3892.tsx} (95%) diff --git a/apps/prs/angular/src/routes/bugs/3982/bug3982.component.html b/apps/prs/angular/src/routes/bugs/3892/bug3892.component.html similarity index 95% rename from apps/prs/angular/src/routes/bugs/3982/bug3982.component.html rename to apps/prs/angular/src/routes/bugs/3892/bug3892.component.html index d4d76cd60e..3b8653f10d 100644 --- a/apps/prs/angular/src/routes/bugs/3982/bug3982.component.html +++ b/apps/prs/angular/src/routes/bugs/3892/bug3892.component.html @@ -1,6 +1,6 @@
-

3982 - Nested close propagation

+

3892 - Nested close propagation

This page tests the issue where popover close events are propegating to parent drawers and modals. @@ -24,12 +24,12 @@

Popover

and leaves the popover open.

Drawer and leaves the drawer open.

Push drawer and leaves the push drawer open.

Modal and leaves the modal open.

- 3982 - Nested close propagation + 3892 - Nested close propagation This page tests the issue where popover close events are propegating to parent drawers and modals. @@ -108,12 +108,12 @@ export function Bug3982Route() { and leaves the popover open.