mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2025-11-09 08:09:32 +08:00
fix(radio): radios can be focused and are announced with group (#27817)
Issue number: resolves #27438 --------- <!-- 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. --> There are a few issues with the modern radio syntax: 1. The native radio is inside the Shadow DOM. As a result, radios are not announced with their parent group with screen readers (i.e. "1 of 3") 2. The native radio cannot be focused inside of `ion-select-popover` on Firefox. 3. The `ionFocus` and `ionBlur` events do not fire. I also discovered an issue with item: 1. Items inside of a Radio Group have a role of `listitem` which prevent radios from being grouped correctly in some browsers. According to https://bugzilla.mozilla.org/show_bug.cgi?id=1840916, browsers are behaving correctly here. The `listitem` role should not be present when an item is used in a radio group (even if the radio group itself is inside a list). ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> Most of the changes are test-related, but I broke it down per commit to make this easier to review:ae77002afd- Item no longer has `role="listitem"` when used inside of a radio group. - Added spec tests to verify the role behavior0a9b7fb91d- I discovered that some the legacy basic test were accidentally using the modern syntax. I corrected this by adding `legacy="true"` to the radios.a8a90e53b2,412d1d54e7, and1d1179b69a- The current radio group tests only tested the legacy radio syntax, and not the modern syntax. - I created a `legacy` directory to house the legacy syntax tests. - I created new tests in the root test directory for the modern syntax. - I also deleted the screenshots for the modern tests here because the tests for `ion-radio` already take screenshots of the radio (even in an item).e2c966e68b- Moved radio roles to the host. This allows Firefox to focus radios and for screen readers to announce the radios as part of a group. - I also added focus/blur listeners so ionFocus and ionBlur firef10eff47a5- I cleaned up the tests here to use a common radio fixture ## 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. --> I tested this with the following setups. ✅ indicates the screen reader announces the group count (i.e. "1 of 4"). ❌ indicates the screen reader does not announce the group count. **Radio in Radio Group:** - iOS + VoiceOver: ✅ - Android + TalkBack: ✅ - macOS + VoiceOver + Safari: ✅ - macOS + VoiceOver + Firefox: ✅ - macOS + VoiceOver + Chrome: ✅ - Windows + NVDA + Chrome: ✅ - Windows + NVDA + Firefox: ✅ **Radio in Item in Radio Group :** - iOS + VoiceOver: ✅ - Android + TalkBack: ❌ (https://bugs.chromium.org/p/chromium/issues/detail?id=1459006) - macOS + VoiceOver + Safari: ✅ - macOS + VoiceOver + Firefox: ✅ - macOS + VoiceOver + Chrome: ❌ (https://bugs.chromium.org/p/chromium/issues/detail?id=1459003) - Windows + NVDA + Chrome: ✅ - Windows + NVDA + Firefox: ✅
This commit is contained in:
@ -2,8 +2,7 @@ import type { ComponentInterface, EventEmitter } from '@stencil/core';
|
||||
import { Component, Element, Event, Host, Method, Prop, State, Watch, h } from '@stencil/core';
|
||||
import type { LegacyFormController } from '@utils/forms';
|
||||
import { createLegacyFormController } from '@utils/forms';
|
||||
import type { Attributes } from '@utils/helpers';
|
||||
import { addEventListener, getAriaLabel, inheritAriaAttributes, removeEventListener } from '@utils/helpers';
|
||||
import { addEventListener, getAriaLabel, removeEventListener } from '@utils/helpers';
|
||||
import { printIonWarning } from '@utils/logging';
|
||||
import { createColorClasses, hostContext } from '@utils/theme';
|
||||
|
||||
@ -31,7 +30,6 @@ export class Radio implements ComponentInterface {
|
||||
private radioGroup: HTMLIonRadioGroupElement | null = null;
|
||||
private nativeInput!: HTMLInputElement;
|
||||
private legacyFormController!: LegacyFormController;
|
||||
private inheritedAttributes: Attributes = {};
|
||||
|
||||
// This flag ensures we log the deprecation warning at most once.
|
||||
private hasLoggedDeprecationWarning = false;
|
||||
@ -135,8 +133,7 @@ export class Radio implements ComponentInterface {
|
||||
ev.stopPropagation();
|
||||
ev.preventDefault();
|
||||
|
||||
const element = this.legacyFormController.hasLegacyControl() ? this.el : this.nativeInput;
|
||||
element.focus();
|
||||
this.el.focus();
|
||||
}
|
||||
|
||||
/** @internal */
|
||||
@ -167,12 +164,6 @@ export class Radio implements ComponentInterface {
|
||||
|
||||
componentWillLoad() {
|
||||
this.emitStyle();
|
||||
|
||||
if (!this.legacyFormController.hasLegacyControl()) {
|
||||
this.inheritedAttributes = {
|
||||
...inheritAriaAttributes(this.el),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@Watch('checked')
|
||||
@ -201,7 +192,34 @@ export class Radio implements ComponentInterface {
|
||||
};
|
||||
|
||||
private onClick = () => {
|
||||
this.checked = this.nativeInput.checked;
|
||||
const { radioGroup, checked } = this;
|
||||
|
||||
/**
|
||||
* The legacy control uses a native input inside
|
||||
* of the radio host, so we can set this.checked
|
||||
* to the state of the nativeInput. RadioGroup
|
||||
* will prevent the native input from checking if
|
||||
* allowEmptySelection="false" by calling ev.preventDefault().
|
||||
*/
|
||||
if (this.legacyFormController.hasLegacyControl()) {
|
||||
this.checked = this.nativeInput.checked;
|
||||
return;
|
||||
}
|
||||
|
||||
/**
|
||||
* The modern control does not use a native input
|
||||
* inside of the radio host, so we cannot rely on the
|
||||
* ev.preventDefault() behavior above. If the radio
|
||||
* is checked and the parent radio group allows for empty
|
||||
* selection, then we can set the checked state to false.
|
||||
* Otherwise, the checked state should always be set
|
||||
* to true because the checked state cannot be toggled.
|
||||
*/
|
||||
if (checked && radioGroup?.allowEmptySelection) {
|
||||
this.checked = false;
|
||||
} else {
|
||||
this.checked = true;
|
||||
}
|
||||
};
|
||||
|
||||
private onFocus = () => {
|
||||
@ -232,23 +250,14 @@ export class Radio implements ComponentInterface {
|
||||
}
|
||||
|
||||
private renderRadio() {
|
||||
const {
|
||||
checked,
|
||||
disabled,
|
||||
inputId,
|
||||
color,
|
||||
el,
|
||||
justify,
|
||||
labelPlacement,
|
||||
inheritedAttributes,
|
||||
hasLabel,
|
||||
buttonTabindex,
|
||||
} = this;
|
||||
const { checked, disabled, color, el, justify, labelPlacement, hasLabel, buttonTabindex } = this;
|
||||
const mode = getIonMode(this);
|
||||
const inItem = hostContext('ion-item', el);
|
||||
|
||||
return (
|
||||
<Host
|
||||
onFocus={this.onFocus}
|
||||
onBlur={this.onBlur}
|
||||
onClick={this.onClick}
|
||||
class={createColorClasses(color, {
|
||||
[mode]: true,
|
||||
@ -261,21 +270,12 @@ export class Radio implements ComponentInterface {
|
||||
'ion-activatable': !inItem,
|
||||
'ion-focusable': !inItem,
|
||||
})}
|
||||
role="radio"
|
||||
aria-checked={checked ? 'true' : 'false'}
|
||||
aria-disabled={disabled ? 'true' : null}
|
||||
tabindex={buttonTabindex}
|
||||
>
|
||||
<label class="radio-wrapper">
|
||||
{/*
|
||||
The native control must be rendered
|
||||
before the visible label text due to https://bugs.webkit.org/show_bug.cgi?id=251951
|
||||
*/}
|
||||
<input
|
||||
type="radio"
|
||||
checked={checked}
|
||||
disabled={disabled}
|
||||
id={inputId}
|
||||
tabindex={buttonTabindex}
|
||||
ref={(nativeEl) => (this.nativeInput = nativeEl as HTMLInputElement)}
|
||||
{...inheritedAttributes}
|
||||
/>
|
||||
<div
|
||||
class={{
|
||||
'label-text-wrapper': true,
|
||||
|
||||
@ -16,10 +16,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
|
||||
});
|
||||
|
||||
test.describe(title('radio: keyboard navigation'), () => {
|
||||
test.beforeEach(async ({ page, skip }) => {
|
||||
// TODO (FW-2979)
|
||||
skip.browser('webkit', 'Safari 16 only allows text fields and pop-up menus to be focused.');
|
||||
|
||||
test.beforeEach(async ({ page }) => {
|
||||
await page.setContent(
|
||||
`
|
||||
<ion-app>
|
||||
|
||||
@ -95,7 +95,7 @@ configs().forEach(({ title, screenshot, config }) => {
|
||||
test('should apply color correctly', async ({ page }) => {
|
||||
await page.setContent(
|
||||
`
|
||||
<ion-radio slot="start" value="pepperoni" color="success"></ion-radio>
|
||||
<ion-radio legacy="true" value="pepperoni" color="success"></ion-radio>
|
||||
`,
|
||||
config
|
||||
);
|
||||
@ -138,7 +138,7 @@ configs({ directions: ['ltr'] }).forEach(({ title, config }) => {
|
||||
test('radio should be checked when activated even without a radio group', async ({ page }) => {
|
||||
await page.setContent(
|
||||
`
|
||||
<ion-radio slot="start" value="pepperoni"></ion-radio>
|
||||
<ion-radio legacy="true" value="pepperoni"></ion-radio>
|
||||
`,
|
||||
config
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user