Skip to content

Commit

Permalink
fix(action-menu): fix toggle logic when action-menu is reconnected (#…
Browse files Browse the repository at this point in the history
…11139)

**Related Issue:** #10731 

## Summary

Fixes a regression caused by #10310 where the change to the internal
popover to use `triggerDisabled` would no longer work when the component
was reconnected.
  • Loading branch information
jcfranco authored and benelan committed Feb 8, 2025
1 parent 386355b commit 064d783
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { newE2EPage } from "@arcgis/lumina-compiler/puppeteerTesting";
import { describe, expect, it } from "vitest";
import { E2EPage, newE2EPage } from "@arcgis/lumina-compiler/puppeteerTesting";
import { beforeEach, describe, expect, it } from "vitest";
import { html } from "../../../support/formatting";
import {
accessible,
Expand Down Expand Up @@ -202,6 +202,53 @@ describe("calcite-action-menu", () => {
await isElementFocused(page, focusTargetSelector);
});

describe("adding/removing from DOM", () => {
let page: E2EPage;

beforeEach(async (): Promise<void> => {
page = await newE2EPage();
await skipAnimations(page);
});

async function testToggle(triggerSelector: string): Promise<void> {
await page.evaluate(() => {
const actionMenu = document.querySelector("calcite-action-menu");
actionMenu.remove();
document.body.append(actionMenu);
});
await page.waitForChanges();

const trigger = await page.find(triggerSelector);
await trigger.click();

const actionMenu = await page.find("calcite-action-menu");
expect(await actionMenu.getProperty("open")).toBe(true);
}

it("should toggle with default trigger", async () => {
await page.setContent(html`
<calcite-action-menu>
<calcite-action text="Add" icon="plus" text-enabled></calcite-action>
<calcite-action text="Remove" icon="minus" text-enabled></calcite-action>
<calcite-action text="Banana" icon="banana" text-enabled></calcite-action>
</calcite-action-menu>
`);
await testToggle(`calcite-action-menu >>> .${CSS.defaultTrigger}`);
});

it("should toggle with slotted trigger", async () => {
await page.setContent(html`
<calcite-action-menu>
<calcite-action id="trigger" slot="${SLOTS.trigger}" text="Toggle" icon="toggle"></calcite-action>
<calcite-action text="Add" icon="plus" text-enabled></calcite-action>
<calcite-action text="Remove" icon="minus" text-enabled></calcite-action>
<calcite-action text="Banana" icon="banana" text-enabled></calcite-action>
</calcite-action-menu>
`);
await testToggle("#trigger");
});
});

it("should honor scale of expand icon", async () => {
const page = await newE2EPage({ html: `<calcite-action-menu scale="l"></calcite-action-menu>` });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,8 @@ export class ActionMenu extends LitElement implements LoadableComponent {
"keydown",
this.menuButtonKeyDown,
) /* TODO: [MIGRATION] If possible, refactor to use on* JSX prop or this.listen()/this.listenOn() utils - they clean up event listeners automatically, thus prevent memory leaks */;

this.menuButtonEl = null;
}

private setMenuButtonEl(event: Event): void {
Expand All @@ -334,7 +336,10 @@ export class ActionMenu extends LitElement implements LoadableComponent {

private setDefaultMenuButtonEl(el: Action["el"]): void {
this.defaultMenuButtonEl = el;
this.connectMenuButtonEl();

if (el) {
this.connectMenuButtonEl();
}
}

private setPopoverEl(el: Popover["el"]): void {
Expand Down

0 comments on commit 064d783

Please sign in to comment.