From eaface13b4174fab7117543ab5184d7fc7cd0452 Mon Sep 17 00:00:00 2001 From: haryopba Date: Mon, 11 Aug 2025 13:13:49 +0700 Subject: [PATCH 1/3] fix(select): add Enter keypress to dismiss --- .../components/select-modal/select-modal.tsx | 2 +- .../test/basic/select-modal.e2e.ts | 18 ++++++++++++++++++ .../components/select-modal/test/fixtures.ts | 5 +++++ .../select-popover/select-popover.tsx | 2 +- .../test/basic/select-popover.e2e.ts | 18 ++++++++++++++++++ .../components/select-popover/test/fixtures.ts | 5 +++++ 6 files changed, 48 insertions(+), 2 deletions(-) diff --git a/core/src/components/select-modal/select-modal.tsx b/core/src/components/select-modal/select-modal.tsx index 5925d209640..77e872e86a9 100644 --- a/core/src/components/select-modal/select-modal.tsx +++ b/core/src/components/select-modal/select-modal.tsx @@ -95,7 +95,7 @@ export class SelectModal implements ComponentInterface { labelPlacement="end" onClick={() => this.closeModal()} onKeyUp={(ev) => { - if (ev.key === ' ') { + if (ev.key === 'Enter' || ev.key === ' ') { /** * Selecting a radio option with keyboard navigation, * either through the Enter or Space keys, should diff --git a/core/src/components/select-modal/test/basic/select-modal.e2e.ts b/core/src/components/select-modal/test/basic/select-modal.e2e.ts index 126082712c3..8ea21183d88 100644 --- a/core/src/components/select-modal/test/basic/select-modal.e2e.ts +++ b/core/src/components/select-modal/test/basic/select-modal.e2e.ts @@ -64,6 +64,24 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { await expect(selectModalPage.modal).not.toBeVisible(); }); + test('pressing Enter on an unselected option should dismiss the modal', async () => { + 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 }) => { + 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); diff --git a/core/src/components/select-modal/test/fixtures.ts b/core/src/components/select-modal/test/fixtures.ts index 2058848aa84..e1df642c78b 100644 --- a/core/src/components/select-modal/test/fixtures.ts +++ b/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'; diff --git a/core/src/components/select-popover/select-popover.tsx b/core/src/components/select-popover/select-popover.tsx index 00a8ccf6f2e..cb2e7a1a24d 100644 --- a/core/src/components/select-popover/select-popover.tsx +++ b/core/src/components/select-popover/select-popover.tsx @@ -160,7 +160,7 @@ export class SelectPopover implements ComponentInterface { disabled={option.disabled} onClick={() => this.dismissParentPopover()} onKeyUp={(ev) => { - if (ev.key === ' ') { + if (ev.key === 'Enter' || ev.key === ' ') { /** * Selecting a radio option with keyboard navigation, * either through the Enter or Space keys, should diff --git a/core/src/components/select-popover/test/basic/select-popover.e2e.ts b/core/src/components/select-popover/test/basic/select-popover.e2e.ts index 14cc0c0c146..b333d5a8fca 100644 --- a/core/src/components/select-popover/test/basic/select-popover.e2e.ts +++ b/core/src/components/select-popover/test/basic/select-popover.e2e.ts @@ -63,6 +63,24 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { await selectPopoverPage.ionPopoverDidDismiss.next(); await expect(selectPopoverPage.popover).not.toBeVisible(); }); + + test('pressing Enter on an unselected option should dismiss the popover', async () => { + 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 }) => { + 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(); + }); }); }); }); diff --git a/core/src/components/select-popover/test/fixtures.ts b/core/src/components/select-popover/test/fixtures.ts index 14c81033763..5444a889c04 100644 --- a/core/src/components/select-popover/test/fixtures.ts +++ b/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'; From ce86063912e65dfec65d81fe6170f8386d8fdbdd Mon Sep 17 00:00:00 2001 From: haryopba Date: Mon, 25 Aug 2025 12:41:38 +0700 Subject: [PATCH 2/3] fix(radio-group): add Enter key support for selection --- .../components/radio-group/radio-group.tsx | 4 +- .../radio-group/test/basic/radio-group.e2e.ts | 40 +++++++++++++++++-- .../components/radio-group/test/fixtures.ts | 20 ++++++++-- .../test/search/radio-group.e2e.ts | 2 +- 4 files changed, 56 insertions(+), 10 deletions(-) diff --git a/core/src/components/radio-group/radio-group.tsx b/core/src/components/radio-group/radio-group.tsx index c3e1e4c0b0e..e3c079c73c0 100644 --- a/core/src/components/radio-group/radio-group.tsx +++ b/core/src/components/radio-group/radio-group.tsx @@ -212,8 +212,8 @@ export class RadioGroup implements ComponentInterface { } // Update the radio group value when a user presses the - // space bar on top of a selected radio - if ([' '].includes(ev.key)) { + // enter or space bar on top of a selected radio + if (['Enter', ' '].includes(ev.key)) { const previousValue = this.value; this.value = this.allowEmptySelection && this.value !== undefined ? undefined : current.value; if (previousValue !== this.value || this.allowEmptySelection) { diff --git a/core/src/components/radio-group/test/basic/radio-group.e2e.ts b/core/src/components/radio-group/test/basic/radio-group.e2e.ts index 96b7b5d9556..6ddc2abdd3e 100644 --- a/core/src/components/radio-group/test/basic/radio-group.e2e.ts +++ b/core/src/components/radio-group/test/basic/radio-group.e2e.ts @@ -26,7 +26,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => config ); - await radioFixture.checkRadio('keyboard'); + await radioFixture.checkRadio({ method: 'keyboard', key: 'Space' }); await radioFixture.expectChecked(true); }); @@ -42,7 +42,39 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => config ); - await radioFixture.checkRadio('keyboard'); + await radioFixture.checkRadio({ method: 'keyboard', key: 'Space' }); + await radioFixture.expectChecked(false); + }); + + test('enter should not deselect without allowEmptySelection', async ({ page }) => { + await page.setContent( + ` + + + One + + + `, + config + ); + + await radioFixture.checkRadio({ method: 'keyboard', key: 'Enter' }); + await radioFixture.expectChecked(true); + }); + + test('enter should deselect with allowEmptySelection', async ({ page }) => { + await page.setContent( + ` + + + One + + + `, + config + ); + + await radioFixture.checkRadio({ method: 'keyboard', key: 'Enter' }); await radioFixture.expectChecked(false); }); @@ -58,7 +90,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => config ); - await radioFixture.checkRadio('mouse'); + await radioFixture.checkRadio({ method: 'mouse' }); await radioFixture.expectChecked(true); }); @@ -74,7 +106,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => config ); - await radioFixture.checkRadio('mouse'); + await radioFixture.checkRadio({ method: 'mouse' }); await radioFixture.expectChecked(false); }); diff --git a/core/src/components/radio-group/test/fixtures.ts b/core/src/components/radio-group/test/fixtures.ts index 21722126d7f..b1002a4971f 100644 --- a/core/src/components/radio-group/test/fixtures.ts +++ b/core/src/components/radio-group/test/fixtures.ts @@ -2,6 +2,19 @@ import type { Locator } from '@playwright/test'; import { expect } from '@playwright/test'; import type { E2EPage } from '@utils/test/playwright'; +interface CheckRadioKeyboardOptions { + method: 'keyboard'; + key: 'Space' | 'Enter'; + selector?: string; +} + +interface CheckRadioMouseOptions { + method: 'mouse'; + selector?: string; +} + +type CheckRadioOptions = CheckRadioKeyboardOptions | CheckRadioMouseOptions; + export class RadioFixture { readonly page: E2EPage; @@ -11,13 +24,14 @@ export class RadioFixture { this.page = page; } - async checkRadio(method: 'keyboard' | 'mouse', selector = 'ion-radio') { + async checkRadio(options: CheckRadioOptions) { const { page } = this; + const selector = options.selector ?? 'ion-radio'; const radio = (this.radio = page.locator(selector)); - if (method === 'keyboard') { + if (options.method === 'keyboard') { await radio.focus(); - await page.keyboard.press('Space'); + await page.keyboard.press(options.key); } else { await radio.click(); } diff --git a/core/src/components/radio-group/test/search/radio-group.e2e.ts b/core/src/components/radio-group/test/search/radio-group.e2e.ts index 121acd4cd6a..4af2b46450e 100644 --- a/core/src/components/radio-group/test/search/radio-group.e2e.ts +++ b/core/src/components/radio-group/test/search/radio-group.e2e.ts @@ -17,7 +17,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => const searchbarInput = page.locator('ion-searchbar input'); // select radio - const radio = await radioFixture.checkRadio('mouse', 'ion-radio[value=two]'); + const radio = await radioFixture.checkRadio({ method: 'mouse', selector: 'ion-radio[value=two]' }); await radioFixture.expectChecked(true); // filter radio so it is not in DOM From 4b11454ac740eb8919ed74ae22977279e94b14ab Mon Sep 17 00:00:00 2001 From: haryopba Date: Mon, 25 Aug 2025 12:44:27 +0700 Subject: [PATCH 3/3] fix(select): prevent modal form closing immediately when opened with Enter --- core/src/components/select-modal/select-modal.tsx | 14 +++++++++++--- .../components/select-popover/select-popover.tsx | 14 +++++++++++--- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/core/src/components/select-modal/select-modal.tsx b/core/src/components/select-modal/select-modal.tsx index 77e872e86a9..c718dd5d629 100644 --- a/core/src/components/select-modal/select-modal.tsx +++ b/core/src/components/select-modal/select-modal.tsx @@ -94,12 +94,20 @@ export class SelectModal implements ComponentInterface { justify="start" labelPlacement="end" onClick={() => this.closeModal()} + onKeyDown={(ev) => { + if (ev.key === 'Enter') { + /** + * Selecting a radio option with keyboard navigation, + * Enter key should dismiss the modal. + */ + this.closeModal(); + } + }} onKeyUp={(ev) => { - if (ev.key === 'Enter' || ev.key === ' ') { + if (ev.key === ' ') { /** * Selecting a radio option with keyboard navigation, - * either through the Enter or Space keys, should - * dismiss the modal. + * Space key should dismiss the modal. */ this.closeModal(); } diff --git a/core/src/components/select-popover/select-popover.tsx b/core/src/components/select-popover/select-popover.tsx index cb2e7a1a24d..0ad36ee1ebd 100644 --- a/core/src/components/select-popover/select-popover.tsx +++ b/core/src/components/select-popover/select-popover.tsx @@ -159,12 +159,20 @@ export class SelectPopover implements ComponentInterface { value={option.value} disabled={option.disabled} onClick={() => this.dismissParentPopover()} + onKeyDown={(ev) => { + if (ev.key === 'Enter') { + /** + * Selecting a radio option with keyboard navigation, + * Enter key should dismiss the popover. + */ + this.dismissParentPopover(); + } + }} onKeyUp={(ev) => { - if (ev.key === 'Enter' || ev.key === ' ') { + if (ev.key === ' ') { /** * Selecting a radio option with keyboard navigation, - * either through the Enter or Space keys, should - * dismiss the popover. + * Space key should dismiss the popover. */ this.dismissParentPopover(); }