From 26a8bc623091ad93411d1124e417ae7ad191052f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pau=20Ferrer=20Oca=C3=B1a?= Date: Thu, 12 Jun 2025 16:57:49 +0200 Subject: [PATCH 1/3] fix(select): focus the correct selected item See https://github.com/ionic-team/ionic-framework/issues/30480 --- core/src/components/select/select.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/components/select/select.tsx b/core/src/components/select/select.tsx index 986e025062d..7df8049d13a 100644 --- a/core/src/components/select/select.tsx +++ b/core/src/components/select/select.tsx @@ -349,7 +349,7 @@ export class Select implements ComponentInterface { const indexOfSelected = this.childOpts.findIndex((o) => o.value === this.value); if (indexOfSelected > -1) { const selectedItem = overlay.querySelector( - `.select-interface-option:nth-child(${indexOfSelected + 1})` + `.select-interface-option:nth-of-type(${indexOfSelected + 1})` ); if (selectedItem) { From 11dbf56c8ab3fe56074f510d193c88e9af834ab4 Mon Sep 17 00:00:00 2001 From: Brandy Smith <6577830+brandyscarney@users.noreply.github.com> Date: Mon, 30 Jun 2025 17:00:05 -0400 Subject: [PATCH 2/3] test(select): add interface tests for the focused class --- .../select/test/basic/select.e2e.ts | 224 ++++++++++++++++++ 1 file changed, 224 insertions(+) diff --git a/core/src/components/select/test/basic/select.e2e.ts b/core/src/components/select/test/basic/select.e2e.ts index 8be07e84d68..e90f7a2b220 100644 --- a/core/src/components/select/test/basic/select.e2e.ts +++ b/core/src/components/select/test/basic/select.e2e.ts @@ -34,6 +34,66 @@ configs({ directions: ['ltr'] }).forEach(({ title, config, screenshot }) => { const alert = page.locator('ion-alert'); await expect(alert).toHaveScreenshot(screenshot(`select-basic-alert-scroll-to-selected`)); }); + + test('it should not focus any option when opened with no value', async ({ page }) => { + // ion-app is required to apply the focused styles + await page.setContent( + ` + + + Apples + Bananas + Oranges + + + `, + config + ); + + const select = page.locator('ion-select'); + const ionAlertDidPresent = await page.spyOnEvent('ionAlertDidPresent'); + + await select.click(); + await ionAlertDidPresent.next(); + + await page.waitForChanges(); + + const alert = page.locator('ion-alert'); + + // Verify that no option has the ion-focused class + const focusedOptions = alert.locator('.alert-radio-button.ion-focused'); + await expect(focusedOptions).toHaveCount(0); + }); + + test('it should not focus any option when opened with a value', async ({ page }) => { + // ion-app is required to apply the focused styles + await page.setContent( + ` + + + Apples + Bananas + Oranges + + + `, + config + ); + + const select = page.locator('ion-select'); + const ionAlertDidPresent = await page.spyOnEvent('ionAlertDidPresent'); + + await select.click(); + await ionAlertDidPresent.next(); + + await page.waitForChanges(); + + const alert = page.locator('ion-alert'); + + // Alert interface doesn't apply ion-focused class to selected options + const focusedOptions = alert.locator('.alert-radio-button.ion-focused'); + await expect(focusedOptions).toHaveCount(0); + }); }); test.describe('select: action sheet', () => { @@ -56,6 +116,107 @@ configs({ directions: ['ltr'] }).forEach(({ title, config, screenshot }) => { const actionSheet = page.locator('ion-action-sheet'); await expect(actionSheet).toHaveScreenshot(screenshot(`select-basic-action-sheet-scroll-to-selected`)); }); + + test('it should not focus any option when opened with no value', async ({ page }) => { + // ion-app is required to apply the focused styles + await page.setContent( + ` + + + Apples + Bananas + Oranges + + + `, + config + ); + + const select = page.locator('ion-select'); + const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent'); + + await select.click(); + await ionActionSheetDidPresent.next(); + + await page.waitForChanges(); + + const actionSheet = page.locator('ion-action-sheet'); + + // Verify that none of the options have the ion-focused class + const focusedOptions = actionSheet.locator('.action-sheet-button.ion-focused'); + await expect(focusedOptions).toHaveCount(0); + }); + + test('it should focus the second option when opened', async ({ page }) => { + // ion-app is required to apply the focused styles + await page.setContent( + ` + + + Apples + Bananas + Oranges + + + `, + config + ); + + const select = page.locator('ion-select'); + const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent'); + + await select.click(); + await ionActionSheetDidPresent.next(); + + await page.waitForChanges(); + + const actionSheet = page.locator('ion-action-sheet'); + + // Find the button containing "Bananas" and verify it has the ion-focused class + const bananasOption = actionSheet.locator('.action-sheet-button:has-text("Bananas")'); + await expect(bananasOption).toHaveClass(/ion-focused/); + }); + + test('it should focus the second option when opened with a header', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/30480', + }); + + // ion-app is required to apply the focused styles + await page.setContent( + ` + + + Apples + Bananas + Oranges + + + `, + config + ); + + const select = page.locator('ion-select'); + await select.evaluate((el: HTMLIonSelectElement) => { + el.interfaceOptions = { + header: 'Header', + }; + }); + + const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent'); + + await select.click(); + await ionActionSheetDidPresent.next(); + + await page.waitForChanges(); + + const actionSheet = page.locator('ion-action-sheet'); + + // Find the option containing "Bananas" and verify it has the ion-focused class + const bananasOption = actionSheet.locator('.action-sheet-button:has-text("Bananas")'); + await expect(bananasOption).toHaveClass(/ion-focused/); + }); }); test.describe('select: popover', () => { @@ -78,6 +239,39 @@ configs({ directions: ['ltr'] }).forEach(({ title, config, screenshot }) => { await expect(popover).toBeVisible(); }); + test('it should focus the second option when value is set', async ({ page, skip }) => { + // TODO (ROU-5437) + skip.browser('webkit', 'Safari 16 only allows text fields and pop-up menus to be focused.'); + + // ion-app is required to apply the focused styles + await page.setContent( + ` + + + Apples + Bananas + Oranges + + + `, + config + ); + + const select = page.locator('ion-select'); + const ionPopoverDidPresent = await page.spyOnEvent('ionPopoverDidPresent'); + + await select.click(); + await ionPopoverDidPresent.next(); + + await page.waitForChanges(); + + const popover = page.locator('ion-popover'); + + // Find the option containing "Bananas" and verify it has the ion-focused class + const bananasOption = popover.locator('.select-interface-option:has-text("Bananas")'); + await expect(bananasOption).toHaveClass(/ion-focused/); + }); + test('it should scroll to selected option when opened', async ({ page }) => { const ionPopoverDidPresent = await page.spyOnEvent('ionPopoverDidPresent'); @@ -106,6 +300,36 @@ configs({ directions: ['ltr'] }).forEach(({ title, config, screenshot }) => { await expect(modal).toBeVisible(); }); + test('it should focus the second option when value is set', async ({ page }) => { + // ion-app is required to apply the focused styles + await page.setContent( + ` + + + Apples + Bananas + Oranges + + + `, + config + ); + + const select = page.locator('ion-select'); + const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); + + await select.click(); + await ionModalDidPresent.next(); + + await page.waitForChanges(); + + const modal = page.locator('ion-modal'); + + // Find the option containing "Bananas" and verify it has the ion-focused class + const bananasOption = modal.locator('.select-interface-option:has-text("Bananas")'); + await expect(bananasOption).toHaveClass(/ion-focused/); + }); + test('it should scroll to selected option when opened', async ({ page }) => { const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); From a044f83998aa9bdb18f820d56b1891ade9a82c8f Mon Sep 17 00:00:00 2001 From: Brandy Smith <6577830+brandyscarney@users.noreply.github.com> Date: Mon, 30 Jun 2025 17:08:21 -0400 Subject: [PATCH 3/3] style: test name consistency --- core/src/components/select/test/basic/select.e2e.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/components/select/test/basic/select.e2e.ts b/core/src/components/select/test/basic/select.e2e.ts index e90f7a2b220..bf23678e657 100644 --- a/core/src/components/select/test/basic/select.e2e.ts +++ b/core/src/components/select/test/basic/select.e2e.ts @@ -147,7 +147,7 @@ configs({ directions: ['ltr'] }).forEach(({ title, config, screenshot }) => { await expect(focusedOptions).toHaveCount(0); }); - test('it should focus the second option when opened', async ({ page }) => { + test('it should focus the second option when opened with a value', async ({ page }) => { // ion-app is required to apply the focused styles await page.setContent( ` @@ -177,7 +177,7 @@ configs({ directions: ['ltr'] }).forEach(({ title, config, screenshot }) => { await expect(bananasOption).toHaveClass(/ion-focused/); }); - test('it should focus the second option when opened with a header', async ({ page }) => { + test('it should focus the second option when opened with a value and a header', async ({ page }) => { test.info().annotations.push({ type: 'issue', description: 'https://github.com/ionic-team/ionic-framework/issues/30480', @@ -239,7 +239,7 @@ configs({ directions: ['ltr'] }).forEach(({ title, config, screenshot }) => { await expect(popover).toBeVisible(); }); - test('it should focus the second option when value is set', async ({ page, skip }) => { + test('it should focus the second option when opened with a value', async ({ page, skip }) => { // TODO (ROU-5437) skip.browser('webkit', 'Safari 16 only allows text fields and pop-up menus to be focused.'); @@ -300,7 +300,7 @@ configs({ directions: ['ltr'] }).forEach(({ title, config, screenshot }) => { await expect(modal).toBeVisible(); }); - test('it should focus the second option when value is set', async ({ page }) => { + test('it should focus the second option when opened with a value', async ({ page }) => { // ion-app is required to apply the focused styles await page.setContent( `