Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/two-goats-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@stackoverflow/stacks": minor
"@stackoverflow/stacks-svelte": minor
---

Fix popover focus-leave dismissal
170 changes: 170 additions & 0 deletions packages/stacks-classic/lib/components/popover/popover.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
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`<a href="#" data-testid="popover-link">View more</a>`,
renderOutsideButton = true,
role = "menu",
}: {
content?: ReturnType<typeof html>;
renderOutsideButton?: boolean;
role?: string;
} = {}) => html`
<button
class="s-btn"
aria-controls="popover-example"
data-controller="s-popover"
data-action="s-popover#toggle"
data-testid="popover-trigger"
>
Toggle popover
</button>
<div
class="s-popover"
id="popover-example"
role="${role}"
data-testid="popover"
>
<div class="s-popover--content">${content}</div>
</div>
${renderOutsideButton
? html`<button data-testid="outside-button">Outside button</button>`
: 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 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 hide when focus moves from the trigger to an outside button", async () => {
await fixture(createPopover({ content: html`<span>View more</span>` }));

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");
});
});
48 changes: 48 additions & 0 deletions packages/stacks-classic/lib/components/popover/popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ export abstract class BasePopoverController extends Stacks.StacksController {
}
}

/**
* Only menu popovers dismiss when focus leaves the reference/popover pair.
*/
protected get shouldHideOnFocusLeave() {
return this.popoverElement?.getAttribute("role") === "menu";
}

/**
* Initializes and validates controller variables
*/
Expand Down Expand Up @@ -316,6 +323,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
Expand Down Expand Up @@ -352,9 +360,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
);
}

/**
Expand All @@ -363,6 +381,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
);
}

/**
Expand Down Expand Up @@ -402,6 +428,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.
Expand Down
48 changes: 43 additions & 5 deletions packages/stacks-svelte/src/components/Popover/Popover.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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<HTMLElement>) => void;
onFocusOut: (e: FocusEvent) => void;
open: () => void;
openTooltip: () => void;
close: () => void;
close: (options?: CloseOptions) => void;
closeTooltip: () => void;
toggle: () => void;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -198,6 +209,28 @@
close();
};

const onFocusOut = (e: FocusEvent) => {
if (tooltip || !pstate.visible || !pstate.closeOnFocusLeave) {
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();
Expand All @@ -213,14 +246,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,
Expand Down
Loading
Loading