fix(select): do not focus disabled popover option (#28309)

Issue number: resolves #28284

---------

<!-- 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. -->

Select focuses the first popover option when no value is provided. This
means that the first option is focused even if it disabled.

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

- Select focuses the first **enabled** popover option when no value is
provided.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!-- If this introduces a breaking change, please describe the impact
and migration path for existing applications below. -->


## Other information

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

---------

Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
This commit is contained in:
Liam DeBeasi
2023-10-10 15:02:50 -04:00
committed by GitHub
parent d0057352fe
commit eee2115fd2
2 changed files with 72 additions and 19 deletions

View File

@ -316,29 +316,46 @@ export class Select implements ComponentInterface {
// focus selected option for popovers // focus selected option for popovers
if (this.interface === 'popover') { if (this.interface === 'popover') {
let indexOfSelected = this.childOpts.map((o) => o.value).indexOf(this.value); const indexOfSelected = this.childOpts.map((o) => o.value).indexOf(this.value);
indexOfSelected = indexOfSelected > -1 ? indexOfSelected : 0; // default to first option if nothing selected
const selectedItem = overlay.querySelector<HTMLElement>(
`.select-interface-option:nth-child(${indexOfSelected + 1})`
);
if (selectedItem) { if (indexOfSelected > -1) {
focusElement(selectedItem); const selectedItem = overlay.querySelector<HTMLElement>(
`.select-interface-option:nth-child(${indexOfSelected + 1})`
);
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<HTMLElement>('ion-radio, ion-checkbox');
if (interactiveEl) {
interactiveEl.focus();
}
}
} else {
/** /**
* Browsers such as Firefox do not * If no value is set then focus the first enabled option.
* 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<HTMLElement>('ion-radio, ion-checkbox'); const firstEnabledOption = overlay.querySelector<HTMLElement>(
if (interactiveEl) { 'ion-radio:not(.radio-disabled), ion-checkbox:not(.checkbox-disabled)'
interactiveEl.focus(); );
if (firstEnabledOption) {
focusElement(firstEnabledOption.closest('ion-item')!);
/**
* Focus the option for the same reason as we do above.
*/
firstEnabledOption.focus();
} }
} }
} }

View File

@ -0,0 +1,36 @@
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
test.describe(title('select: disabled options'), () => {
test('should not focus a disabled option when no value is set', async ({ page, skip }) => {
// TODO (FW-2979)
skip.browser('webkit', 'Safari 16 only allows text fields and pop-up menus to be focused.');
test.info().annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/28284',
});
await page.setContent(
`
<ion-select interface="popover">
<ion-select-option value="a" disabled="true">A</ion-select-option>
<ion-select-option value="b">B</ion-select-option>
</ion-select>
`,
config
);
const select = page.locator('ion-select');
const popover = page.locator('ion-popover');
const ionPopoverDidPresent = await page.spyOnEvent('ionPopoverDidPresent');
await select.click();
await ionPopoverDidPresent.next();
const popoverOption = popover.locator('.select-interface-option:nth-of-type(2) ion-radio');
await expect(popoverOption).toBeFocused();
});
});
});