fix(core): keep popover button trigger active while open EIX-151

.changeset/popover-trigger-open-active.md

@@ -0,0 +1,7 @@
+---
+'@siemens/ix': patch
+---
+
+Keep `ix-button` / `ix-icon-button` popover triggers in the Active look while the popover is open.
+
+Fixes #1402

packages/core/src/components/popover/popover.tsx

@@ -690,7 +690,7 @@ export class Popover
       return undefined;
     }
 
-    if (el.tagName === 'IX-BUTTON' || el.tagName === 'IX-ICON-BUTTON') {
+    if (this.isIxButtonTrigger(el)) {
       const inner = el.shadowRoot?.querySelector<HTMLElement>(
         'button, a[role="button"]'
       );
@@ -701,6 +701,21 @@ export class Popover
     return el;
   }
 
+  private isIxButtonTrigger(element: HTMLElement): boolean {
+    return (
+      element.tagName === 'IX-BUTTON' || element.tagName === 'IX-ICON-BUTTON'
+    );
+  }
+
+  private updateTriggerActive(expanded: boolean) {
+    const triggerElement = this.triggerElement;
+    if (!triggerElement || !this.isIxButtonTrigger(triggerElement)) {
+      return;
+    }
+
+    triggerElement.classList.toggle('active', expanded);
+  }
+
   private clearTriggerAriaAttributes(element: HTMLElement) {
     element.removeAttribute('aria-expanded');
     element.removeAttribute('aria-controls');
@@ -721,13 +736,16 @@ export class Popover
     if (target !== triggerElement) {
       this.clearTriggerAriaAttributes(triggerElement);
     }
+
+    this.updateTriggerActive(expanded);
   }
 
   private clearTriggerAria() {
     if (!this.triggerElement) {
       return;
     }
 
+    this.updateTriggerActive(false);
     this.clearTriggerAriaAttributes(this.triggerElement);
 
     const inner = this.triggerElement.shadowRoot?.querySelector<HTMLElement>(

packages/core/src/components/popover/test/popover.ct.ts

@@ -1256,5 +1256,145 @@ regressionTest.describe('ix-popover', () => {
         await popover.runAxe(makeAxeBuilder);
       }
     );
+
+    regressionTest(
+      'passes axe for icon-button trigger',
+      async ({ mount, page, makeAxeBuilder }) => {
+        await mount(
+          html`
+            <ix-icon-button
+              id="trigger"
+              icon="info"
+              aria-label="Open info"
+            ></ix-icon-button>
+            <ix-popover id="popover" trigger="trigger" close-on-click-outside>
+              <ix-popover-header>Title</ix-popover-header>
+              <ix-popover-content>Body</ix-popover-content>
+              <ix-popover-footer>
+                <ix-button id="popover-dismiss">Dismiss</ix-button>
+              </ix-popover-footer>
+            </ix-popover>
+          `,
+          { icons: { iconInfo } }
+        );
+
+        await page.locator('ix-icon-button#trigger').click();
+        await expect(page.locator('ix-popover')).toHaveAttribute('show', '');
+        await expect(page.locator('ix-icon-button#trigger')).toHaveClass(
+          /\bactive\b/
+        );
+
+        const results = await makeAxeBuilder()
+          .exclude('ix-icon-button')
+          .analyze();
+        expect(results.violations).toEqual([]);
+      }
+    );
+  });
+
+  regressionTest.describe('trigger active class', () => {
+    regressionTest(
+      'toggles active on ix-button open and close',
+      async ({ mount, page }) => {
+        await mountPopover(mount, page, interactivePopoverMarkup());
+        const popover = new PopoverPage(page);
+        const trigger = popover.trigger;
+
+        await expect(trigger).not.toHaveClass(/\bactive\b/);
+        await popover.open();
+        await expect(trigger).toHaveClass(/\bactive\b/);
+        await popover.closeWithEscape();
+        await expect(trigger).not.toHaveClass(/\bactive\b/);
+      }
+    );
+
+    regressionTest(
+      'toggles active on ix-icon-button open and close',
+      async ({ mount, page }) => {
+        await mount(
+          html`
+            <ix-icon-button
+              id="trigger"
+              icon="info"
+              aria-label="Open"
+            ></ix-icon-button>
+            <ix-popover id="popover" trigger="trigger">
+              <ix-popover-header>Title</ix-popover-header>
+              <ix-popover-content>Body</ix-popover-content>
+              <ix-popover-footer>
+                <ix-button id="popover-dismiss">Dismiss</ix-button>
+              </ix-popover-footer>
+            </ix-popover>
+          `,
+          { icons: { iconInfo } }
+        );
+
+        const trigger = page.locator('ix-icon-button#trigger');
+        const host = page.locator('ix-popover');
+
+        await expect(trigger).not.toHaveClass(/\bactive\b/);
+        await trigger.click();
+        await expect(host).toHaveAttribute('show', '');
+        await expect(trigger).toHaveClass(/\bactive\b/);
+        await page.keyboard.press('Escape');
+        await expect(host).not.toHaveAttribute('show', '');
+        await expect(trigger).not.toHaveClass(/\bactive\b/);
+      }
+    );
+
+    regressionTest(
+      'clears active when trigger is cleared',
+      async ({ mount, page }) => {
+        await mountPopover(mount, page, interactivePopoverMarkup());
+        const popover = new PopoverPage(page);
+        const trigger = popover.trigger;
+
+        await popover.open();
+        await expect(trigger).toHaveClass(/\bactive\b/);
+
+        await page
+          .locator('ix-popover')
+          .evaluate((el: HTMLIxPopoverElement) => {
+            el.trigger = undefined;
+          });
+
+        await expect(trigger).not.toHaveClass(/\bactive\b/);
+      }
+    );
+
+    regressionTest(
+      'moves active when trigger changes',
+      async ({ mount, page }) => {
+        await mount(html`
+          <ix-button id="trigger-a">Open A</ix-button>
+          <ix-button id="trigger-b">Open B</ix-button>
+          <ix-popover id="popover" trigger="trigger-a">
+            <ix-popover-header>Title</ix-popover-header>
+            <ix-popover-content>Body</ix-popover-content>
+            <ix-popover-footer>
+              <ix-button id="popover-dismiss">Dismiss</ix-button>
+            </ix-popover-footer>
+          </ix-popover>
+        `);
+
+        const host = page.locator('ix-popover');
+        const triggerA = page.locator('ix-button#trigger-a');
+        const triggerB = page.locator('ix-button#trigger-b');
+
+        await expect(triggerA).toHaveAttribute('data-ix-popover-trigger', '');
+        await host.evaluate((el: HTMLIxPopoverElement) => el.showPopover());
+        await expect(host).toHaveAttribute('show', '');
+        await expect(triggerA).toHaveClass(/\bactive\b/);
+        await expect(triggerB).not.toHaveClass(/\bactive\b/);
+
+        await host.evaluate((el: HTMLIxPopoverElement) => {
+          el.trigger = 'trigger-b';
+        });
+
+        await expect(triggerB).toHaveAttribute('data-ix-popover-trigger', '');
+        await expect(triggerA).not.toHaveClass(/\bactive\b/);
+        await expect(triggerB).toHaveClass(/\bactive\b/);
+      }
+    );
   });
 });

testing/visual-testing/tests/popover/popover.e2e.ts

@@ -7,14 +7,36 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import { expect } from '@playwright/test';
+import { expect, Page } from '@playwright/test';
 import { regressionTest } from '@utils/test';
 
 const snapshotOptions = {
   threshold: 0.05,
   maxDiffPixelRatio: 0.01,
 };
 
+async function openAndSettlePopover(page: Page) {
+  const popover = page.locator('ix-popover').first();
+  const trigger = page.locator('ix-button#trigger').first();
+
+  await expect(trigger).toBeVisible();
+  await popover.evaluate((el: HTMLIxPopoverElement) => el.showPopover());
+  await expect(popover).toHaveAttribute('show', '');
+
+  // Scope by panel id so nested popovers (slot content) do not match twice.
+  const panelId = await popover.getAttribute('data-ix-popover');
+  expect(panelId).toBeTruthy();
+  const dialog = page.locator(`dialog#${panelId}`);
+  await expect(dialog).toBeVisible();
+  await expect(trigger).toHaveClass(/\bactive\b/);
+
+  // Pointer off trigger so the snapshot is Active-while-open, not :hover.
+  await page.mouse.move(5, 5, { steps: 10 });
+  await expect(popover).toHaveAttribute('show', '');
+  await expect(dialog).toBeVisible();
+  await expect(trigger).toHaveClass(/\bactive\b/);
+}
+
 regressionTest.describe('popover', () => {
   (
     [
@@ -34,7 +56,7 @@ regressionTest.describe('popover', () => {
   ).forEach((variant) => {
     regressionTest(variant, async ({ page }) => {
       await page.goto(`popover/${variant}`);
-      await page.waitForTimeout(500);
+      await openAndSettlePopover(page);
 
       expect(await page.screenshot({ fullPage: true })).toMatchSnapshot(
         snapshotOptions
@@ -45,8 +67,16 @@ regressionTest.describe('popover', () => {
   regressionTest('hover-trigger', async ({ page }) => {
     await page.goto('popover/hover-trigger');
 
-    await page.locator('ix-button#trigger').hover();
-    await page.waitForTimeout(500);
+    const trigger = page.locator('ix-button#trigger');
+    const popover = page.locator('ix-popover');
+
+    await trigger.hover();
+    await expect(popover).toHaveAttribute('show', '');
+
+    const panelId = await popover.getAttribute('data-ix-popover');
+    expect(panelId).toBeTruthy();
+    await expect(page.locator(`dialog#${panelId}`)).toBeVisible();
+    await expect(trigger).toHaveClass(/\bactive\b/);
 
     expect(await page.screenshot({ fullPage: true })).toMatchSnapshot(
       snapshotOptions