fix(select): select focused option on Enter in popover and modal interfaces (#31093)

Issue number: resolves #30561

---------

<!-- Please do not submit updates to dependencies unless it fixes an
issue. -->

<!-- Please try to limit your pull request to one type (bugfix, feature,
etc). Submit multiple pull requests if needed. -->

## What is the current behavior?
<!-- Please describe the current behavior that you are modifying. -->

When an `ion-select` with `interface="popover"` or `interface="modal"`
is opened with the keyboard, pressing Enter on a focused option doesn't
commit the value or dismiss the overlay. Only Space works. The `onKeyUp`
handlers in `select-popover.tsx` and `select-modal.tsx` only checked for
`' '`, despite the comment claiming Enter was supported, and
`ion-radio-group`'s arrow-key handler never committed the focused value
on Enter the way native `<select>` does.

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

Pressing Enter on a focused option in the popover or modal interface now
commits that option as the select's value and dismisses the overlay,
matching Space and native `<select>` behavior. `radio-group.tsx` gains
an Enter branch inside its select-interface arrow-key handler that sets
`value` to the focused radio and emits `ionChange` when it differs from
the previous value. `select-popover.tsx` and `select-modal.tsx` track
the `onKeyDown` target so `onKeyUp` only dismisses when the Enter press
started on the same option. That guard stops a held Enter on the
triggering `ion-select` from re-firing inside the just-opened overlay
and auto-dismissing it. The `!ev.repeat` check on keydown covers the
same case for the radio-group commit.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

The keydown/keyup target-matching pattern is the same trick the browser
uses to avoid firing a click when keydown and keyup happen on different
elements. It's needed here because Enter on the `ion-select` trigger
opens the overlay on keydown, and without the guard the corresponding
keyup (now inside the overlay) would immediately commit and dismiss.

Preview page:
- [select /
basic](https://ionic-framework-git-fw-6754-ionic1.vercel.app/src/components/select/test/basic?ionic)

Author: ShaneK

core/src/components/radio-group/radio-group.tsx

@@ -290,6 +290,19 @@ export class RadioGroup implements ComponentInterface {
         // to the bottom of the screen
         ev.preventDefault();
       }
+
+      // Inside a select interface, Enter commits the focused radio
+      // value (matching native <select>). The !ev.repeat guard stops
+      // a held Enter on the triggering ion-select from re-committing
+      // once focus lands in the opened popover/modal.
+      if (ev.key === 'Enter' && inSelectInterface && !ev.repeat) {
+        const previousValue = this.value;
+        this.value = current.value;
+        if (previousValue !== this.value) {
+          this.emitValueChange(ev);
+        }
+        ev.preventDefault();
+      }
     }
   }
 

core/src/components/radio-group/test/basic/radio-group.e2e.ts

@@ -46,6 +46,32 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
       await radioFixture.expectChecked(false);
     });
 
+    test('Enter outside a select interface should not change the value', async ({ page }, testInfo) => {
+      testInfo.annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/30561',
+      });
+
+      await page.setContent(
+        `
+        <ion-radio-group value="one">
+          <ion-item>
+            <ion-radio id="one" value="one">One</ion-radio>
+          </ion-item>
+          <ion-item>
+            <ion-radio id="two" value="two">Two</ion-radio>
+          </ion-item>
+        </ion-radio-group>
+      `,
+        config
+      );
+
+      await radioFixture.checkRadio('keyboard', 'ion-radio#two', 'Enter');
+
+      const group = page.locator('ion-radio-group');
+      await expect(group).toHaveJSProperty('value', 'one');
+    });
+
     test('click should not deselect without allowEmptySelection', async ({ page }) => {
       await page.setContent(
         `

core/src/components/radio-group/test/fixtures.ts

@@ -11,13 +11,13 @@ export class RadioFixture {
     this.page = page;
   }
 
-  async checkRadio(method: 'keyboard' | 'mouse', selector = 'ion-radio') {
+  async checkRadio(method: 'keyboard' | 'mouse', selector = 'ion-radio', key: 'Space' | 'Enter' = 'Space') {
     const { page } = this;
     const radio = (this.radio = page.locator(selector));
 
     if (method === 'keyboard') {
       await radio.focus();
-      await page.keyboard.press('Space');
+      await page.keyboard.press(key);
     } else {
       await radio.click();
     }

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

@@ -21,6 +21,11 @@ import type { SelectModalOption } from './select-modal-interface';
 export class SelectModal implements ComponentInterface {
   @Element() el!: HTMLIonSelectModalElement;
 
+  // Tracks the option that received Enter-keydown so keyup only
+  // dismisses when the press started on the same option. Prevents
+  // Enter on the triggering ion-select from auto-dismissing.
+  private pendingEnterTarget: HTMLElement | null = null;
+
   @Prop() header?: string;
 
   /**
@@ -99,14 +104,21 @@ export class SelectModal implements ComponentInterface {
               justify="start"
               labelPlacement="end"
               onClick={() => this.closeModal()}
+              onKeyDown={(ev) => {
+                if (ev.key === 'Enter' && !ev.repeat) {
+                  this.pendingEnterTarget = ev.currentTarget as HTMLElement;
+                }
+              }}
               onKeyUp={(ev) => {
                 if (ev.key === ' ') {
-                  /**
-                   * Selecting a radio option with keyboard navigation,
-                   * either through the Enter or Space keys, should
-                   * dismiss the modal.
-                   */
+                  // Space selects and dismisses in one press.
                   this.closeModal();
+                } else if (ev.key === 'Enter') {
+                  const shouldClose = this.pendingEnterTarget === ev.currentTarget;
+                  this.pendingEnterTarget = null;
+                  if (shouldClose) {
+                    this.closeModal();
+                  }
                 }
               }}
             >

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

@@ -64,6 +64,35 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
         await expect(selectModalPage.modal).not.toBeVisible();
       });
 
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      test('pressing Enter on an unselected option should dismiss the modal', async ({ page: _page }, testInfo) => {
+        testInfo.annotations.push({
+          type: 'issue',
+          description: 'https://github.com/ionic-team/ionic-framework/issues/30561',
+        });
+
+        await selectModalPage.setup(config, options, false);
+
+        await selectModalPage.pressEnterOnOption('apple');
+        await selectModalPage.ionModalDidDismiss.next();
+        await expect(selectModalPage.modal).not.toBeVisible();
+      });
+
+      test('pressing Enter on a selected option should dismiss the modal', async ({ browserName }, testInfo) => {
+        testInfo.annotations.push({
+          type: 'issue',
+          description: 'https://github.com/ionic-team/ionic-framework/issues/30561',
+        });
+
+        test.skip(browserName === 'firefox', 'Same behavior as ROU-5437');
+
+        await selectModalPage.setup(config, checkedOptions, false);
+
+        await selectModalPage.pressEnterOnOption('apple');
+        await selectModalPage.ionModalDidDismiss.next();
+        await expect(selectModalPage.modal).not.toBeVisible();
+      });
+
       test('clicking the close button should dismiss the modal', async () => {
         await selectModalPage.setup(config, options, false);
 

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

@@ -63,6 +63,11 @@ export class SelectModalPage {
     await option.press('Space');
   }
 
+  async pressEnterOnOption(value: string) {
+    const option = this.getOption(value);
+    await option.press('Enter');
+  }
+
   private getOption(value: string) {
     const { multiple, selectModal } = this;
     const selector = multiple ? 'ion-checkbox' : 'ion-radio';

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

@@ -22,6 +22,12 @@ import type { SelectPopoverOption } from './select-popover-interface';
 })
 export class SelectPopover implements ComponentInterface {
   @Element() el!: HTMLIonSelectPopoverElement;
+
+  // Tracks the option that received Enter-keydown so keyup only
+  // dismisses when the press started on the same option. Prevents
+  // Enter on the triggering ion-select from auto-dismissing.
+  private pendingEnterTarget: HTMLElement | null = null;
+
   /**
    * The header text of the popover
    */
@@ -159,14 +165,21 @@ export class SelectPopover implements ComponentInterface {
               value={option.value}
               disabled={option.disabled}
               onClick={() => this.dismissParentPopover()}
+              onKeyDown={(ev) => {
+                if (ev.key === 'Enter' && !ev.repeat) {
+                  this.pendingEnterTarget = ev.currentTarget as HTMLElement;
+                }
+              }}
               onKeyUp={(ev) => {
                 if (ev.key === ' ') {
-                  /**
-                   * Selecting a radio option with keyboard navigation,
-                   * either through the Enter or Space keys, should
-                   * dismiss the popover.
-                   */
+                  // Space selects and dismisses in one press.
                   this.dismissParentPopover();
+                } else if (ev.key === 'Enter') {
+                  const shouldDismiss = this.pendingEnterTarget === ev.currentTarget;
+                  this.pendingEnterTarget = null;
+                  if (shouldDismiss) {
+                    this.dismissParentPopover();
+                  }
                 }
               }}
             >

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

@@ -63,6 +63,35 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
         await selectPopoverPage.ionPopoverDidDismiss.next();
         await expect(selectPopoverPage.popover).not.toBeVisible();
       });
+
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      test('pressing Enter on an unselected option should dismiss the popover', async ({ page: _page }, testInfo) => {
+        testInfo.annotations.push({
+          type: 'issue',
+          description: 'https://github.com/ionic-team/ionic-framework/issues/30561',
+        });
+
+        await selectPopoverPage.setup(config, options, false);
+
+        await selectPopoverPage.pressEnterOnOption('apple');
+        await selectPopoverPage.ionPopoverDidDismiss.next();
+        await expect(selectPopoverPage.popover).not.toBeVisible();
+      });
+
+      test('pressing Enter on a selected option should dismiss the popover', async ({ browserName }, testInfo) => {
+        testInfo.annotations.push({
+          type: 'issue',
+          description: 'https://github.com/ionic-team/ionic-framework/issues/30561',
+        });
+
+        test.skip(browserName === 'firefox', 'Same behavior as ROU-5437');
+
+        await selectPopoverPage.setup(config, checkedOptions, false);
+
+        await selectPopoverPage.pressEnterOnOption('apple');
+        await selectPopoverPage.ionPopoverDidDismiss.next();
+        await expect(selectPopoverPage.popover).not.toBeVisible();
+      });
     });
   });
 });

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

@@ -63,6 +63,11 @@ export class SelectPopoverPage {
     await option.press('Space');
   }
 
+  async pressEnterOnOption(value: string) {
+    const option = this.getOption(value);
+    await option.press('Enter');
+  }
+
   private getOption(value: string) {
     const { multiple, selectPopover } = this;
     const selector = multiple ? 'ion-checkbox' : 'ion-radio';