diff --git a/.changeset/two-goats-press.md b/.changeset/two-goats-press.md new file mode 100644 index 0000000000..dcafe9a75e --- /dev/null +++ b/.changeset/two-goats-press.md @@ -0,0 +1,6 @@ +--- +"@stackoverflow/stacks": minor +"@stackoverflow/stacks-svelte": minor +--- + +Fix popover focus-leave dismissal diff --git a/packages/stacks-classic/lib/components/popover/popover.test.ts b/packages/stacks-classic/lib/components/popover/popover.test.ts new file mode 100644 index 0000000000..b84f1a3854 --- /dev/null +++ b/packages/stacks-classic/lib/components/popover/popover.test.ts @@ -0,0 +1,225 @@ +import { html, fixture, expect } from "@open-wc/testing"; +import { screen, waitFor } from "@testing-library/dom"; +import userEvent from "@testing-library/user-event"; +import "../../index"; + +const user = userEvent.setup(); + +const createPopover = ({ + content = html`View more`, + hideOnOutsideClick = "always", + renderOutsideButton = true, + role = "menu", +}: { + content?: ReturnType; + hideOnOutsideClick?: string; + renderOutsideButton?: boolean; + role?: string; +} = {}) => html` + +
+
${content}
+
+ ${renderOutsideButton + ? html`` + : null} +`; + +describe("popover", () => { + it('should set aria-expanded="true" when shown and "false" when hidden', async () => { + await fixture(createPopover({ renderOutsideButton: false })); + + const trigger = screen.getByTestId("popover-trigger"); + const popover = screen.getByTestId("popover"); + + await waitFor(() => + expect(trigger).to.have.attribute("aria-expanded", "false") + ); + + await user.click(trigger); + + await waitFor(() => expect(popover).to.have.class("is-visible")); + expect(trigger).to.have.attribute("aria-expanded", "true"); + + await user.click(trigger); + + await waitFor(() => expect(popover).not.to.have.class("is-visible")); + expect(trigger).to.have.attribute("aria-expanded", "false"); + }); + + it("should stay open when focus moves from the trigger into the popover", async () => { + await fixture(createPopover()); + + const trigger = screen.getByTestId("popover-trigger"); + const popover = screen.getByTestId("popover"); + const link = screen.getByTestId("popover-link"); + + await user.click(trigger); + await waitFor(() => expect(popover).to.have.class("is-visible")); + + await user.tab(); + + expect(link).to.have.focus; + expect(popover).to.have.class("is-visible"); + expect(trigger).to.have.attribute("aria-expanded", "true"); + }); + + it("should stay open when focus moves from the popover back to the trigger", async () => { + await fixture(createPopover()); + + const trigger = screen.getByTestId("popover-trigger"); + const popover = screen.getByTestId("popover"); + const link = screen.getByTestId("popover-link"); + + await user.click(trigger); + await waitFor(() => expect(popover).to.have.class("is-visible")); + + await user.tab(); + expect(link).to.have.focus; + + await user.tab({ shift: true }); + + expect(trigger).to.have.focus; + expect(popover).to.have.class("is-visible"); + expect(trigger).to.have.attribute("aria-expanded", "true"); + }); + + it("should hide when focus moves from the popover to an outside button", async () => { + await fixture(createPopover()); + + const trigger = screen.getByTestId("popover-trigger"); + const popover = screen.getByTestId("popover"); + const outsideButton = screen.getByTestId("outside-button"); + + await user.click(trigger); + await waitFor(() => expect(popover).to.have.class("is-visible")); + + await user.tab(); + await user.tab(); + + expect(outsideButton).to.have.focus; + await waitFor(() => expect(popover).not.to.have.class("is-visible")); + expect(trigger).to.have.attribute("aria-expanded", "false"); + }); + + it("should hide when focus leaves a popover containing a menu", async () => { + await fixture( + createPopover({ + content: html` + + `, + role: "dialog", + }) + ); + + const trigger = screen.getByTestId("popover-trigger"); + const popover = screen.getByTestId("popover"); + const outsideButton = screen.getByTestId("outside-button"); + + await user.click(trigger); + await waitFor(() => expect(popover).to.have.class("is-visible")); + + await user.tab(); + await user.tab(); + + expect(outsideButton).to.have.focus; + await waitFor(() => expect(popover).not.to.have.class("is-visible")); + expect(trigger).to.have.attribute("aria-expanded", "false"); + }); + + it("should stay open when focus moves outside a non-menu popover", async () => { + await fixture(createPopover({ role: "dialog" })); + + const trigger = screen.getByTestId("popover-trigger"); + const popover = screen.getByTestId("popover"); + const outsideButton = screen.getByTestId("outside-button"); + + await user.click(trigger); + await waitFor(() => expect(popover).to.have.class("is-visible")); + + await user.tab(); + await user.tab(); + + expect(outsideButton).to.have.focus; + expect(popover).to.have.class("is-visible"); + expect(trigger).to.have.attribute("aria-expanded", "true"); + }); + + it("should stay open when focus leaves a menu popover that disables auto-dismissal", async () => { + await fixture(createPopover({ hideOnOutsideClick: "never" })); + + const trigger = screen.getByTestId("popover-trigger"); + const popover = screen.getByTestId("popover"); + const outsideButton = screen.getByTestId("outside-button"); + + await user.click(trigger); + await waitFor(() => expect(popover).to.have.class("is-visible")); + + await user.tab(); + await user.tab(); + + expect(outsideButton).to.have.focus; + expect(popover).to.have.class("is-visible"); + expect(trigger).to.have.attribute("aria-expanded", "true"); + }); + + it("should hide when focus moves from the trigger to an outside button", async () => { + await fixture(createPopover({ content: html`View more` })); + + const trigger = screen.getByTestId("popover-trigger"); + const popover = screen.getByTestId("popover"); + const outsideButton = screen.getByTestId("outside-button"); + + await user.click(trigger); + await waitFor(() => expect(popover).to.have.class("is-visible")); + + await user.tab(); + + expect(outsideButton).to.have.focus; + await waitFor(() => expect(popover).not.to.have.class("is-visible")); + expect(trigger).to.have.attribute("aria-expanded", "false"); + }); + + it("should hide when focus leaves with no related target", async () => { + await fixture(createPopover()); + + const trigger = screen.getByTestId("popover-trigger"); + const popover = screen.getByTestId("popover"); + + await user.click(trigger); + await waitFor(() => expect(popover).to.have.class("is-visible")); + + trigger.dispatchEvent( + new FocusEvent("focusout", { + bubbles: true, + relatedTarget: null, + }) + ); + + expect(popover).not.to.have.class("is-visible"); + expect(trigger).to.have.attribute("aria-expanded", "false"); + }); +}); diff --git a/packages/stacks-classic/lib/components/popover/popover.ts b/packages/stacks-classic/lib/components/popover/popover.ts index 137d47069d..bfb14ad9cb 100644 --- a/packages/stacks-classic/lib/components/popover/popover.ts +++ b/packages/stacks-classic/lib/components/popover/popover.ts @@ -85,6 +85,17 @@ export abstract class BasePopoverController extends Stacks.StacksController { } } + /** + * Only menu popovers dismiss when focus leaves the reference/popover pair. + */ + protected get shouldHideOnFocusLeave() { + return ( + this.shouldHideOnOutsideClick && + (this.popoverElement?.getAttribute("role") === "menu" || + !!this.popoverElement?.querySelector('[role="menu"]')) + ); + } + /** * Initializes and validates controller variables */ @@ -316,6 +327,7 @@ export class PopoverController extends BasePopoverController { private boundHideOnOutsideClick!: (event: MouseEvent) => void; private boundHideOnEscapePress!: (event: KeyboardEvent) => void; + private boundHideOnFocusOut!: (event: FocusEvent) => void; /** * Toggles optional classes and accessibility attributes in addition to BasePopoverController.shown @@ -352,9 +364,19 @@ export class PopoverController extends BasePopoverController { this.boundHideOnOutsideClick || this.hideOnOutsideClick.bind(this); this.boundHideOnEscapePress = this.boundHideOnEscapePress || this.hideOnEscapePress.bind(this); + this.boundHideOnFocusOut = + this.boundHideOnFocusOut || this.hideOnFocusOut.bind(this); document.addEventListener("mousedown", this.boundHideOnOutsideClick); document.addEventListener("keyup", this.boundHideOnEscapePress); + this.referenceElement.addEventListener( + "focusout", + this.boundHideOnFocusOut + ); + this.popoverElement.addEventListener( + "focusout", + this.boundHideOnFocusOut + ); } /** @@ -363,6 +385,14 @@ export class PopoverController extends BasePopoverController { protected unbindDocumentEvents() { document.removeEventListener("mousedown", this.boundHideOnOutsideClick); document.removeEventListener("keyup", this.boundHideOnEscapePress); + this.referenceElement.removeEventListener( + "focusout", + this.boundHideOnFocusOut + ); + this.popoverElement.removeEventListener( + "focusout", + this.boundHideOnFocusOut + ); } /** @@ -402,6 +432,28 @@ export class PopoverController extends BasePopoverController { this.hide(e); } + /** + * Forces the popover to hide if keyboard focus leaves both the reference element and the popover. + * @param {FocusEvent} e - The focusout event from the reference or popover element + */ + private hideOnFocusOut(e: FocusEvent) { + if (!this.shouldHideOnFocusLeave) { + return; + } + + const relatedTarget = e.relatedTarget; + + if ( + relatedTarget instanceof Node && + (this.referenceElement.contains(relatedTarget) || + this.popoverElement.contains(relatedTarget)) + ) { + return; + } + + this.hide(e); + } + /** * Toggles all classes on the originating element based on the `class-toggle` data * @param {boolean=} show - A boolean indicating whether this is being triggered by a show or hide. diff --git a/packages/stacks-docs/src/docs/public/system/components/popovers.md b/packages/stacks-docs/src/docs/public/system/components/popovers.md index f95597fe61..d526aedaf2 100644 --- a/packages/stacks-docs/src/docs/public/system/components/popovers.md +++ b/packages/stacks-docs/src/docs/public/system/components/popovers.md @@ -178,6 +178,79 @@ To promote being able to tab to an open popover, it's best to place the popover +### Menu popovers + +Menu popovers are dismissed when keyboard focus leaves the reference and popover. Apply `role="menu"` to the contained menu, or to the `.s-popover` root, to enable this behavior. Generic popovers that keep the default `role="dialog"` stay open when focus moves outside. + +```html + + +``` + + +
+ + + + + + + + + + + + + + +
+
+ +
+
+ +
+
+ +
+
+
+
+ + +
+ +
+ ### Dismissible In the case of new feature callouts, it may be appropriate to include an explicit dismiss button. You can add one using the styling provided by `.s-popover--close`. diff --git a/packages/stacks-svelte/src/components/Popover/Popover.svelte b/packages/stacks-svelte/src/components/Popover/Popover.svelte index e07d6e94a8..7f0c199e6c 100644 --- a/packages/stacks-svelte/src/components/Popover/Popover.svelte +++ b/packages/stacks-svelte/src/components/Popover/Popover.svelte @@ -3,20 +3,27 @@ import type { Placement } from "@floating-ui/core"; import { getContext } from "svelte"; + export interface CloseOptions { + delay?: number; + restoreFocus?: boolean; + } + export interface PopoverState { id: string; controlled: boolean; visible: boolean | undefined; dismissible: boolean; trapFocus: boolean; + closeOnFocusLeave: boolean; computedPlacement: Placement; tooltip: boolean; floatingRef: (element: HTMLElement) => void; floatingContent: (element: HTMLElement) => void; onOutclick: (e: CustomEvent) => void; + onFocusOut: (e: FocusEvent) => void; open: () => void; openTooltip: () => void; - close: () => void; + close: (options?: CloseOptions) => void; closeTooltip: () => void; toggle: () => void; } @@ -127,6 +134,7 @@ const TOOLTIP_CLOSE_DELAY = 100; let reference: HTMLElement; + let content: HTMLElement; let activeTimeout: number; // if the visible prop is passed, the component is controlled @@ -167,16 +175,19 @@ const closeTooltip = () => { if (!tooltip) return; - close(TOOLTIP_CLOSE_DELAY); + close({ delay: TOOLTIP_CLOSE_DELAY }); }; - const close = (delay: number = 0) => { + const close = ({ + delay = 0, + restoreFocus = !tooltip, + }: CloseOptions = {}) => { window.clearTimeout(activeTimeout); if (controlled) return; if (pstate.visible) { activeTimeout = window.setTimeout(() => { pstate.visible = false; - if (!tooltip) { + if (restoreFocus) { reference.focus(); } // Fires when the popover is closed @@ -198,6 +209,33 @@ close(); }; + const onFocusOut = (e: FocusEvent) => { + if ( + tooltip || + !pstate.visible || + !pstate.closeOnFocusLeave || + !dismissible + ) { + return; + } + + const relatedTarget = e.relatedTarget; + if ( + relatedTarget instanceof Node && + (reference?.contains(relatedTarget) || + content?.contains(relatedTarget)) + ) { + return; + } + + if (controlled) { + onclose?.(); + return; + } + + close({ restoreFocus: false }); + }; + const onKeypress = (e: KeyboardEvent) => { if (e.key === "Escape" && dismissible) { close(); @@ -213,14 +251,19 @@ visible: autoshow, dismissible, trapFocus, + closeOnFocusLeave: false, computedPlacement: placement, tooltip, floatingRef: (element) => { reference = element; floatingRef(element); }, - floatingContent, + floatingContent: (element) => { + content = element; + floatingContent(element); + }, onOutclick, + onFocusOut, open, openTooltip, close, diff --git a/packages/stacks-svelte/src/components/Popover/Popover.test.ts b/packages/stacks-svelte/src/components/Popover/Popover.test.ts index fbd1559a1a..b653e3c17c 100644 --- a/packages/stacks-svelte/src/components/Popover/Popover.test.ts +++ b/packages/stacks-svelte/src/components/Popover/Popover.test.ts @@ -2,7 +2,7 @@ import { mount, unmount, tick, createRawSnippet } from "svelte"; import type { Strategy } from "@floating-ui/core"; import sinon from "sinon"; import { expect } from "@open-wc/testing"; -import { render, screen } from "@testing-library/svelte"; +import { render, screen, waitFor } from "@testing-library/svelte"; import userEvent from "@testing-library/user-event"; import { setViewport } from "@web/test-runner-commands"; import { createSvelteComponentsSnippet } from "../../../test-utils"; @@ -43,6 +43,12 @@ const defaultChildren = { }, }; +afterEach(() => { + document + .querySelectorAll("[data-testid='popover-outside-button']") + .forEach((el) => el.remove()); +}); + describe("Popover", () => { it("should show/hide the popover content when the user click on the reference", async () => { render(Popover, { @@ -680,6 +686,46 @@ describe("Popover", () => { }); describe("when not in tooltip mode", () => { + const focusableContent = { + component: PopoverContent, + props: { + children: createRawSnippet(() => ({ + render: () => + '', + })), + }, + }; + + const focusableMenuContent = { + component: PopoverContent, + props: { + role: "menu", + children: createRawSnippet(() => ({ + render: () => + '', + })), + }, + }; + + const menuContent = { + component: PopoverContent, + props: { + role: "menu", + children: createRawSnippet(() => ({ + render: () => "Popover Content", + })), + }, + }; + + const addOutsideButton = () => { + const outsideButton = document.createElement("button"); + outsideButton.type = "button"; + outsideButton.textContent = "Outside action"; + outsideButton.dataset.testid = "popover-outside-button"; + document.body.append(outsideButton); + return outsideButton; + }; + it("should throw an error if the reference element provided does not have any children of role button", () => { expect(() => render(Popover, { @@ -750,6 +796,229 @@ describe("Popover", () => { expect(reference).to.have.attribute("aria-expanded", "false"); }); + it("should keep the popover open when focus moves from the reference into the content", async () => { + render(Popover, { + props: { + ...defaultProps, + children: createSvelteComponentsSnippet([ + defaultChildren.reference, + focusableMenuContent, + ]), + }, + }); + + const reference = screen.getByRole("button", { name: "Trigger" }); + + await userEvent.click(reference); + expect(screen.getByRole("menu")).to.exist; + + await userEvent.tab(); + + expect(screen.getByRole("menuitem", { name: "Popover action" })).to + .have.focus; + expect(screen.getByRole("menu")).to.exist; + expect(reference).to.have.attribute("aria-expanded", "true"); + }); + + it("should keep the popover open when focus moves from the content back to the reference", async () => { + render(Popover, { + props: { + ...defaultProps, + children: createSvelteComponentsSnippet([ + defaultChildren.reference, + focusableMenuContent, + ]), + }, + }); + + const reference = screen.getByRole("button", { name: "Trigger" }); + + await userEvent.click(reference); + expect(screen.getByRole("menu")).to.exist; + + await userEvent.tab(); + expect(screen.getByRole("menuitem", { name: "Popover action" })).to + .have.focus; + + await userEvent.tab({ shift: true }); + + expect(reference).to.have.focus; + expect(screen.getByRole("menu")).to.exist; + expect(reference).to.have.attribute("aria-expanded", "true"); + }); + + it("should close the popover without restoring focus when focus moves outside", async () => { + render(Popover, { + props: { + ...defaultProps, + children: createSvelteComponentsSnippet([ + defaultChildren.reference, + focusableMenuContent, + ]), + }, + }); + const outsideButton = addOutsideButton(); + + const reference = screen.getByRole("button", { name: "Trigger" }); + + await userEvent.click(reference); + expect(screen.getByRole("menu")).to.exist; + + await userEvent.tab(); + await userEvent.tab(); + + expect(outsideButton).to.have.focus; + await waitFor( + () => expect(screen.queryByRole("menu")).not.to.exist + ); + expect(reference).to.have.attribute("aria-expanded", "false"); + expect(reference).not.to.have.focus; + }); + + it("should stay open when focus moves outside a non-menu popover", async () => { + render(Popover, { + props: { + ...defaultProps, + children: createSvelteComponentsSnippet([ + defaultChildren.reference, + focusableContent, + ]), + }, + }); + const outsideButton = addOutsideButton(); + + const reference = screen.getByRole("button", { name: "Trigger" }); + + await userEvent.click(reference); + expect(screen.getByRole("dialog")).to.exist; + + await userEvent.tab(); + await userEvent.tab(); + + expect(outsideButton).to.have.focus; + expect(screen.getByRole("dialog")).to.exist; + expect(reference).to.have.attribute("aria-expanded", "true"); + expect(reference).not.to.have.focus; + }); + + it("should stay open when focus leaves a menu popover that is not dismissible", async () => { + render(Popover, { + props: { + ...defaultProps, + dismissible: false, + children: createSvelteComponentsSnippet([ + defaultChildren.reference, + focusableMenuContent, + ]), + }, + }); + const outsideButton = addOutsideButton(); + + const reference = screen.getByRole("button", { name: "Trigger" }); + + await userEvent.click(reference); + expect(screen.getByRole("menu")).to.exist; + + await userEvent.tab(); + await userEvent.tab(); + + expect(outsideButton).to.have.focus; + expect(screen.getByRole("menu")).to.exist; + expect(reference).to.have.attribute("aria-expanded", "true"); + expect(reference).not.to.have.focus; + }); + + it("should close the popover when focus moves from the reference directly outside", async () => { + render(Popover, { + props: { + ...defaultProps, + children: createSvelteComponentsSnippet([ + defaultChildren.reference, + menuContent, + ]), + }, + }); + const outsideButton = addOutsideButton(); + + const reference = screen.getByRole("button", { name: "Trigger" }); + + await userEvent.click(reference); + expect(screen.getByRole("menu")).to.exist; + + await userEvent.tab(); + + expect(outsideButton).to.have.focus; + await waitFor( + () => expect(screen.queryByRole("menu")).not.to.exist + ); + expect(reference).to.have.attribute("aria-expanded", "false"); + expect(reference).not.to.have.focus; + }); + + it("should close the popover when focus leaves with no related target", async () => { + render(Popover, { + props: { + ...defaultProps, + children: createSvelteComponentsSnippet([ + defaultChildren.reference, + menuContent, + ]), + }, + }); + + const reference = screen.getByRole("button", { name: "Trigger" }); + + await userEvent.click(reference); + expect(screen.getByRole("menu")).to.exist; + + reference.dispatchEvent( + new FocusEvent("focusout", { + bubbles: true, + relatedTarget: null, + }) + ); + + await waitFor( + () => expect(screen.queryByRole("menu")).not.to.exist + ); + expect(reference).to.have.attribute("aria-expanded", "false"); + }); + + it("should request close without mutating visibility when a controlled popover loses focus", async () => { + const onCloseSpy = sinon.spy(); + const { rerender } = render(Popover, { + props: { + ...defaultProps, + visible: true, + onclose: onCloseSpy, + children: createSvelteComponentsSnippet([ + defaultChildren.reference, + focusableMenuContent, + ]), + }, + }); + const outsideButton = addOutsideButton(); + + const reference = screen.getByRole("button", { name: "Trigger" }); + + await userEvent.tab(); + expect(reference).to.have.focus; + + await userEvent.tab(); + expect(screen.getByRole("menuitem", { name: "Popover action" })).to + .have.focus; + expect(onCloseSpy).not.to.have.been.called; + + await userEvent.tab(); + expect(outsideButton).to.have.focus; + expect(onCloseSpy).to.have.been.calledOnce; + expect(screen.getByRole("menu")).to.exist; + + rerender({ visible: false }); + await tick(); + expect(screen.queryByRole("menu")).not.to.exist; + }); + it("should not show/hide the tooltip content when the user hovers on the reference", async () => { render(Popover, { props: { diff --git a/packages/stacks-svelte/src/components/Popover/PopoverContent.svelte b/packages/stacks-svelte/src/components/Popover/PopoverContent.svelte index 11dcf180ea..eb3b37934e 100644 --- a/packages/stacks-svelte/src/components/Popover/PopoverContent.svelte +++ b/packages/stacks-svelte/src/components/Popover/PopoverContent.svelte @@ -66,6 +66,15 @@ let computedRole = $derived( role || (pstate.tooltip ? "tooltip" : "dialog") ); + + const onFocusOut = (e: FocusEvent) => { + pstate.closeTooltip(); + pstate.onFocusOut(e); + }; + + $effect(() => { + pstate.closeOnFocusLeave = !pstate.tooltip && computedRole === "menu"; + }); @@ -82,7 +91,7 @@ onmouseenter={pstate.openTooltip} onmouseleave={pstate.closeTooltip} onfocusin={pstate.openTooltip} - onfocusout={pstate.closeTooltip} + onfocusout={onFocusOut} data-popper-placement={pstate.computedPlacement} >
diff --git a/packages/stacks-svelte/src/components/Popover/PopoverReference.svelte b/packages/stacks-svelte/src/components/Popover/PopoverReference.svelte index e08a196f51..06c3780890 100644 --- a/packages/stacks-svelte/src/components/Popover/PopoverReference.svelte +++ b/packages/stacks-svelte/src/components/Popover/PopoverReference.svelte @@ -66,7 +66,11 @@ ref.setAttribute("aria-controls", `${pstate.id}-popover`); const toggle = pstate.dismissible ? pstate.toggle : pstate.open; ref.addEventListener("click", toggle); - return () => ref.removeEventListener("click", toggle); + ref.addEventListener("focusout", pstate.onFocusOut); + return () => { + ref.removeEventListener("click", toggle); + ref.removeEventListener("focusout", pstate.onFocusOut); + }; }; const setupTooltip = (ref: HTMLElement, pstate: PopoverState) => { @@ -83,13 +87,20 @@ }; }; + const setupControlledPopover = (ref: HTMLElement, pstate: PopoverState) => { + if (!pstate.tooltip) { + ref.addEventListener("focusout", pstate.onFocusOut); + } + return () => ref.removeEventListener("focusout", pstate.onFocusOut); + }; + onMount(() => { reference = setupRef(elementId, referenceWrapper, pstate); // if the popover is controlled, we delegate all the behavior to the consumer - if (pstate.controlled) return; + if (pstate.controlled) return setupControlledPopover(reference, pstate); - pstate.tooltip + return pstate.tooltip ? setupTooltip(reference, pstate) : setupPopover(reference, pstate); });