From 946807d67b972c41b52c23c8f00feca4c705b224 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 24 Jan 2023 14:17:04 -0500 Subject: [PATCH] fix(select): focusing item works in firefox (#26668) --- core/src/components/select/select.tsx | 23 ++++++++++++++++--- .../select/test/basic/select.e2e.ts | 11 ++++----- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/core/src/components/select/select.tsx b/core/src/components/select/select.tsx index 761cae7e20..5bf8699c19 100644 --- a/core/src/components/select/select.tsx +++ b/core/src/components/select/select.tsx @@ -206,11 +206,28 @@ export class Select implements ComponentInterface { if (this.interface === 'popover') { let indexOfSelected = this.childOpts.map((o) => o.value).indexOf(this.value); indexOfSelected = indexOfSelected > -1 ? indexOfSelected : 0; // default to first option if nothing selected - const selectedEl = overlay.querySelector( + const selectedItem = overlay.querySelector( `.select-interface-option:nth-child(${indexOfSelected + 1})` ); - if (selectedEl) { - focusElement(selectedEl); + + if (selectedItem) { + focusElement(selectedItem); + + /** + * Browsers such as Firefox do not + * correctly delegate focus when manually + * focusing an element with delegatesFocus. + * We work around this by manually focusing + * the interactive element. + * ion-radio and ion-checkbox are the only + * elements that ion-select-popover uses, so + * we only need to worry about those two components + * when focusing. + */ + const interactiveEl = selectedItem.querySelector('ion-radio, ion-checkbox'); + if (interactiveEl) { + interactiveEl.focus(); + } } } diff --git a/core/src/components/select/test/basic/select.e2e.ts b/core/src/components/select/test/basic/select.e2e.ts index 5f9b3545c7..2fdec55473 100644 --- a/core/src/components/select/test/basic/select.e2e.ts +++ b/core/src/components/select/test/basic/select.e2e.ts @@ -72,7 +72,7 @@ test.describe('select: basic', () => { }); test.describe('select: popover', () => { - test('it should open a popover select', async ({ page, browserName, skip }) => { + test('it should open a popover select', async ({ page, skip }) => { // TODO (FW-2979) skip.browser('webkit', 'Safari 16 only allows text fields and pop-up menus to be focused.'); @@ -85,12 +85,9 @@ test.describe('select: basic', () => { const popover = page.locator('ion-popover'); - // TODO(FW-1436) - if (browserName !== 'firefox') { - // select has no value, so first option should be focused by default - const popoverOption1 = await popover.locator('.select-interface-option:first-of-type ion-radio'); - await expect(popoverOption1).toBeFocused(); - } + // select has no value, so first option should be focused by default + const popoverOption1 = popover.locator('.select-interface-option:first-of-type ion-radio'); + await expect(popoverOption1).toBeFocused(); expect(await page.screenshot({ animations: 'disabled' })).toMatchSnapshot( `select-popover-diff-${page.getSnapshotSettings()}.png`