Skip to content
Open
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
225 changes: 225 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,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`<a href="#" data-testid="popover-link">View more</a>`,
hideOnOutsideClick = "always",
renderOutsideButton = true,
role = "menu",
}: {
content?: ReturnType<typeof html>;
hideOnOutsideClick?: string;
renderOutsideButton?: boolean;
role?: string;
} = {}) => html`
<button
class="s-btn"
aria-controls="popover-example"
data-controller="s-popover"
data-action="s-popover#toggle"
data-s-popover-hide-on-outside-click="${hideOnOutsideClick}"
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 hide when focus leaves a popover containing a menu", async () => {
await fixture(
createPopover({
content: html`
<ul class="s-menu" role="menu">
<li class="s-menu--item" role="menuitem">
<button
class="s-menu--action"
data-testid="popover-link"
>
View more
</button>
</li>
</ul>
`,
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`<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");
});
});
52 changes: 52 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,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
*/
Expand Down Expand Up @@ -322,6 +333,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 @@ -358,9 +370,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 @@ -369,6 +391,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 @@ -408,6 +438,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
Loading