fix(select): emit single ionChange event for popover option selection (#26796)

Resolves #26789

core/src/components/select-popover/select-popover.tsx

@@ -1,14 +1,14 @@
 import type { ComponentInterface } from '@stencil/core';
-import { Component, Host, Listen, Prop, h } from '@stencil/core';
+import { Element, Component, Host, Prop, h } from '@stencil/core';
 
 import { getIonMode } from '../../global/ionic-global';
 import { safeCall } from '../../utils/overlays';
 import { getClassMap } from '../../utils/theme';
+import type { CheckboxCustomEvent } from '../checkbox/checkbox-interface';
+import type { RadioGroupCustomEvent } from '../radio-group/radio-group-interface';
 
 import type { SelectPopoverOption } from './select-popover-interface';
 
-// TODO(FW-2832): types
-
 /**
  * @internal
  */
@@ -21,6 +21,7 @@ import type { SelectPopoverOption } from './select-popover-interface';
   scoped: true,
 })
 export class SelectPopover implements ComponentInterface {
+  @Element() el!: HTMLIonSelectPopoverElement;
   /**
    * The header text of the popover
    */
@@ -46,13 +47,7 @@ export class SelectPopover implements ComponentInterface {
    */
   @Prop() options: SelectPopoverOption[] = [];
 
-  @Listen('ionChange')
-  onSelect(ev: any) {
-    this.setChecked(ev);
-    this.callOptionHandler(ev);
-  }
-
-  private findOptionFromEvent(ev: any) {
+  private findOptionFromEvent(ev: CheckboxCustomEvent | RadioGroupCustomEvent) {
     const { options } = this;
     return options.find((o) => o.value === ev.target.value);
   }
@@ -62,7 +57,7 @@ export class SelectPopover implements ComponentInterface {
    * of the selected option(s) and return it in the option
    * handler
    */
-  private callOptionHandler(ev: any) {
+  private callOptionHandler(ev: CheckboxCustomEvent | RadioGroupCustomEvent) {
     const option = this.findOptionFromEvent(ev);
     const values = this.getValues(ev);
 
@@ -72,15 +67,17 @@ export class SelectPopover implements ComponentInterface {
   }
 
   /**
-   * This is required when selecting a radio that is already
-   * selected because it will not trigger the ionChange event
-   * but we still want to close the popover
+   * Dismisses the host popover that the `ion-select-popover`
+   * is rendered within.
    */
-  private rbClick(ev: any) {
-    this.callOptionHandler(ev);
+  private dismissParentPopover() {
+    const popover = this.el.closest('ion-popover');
+    if (popover) {
+      popover.dismiss();
+    }
   }
 
-  private setChecked(ev: any): void {
+  private setChecked(ev: CheckboxCustomEvent): void {
     const { multiple } = this;
     const option = this.findOptionFromEvent(ev);
 
@@ -91,7 +88,7 @@ export class SelectPopover implements ComponentInterface {
     }
   }
 
-  private getValues(ev: any): any | any[] | null {
+  private getValues(ev: CheckboxCustomEvent | RadioGroupCustomEvent): string | string[] | undefined {
     const { multiple, options } = this;
 
     if (multiple) {
@@ -125,6 +122,11 @@ export class SelectPopover implements ComponentInterface {
           value={option.value}
           disabled={option.disabled}
           checked={option.checked}
+          legacy={true}
+          onIonChange={(ev) => {
+            this.setChecked(ev);
+            this.callOptionHandler(ev);
+          }}
         ></ion-checkbox>
         <ion-label>{option.text}</ion-label>
       </ion-item>
@@ -135,11 +137,26 @@ export class SelectPopover implements ComponentInterface {
     const checked = options.filter((o) => o.checked).map((o) => o.value)[0];
 
     return (
-      <ion-radio-group value={checked}>
+      <ion-radio-group value={checked} onIonChange={(ev) => this.callOptionHandler(ev)}>
         {options.map((option) => (
           <ion-item class={getClassMap(option.cssClass)}>
             <ion-label>{option.text}</ion-label>
-            <ion-radio value={option.value} disabled={option.disabled} onClick={(ev) => this.rbClick(ev)}></ion-radio>
+            <ion-radio
+              value={option.value}
+              disabled={option.disabled}
+              legacy={true}
+              onClick={() => this.dismissParentPopover()}
+              onKeyUp={(ev) => {
+                if (ev.key === ' ') {
+                  /**
+                   * Selecting a radio option with keyboard navigation,
+                   * either through the Enter or Space keys, should
+                   * dismiss the popover.
+                   */
+                  this.dismissParentPopover();
+                }
+              }}
+            ></ion-radio>
           </ion-item>
         ))}
       </ion-radio-group>

core/src/components/select-popover/test/basic/index.html

@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<html lang="en" dir="ltr">
+  <head>
+    <meta charset="UTF-8" />
+    <title>Select - Popover</title>
+    <meta
+      name="viewport"
+      content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no"
+    />
+    <link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
+    <link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
+    <script src="../../../../../scripts/testing/scripts.js"></script>
+    <script nomodule src="../../../../../dist/ionic/ionic.js"></script>
+    <script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
+  </head>
+
+  <body>
+    <ion-app>
+      <ion-header>
+        <ion-toolbar>
+          <ion-title>Select Popover - Basic</ion-title>
+        </ion-toolbar>
+      </ion-header>
+
+      <ion-content>
+        <ion-popover class="select-popover" is-open="true">
+          <ion-select-popover multiple="false"></ion-select-popover>
+        </ion-popover>
+      </ion-content>
+    </ion-app>
+
+    <script>
+      const selectPopover = document.querySelector('ion-select-popover');
+      selectPopover.options = [
+        { value: 'apple', text: 'Apple', disabled: false, checked: true },
+        { value: 'banana', text: 'Banana', disabled: false, checked: false },
+      ];
+    </script>
+  </body>
+</html>

core/src/components/select-popover/test/basic/select-popover.e2e.ts

@@ -0,0 +1,65 @@
+import { expect } from '@playwright/test';
+import { test } from '@utils/test/playwright';
+
+import type { SelectPopoverOption } from '../../select-popover-interface';
+import { SelectPopoverPage } from '../fixtures';
+
+const options: SelectPopoverOption[] = [
+  { value: 'apple', text: 'Apple', disabled: false, checked: false },
+  { value: 'banana', text: 'Banana', disabled: false, checked: false },
+];
+
+const checkedOptions: SelectPopoverOption[] = [
+  { value: 'apple', text: 'Apple', disabled: false, checked: true },
+  { value: 'banana', text: 'Banana', disabled: false, checked: false },
+];
+
+test.describe('select-popover: basic', () => {
+  test.beforeEach(({ skip, browserName }) => {
+    skip.rtl();
+    skip.mode('ios', 'Consistent behavior across modes');
+    test.skip(browserName === 'webkit', 'https://ionic-cloud.atlassian.net/browse/FW-2979');
+  });
+
+  test.describe('single selection', () => {
+    let selectPopoverPage: SelectPopoverPage;
+
+    test.beforeEach(async ({ page }) => {
+      selectPopoverPage = new SelectPopoverPage(page);
+    });
+
+    test('clicking an unselected option should dismiss the popover', async () => {
+      await selectPopoverPage.setup(options, false);
+
+      await selectPopoverPage.clickOption('apple');
+      await selectPopoverPage.ionPopoverDidDismiss.next();
+      await expect(selectPopoverPage.popover).not.toBeVisible();
+    });
+
+    test('clicking a selected option should dismiss the popover', async () => {
+      await selectPopoverPage.setup(checkedOptions, false);
+
+      await selectPopoverPage.clickOption('apple');
+      await selectPopoverPage.ionPopoverDidDismiss.next();
+      await expect(selectPopoverPage.popover).not.toBeVisible();
+    });
+
+    test('pressing Space on an unselected option should dismiss the popover', async () => {
+      await selectPopoverPage.setup(options, false);
+
+      await selectPopoverPage.pressSpaceOnOption('apple');
+      await selectPopoverPage.ionPopoverDidDismiss.next();
+      await expect(selectPopoverPage.popover).not.toBeVisible();
+    });
+
+    test('pressing Space on a selected option should dismiss the popover', async ({ browserName }) => {
+      test.skip(browserName === 'firefox', 'Same behavior as https://ionic-cloud.atlassian.net/browse/FW-2979');
+
+      await selectPopoverPage.setup(checkedOptions, false);
+
+      await selectPopoverPage.pressSpaceOnOption('apple');
+      await selectPopoverPage.ionPopoverDidDismiss.next();
+      await expect(selectPopoverPage.popover).not.toBeVisible();
+    });
+  });
+});

core/src/components/select-popover/test/fixtures.ts

@@ -0,0 +1,65 @@
+import type { E2EPage, E2ELocator, EventSpy } from '@utils/test/playwright';
+
+import type { SelectPopoverOption } from '../select-popover-interface';
+
+export class SelectPopoverPage {
+  private page: E2EPage;
+  private multiple?: boolean;
+  private options: SelectPopoverOption[] = [];
+
+  // Locators
+  popover!: E2ELocator;
+  selectPopover!: E2ELocator;
+
+  // Event spies
+  ionPopoverDidDismiss!: EventSpy;
+
+  constructor(page: E2EPage) {
+    this.page = page;
+  }
+
+  async setup(options: SelectPopoverOption[], multiple = false) {
+    const { page } = this;
+
+    await page.setContent(`
+    <ion-popover>
+      <ion-select-popover></ion-select-popover>
+    </ion-popover>
+    <script>
+      const selectPopover = document.querySelector('ion-select-popover');
+      selectPopover.options = ${JSON.stringify(options)};
+      selectPopover.multiple = ${multiple};
+    </script>
+    `);
+
+    const ionPopoverDidPresent = await page.spyOnEvent('ionPopoverDidPresent');
+    this.ionPopoverDidDismiss = await page.spyOnEvent('ionPopoverDidDismiss');
+
+    this.popover = page.locator('ion-popover');
+    this.selectPopover = page.locator('ion-select-popover');
+    this.multiple = multiple;
+    this.options = options;
+
+    await this.popover.evaluate((popover: HTMLIonPopoverElement) => popover.present());
+
+    await ionPopoverDidPresent.next();
+  }
+
+  async clickOption(value: string) {
+    const option = this.getOption(value);
+    await option.click();
+  }
+
+  async pressSpaceOnOption(value: string) {
+    const option = this.getOption(value);
+    await option.press('Space');
+  }
+
+  private getOption(value: string) {
+    const { multiple, selectPopover } = this;
+    const selector = multiple ? 'ion-checkbox' : 'ion-radio';
+    const index = this.options.findIndex((o) => o.value === value);
+
+    return selectPopover.locator(selector).nth(index);
+  }
+}