mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2026-03-13 10:22:08 +08:00
fix(select, action-sheet): use radio role for options (#30769)
Issue number: internal --------- <!-- 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. --> The screen reader does not announce when an option is selected within the action sheet interface. This is because the action sheet uses standard buttons, which do not support a detectable selected state via native properties or ARIA attributes like `aria-checked` or `aria-selected`, creating an inconsistent user experience across different interface types. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Updated the action sheet buttons to accept `role="radio"` - Added keyboard navigation to follow the pattern for radio group - Added test ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> [Basic](https://ionic-framework-git-fw-6818-ionic1.vercel.app/src/components/select/test/basic/) --------- Co-authored-by: Brandy Smith <6577830+brandyscarney@users.noreply.github.com> Co-authored-by: Shane <shane@shanessite.net> Co-authored-by: Brandy Smith <brandyscarney@users.noreply.github.com>
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import type { ComponentInterface, EventEmitter } from '@stencil/core';
|
||||
import { Watch, Component, Element, Event, Host, Method, Prop, h, readTask } from '@stencil/core';
|
||||
import { Watch, Component, Element, Event, Host, Listen, Method, Prop, State, h, readTask } from '@stencil/core';
|
||||
import type { Gesture } from '@utils/gesture';
|
||||
import { createButtonActiveGesture } from '@utils/gesture/button-active';
|
||||
import { raf } from '@utils/helpers';
|
||||
@@ -46,11 +46,18 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
|
||||
private wrapperEl?: HTMLElement;
|
||||
private groupEl?: HTMLElement;
|
||||
private gesture?: Gesture;
|
||||
private hasRadioButtons = false;
|
||||
|
||||
presented = false;
|
||||
lastFocus?: HTMLElement;
|
||||
animation?: any;
|
||||
|
||||
/**
|
||||
* The ID of the currently active/selected radio button.
|
||||
* Used for keyboard navigation and ARIA attributes.
|
||||
*/
|
||||
@State() activeRadioId?: string;
|
||||
|
||||
@Element() el!: HTMLIonActionSheetElement;
|
||||
|
||||
/** @internal */
|
||||
@@ -81,6 +88,22 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
|
||||
* An array of buttons for the action sheet.
|
||||
*/
|
||||
@Prop() buttons: (ActionSheetButton | string)[] = [];
|
||||
@Watch('buttons')
|
||||
buttonsChanged() {
|
||||
const radioButtons = this.getRadioButtons();
|
||||
this.hasRadioButtons = radioButtons.length > 0;
|
||||
|
||||
// Initialize activeRadioId when buttons change
|
||||
if (this.hasRadioButtons) {
|
||||
const checkedButton = radioButtons.find((b) => b.htmlAttributes?.['aria-checked'] === 'true');
|
||||
|
||||
if (checkedButton) {
|
||||
const allButtons = this.getButtons();
|
||||
const checkedIndex = allButtons.indexOf(checkedButton);
|
||||
this.activeRadioId = this.getButtonId(checkedButton, checkedIndex);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Additional classes to apply for custom CSS. If multiple classes are
|
||||
@@ -277,12 +300,53 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all buttons regardless of role.
|
||||
*/
|
||||
private getButtons(): ActionSheetButton[] {
|
||||
return this.buttons.map((b) => {
|
||||
return typeof b === 'string' ? { text: b } : b;
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all radio buttons (buttons with role="radio").
|
||||
*/
|
||||
private getRadioButtons(): ActionSheetButton[] {
|
||||
return this.getButtons().filter((b) => {
|
||||
const role = b.htmlAttributes?.role;
|
||||
return role === 'radio' && !isCancel(role);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Handle radio button selection and update aria-checked state.
|
||||
*
|
||||
* @param button The radio button that was selected.
|
||||
*/
|
||||
private selectRadioButton(button: ActionSheetButton) {
|
||||
const buttonId = this.getButtonId(button);
|
||||
|
||||
// Set the active radio ID (this will trigger a re-render and update aria-checked)
|
||||
this.activeRadioId = buttonId;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get or generate an ID for a button.
|
||||
*
|
||||
* @param button The button for which to get the ID.
|
||||
* @param index Optional index of the button in the buttons array.
|
||||
* @returns The ID of the button.
|
||||
*/
|
||||
private getButtonId(button: ActionSheetButton, index?: number): string {
|
||||
if (button.id) {
|
||||
return button.id;
|
||||
}
|
||||
const allButtons = this.getButtons();
|
||||
const buttonIndex = index !== undefined ? index : allButtons.indexOf(button);
|
||||
return `action-sheet-button-${this.overlayIndex}-${buttonIndex}`;
|
||||
}
|
||||
|
||||
private onBackdropTap = () => {
|
||||
this.dismiss(undefined, BACKDROP);
|
||||
};
|
||||
@@ -295,6 +359,96 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* When the action sheet has radio buttons, we want to follow the
|
||||
* keyboard navigation pattern for radio groups:
|
||||
* - Arrow Down/Right: Move to the next radio button (wrap to first if at end)
|
||||
* - Arrow Up/Left: Move to the previous radio button (wrap to last if at start)
|
||||
* - Space/Enter: Select the focused radio button and trigger its handler
|
||||
*/
|
||||
@Listen('keydown')
|
||||
onKeydown(ev: KeyboardEvent) {
|
||||
// Only handle keyboard navigation if we have radio buttons
|
||||
if (!this.hasRadioButtons || !this.presented) {
|
||||
return;
|
||||
}
|
||||
|
||||
const target = ev.target as HTMLElement;
|
||||
|
||||
// Ignore if the target element is not within the action sheet or not a radio button
|
||||
if (
|
||||
!this.el.contains(target) ||
|
||||
!target.classList.contains('action-sheet-button') ||
|
||||
target.getAttribute('role') !== 'radio'
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Get all radio button elements and filter out disabled ones
|
||||
const radios = Array.from(this.el.querySelectorAll('.action-sheet-button[role="radio"]')).filter(
|
||||
(el) => !(el as HTMLButtonElement).disabled
|
||||
) as HTMLButtonElement[];
|
||||
const currentIndex = radios.findIndex((radio) => radio.id === target.id);
|
||||
|
||||
if (currentIndex === -1) {
|
||||
return;
|
||||
}
|
||||
|
||||
const allButtons = this.getButtons();
|
||||
const radioButtons = this.getRadioButtons();
|
||||
/**
|
||||
* Build a map of button element IDs to their ActionSheetButton
|
||||
* config objects.
|
||||
* This allows us to quickly look up which button config corresponds
|
||||
* to a DOM element when handling keyboard navigation
|
||||
* (e.g., whenuser presses Space/Enter or arrow keys).
|
||||
* The key is the ID that was set on the DOM element during render,
|
||||
* and the value is the ActionSheetButton config that contains the
|
||||
* handler and other properties.
|
||||
*/
|
||||
const buttonIdMap = new Map<string, ActionSheetButton>();
|
||||
|
||||
radioButtons.forEach((b) => {
|
||||
const allIndex = allButtons.indexOf(b);
|
||||
const buttonId = this.getButtonId(b, allIndex);
|
||||
buttonIdMap.set(buttonId, b);
|
||||
});
|
||||
|
||||
let nextEl: HTMLButtonElement | undefined;
|
||||
|
||||
if (['ArrowDown', 'ArrowRight'].includes(ev.key)) {
|
||||
ev.preventDefault();
|
||||
ev.stopPropagation();
|
||||
|
||||
nextEl = currentIndex === radios.length - 1 ? radios[0] : radios[currentIndex + 1];
|
||||
} else if (['ArrowUp', 'ArrowLeft'].includes(ev.key)) {
|
||||
ev.preventDefault();
|
||||
ev.stopPropagation();
|
||||
|
||||
nextEl = currentIndex === 0 ? radios[radios.length - 1] : radios[currentIndex - 1];
|
||||
} else if (ev.key === ' ' || ev.key === 'Enter') {
|
||||
ev.preventDefault();
|
||||
ev.stopPropagation();
|
||||
|
||||
const button = buttonIdMap.get(target.id);
|
||||
if (button) {
|
||||
this.selectRadioButton(button);
|
||||
this.buttonClick(button);
|
||||
}
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
// Focus the next radio button
|
||||
if (nextEl) {
|
||||
const button = buttonIdMap.get(nextEl.id);
|
||||
if (button) {
|
||||
this.selectRadioButton(button);
|
||||
nextEl.focus();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
connectedCallback() {
|
||||
prepareOverlay(this.el);
|
||||
this.triggerChanged();
|
||||
@@ -312,6 +466,8 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
|
||||
if (!this.htmlAttributes?.id) {
|
||||
setOverlayId(this.el);
|
||||
}
|
||||
// Initialize activeRadioId for radio buttons
|
||||
this.buttonsChanged();
|
||||
}
|
||||
|
||||
componentDidLoad() {
|
||||
@@ -355,8 +511,82 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
|
||||
this.triggerChanged();
|
||||
}
|
||||
|
||||
private renderActionSheetButtons(filteredButtons: ActionSheetButton[]) {
|
||||
const mode = getIonMode(this);
|
||||
const { activeRadioId } = this;
|
||||
|
||||
return filteredButtons.map((b, index) => {
|
||||
const isRadio = b.htmlAttributes?.role === 'radio';
|
||||
const buttonId = this.getButtonId(b, index);
|
||||
const radioButtons = this.getRadioButtons();
|
||||
const isActiveRadio = isRadio && buttonId === activeRadioId;
|
||||
const isFirstRadio = isRadio && b === radioButtons[0];
|
||||
|
||||
// For radio buttons, set tabindex: 0 for the active one, -1 for others
|
||||
// For non-radio buttons, use default tabindex (undefined, which means 0)
|
||||
|
||||
/**
|
||||
* For radio buttons, set tabindex based on activeRadioId
|
||||
* - If the button is the active radio, tabindex is 0
|
||||
* - If no radio is active, the first radio button should have tabindex 0
|
||||
* - All other radio buttons have tabindex -1
|
||||
* For non-radio buttons, use default tabindex (undefined, which means 0)
|
||||
*/
|
||||
let tabIndex: number | undefined;
|
||||
|
||||
if (isRadio) {
|
||||
// Focus on the active radio button
|
||||
if (isActiveRadio) {
|
||||
tabIndex = 0;
|
||||
} else if (!activeRadioId && isFirstRadio) {
|
||||
// No active radio, first radio gets focus
|
||||
tabIndex = 0;
|
||||
} else {
|
||||
// All other radios are not focusable
|
||||
tabIndex = -1;
|
||||
}
|
||||
} else {
|
||||
tabIndex = undefined;
|
||||
}
|
||||
|
||||
// For radio buttons, set aria-checked based on activeRadioId
|
||||
// Otherwise, use the value from htmlAttributes if provided
|
||||
const htmlAttrs = { ...b.htmlAttributes };
|
||||
if (isRadio) {
|
||||
htmlAttrs['aria-checked'] = isActiveRadio ? 'true' : 'false';
|
||||
}
|
||||
|
||||
return (
|
||||
<button
|
||||
{...htmlAttrs}
|
||||
role={isRadio ? 'radio' : undefined}
|
||||
type="button"
|
||||
id={buttonId}
|
||||
class={{
|
||||
...buttonClass(b),
|
||||
'action-sheet-selected': isActiveRadio,
|
||||
}}
|
||||
onClick={() => {
|
||||
if (isRadio) {
|
||||
this.selectRadioButton(b);
|
||||
}
|
||||
this.buttonClick(b);
|
||||
}}
|
||||
disabled={b.disabled}
|
||||
tabIndex={tabIndex}
|
||||
>
|
||||
<span class="action-sheet-button-inner">
|
||||
{b.icon && <ion-icon icon={b.icon} aria-hidden="true" lazy={false} class="action-sheet-icon" />}
|
||||
{b.text}
|
||||
</span>
|
||||
{mode === 'md' && <ion-ripple-effect></ion-ripple-effect>}
|
||||
</button>
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
render() {
|
||||
const { header, htmlAttributes, overlayIndex } = this;
|
||||
const { header, htmlAttributes, overlayIndex, hasRadioButtons } = this;
|
||||
const mode = getIonMode(this);
|
||||
const allButtons = this.getButtons();
|
||||
const cancelButton = allButtons.find((b) => b.role === 'cancel');
|
||||
@@ -388,7 +618,11 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
|
||||
|
||||
<div class="action-sheet-wrapper ion-overlay-wrapper" ref={(el) => (this.wrapperEl = el)}>
|
||||
<div class="action-sheet-container">
|
||||
<div class="action-sheet-group" ref={(el) => (this.groupEl = el)}>
|
||||
<div
|
||||
class="action-sheet-group"
|
||||
ref={(el) => (this.groupEl = el)}
|
||||
role={hasRadioButtons ? 'radiogroup' : undefined}
|
||||
>
|
||||
{header !== undefined && (
|
||||
<div
|
||||
id={headerID}
|
||||
@@ -401,22 +635,7 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
|
||||
{this.subHeader && <div class="action-sheet-sub-title">{this.subHeader}</div>}
|
||||
</div>
|
||||
)}
|
||||
{buttons.map((b) => (
|
||||
<button
|
||||
{...b.htmlAttributes}
|
||||
type="button"
|
||||
id={b.id}
|
||||
class={buttonClass(b)}
|
||||
onClick={() => this.buttonClick(b)}
|
||||
disabled={b.disabled}
|
||||
>
|
||||
<span class="action-sheet-button-inner">
|
||||
{b.icon && <ion-icon icon={b.icon} aria-hidden="true" lazy={false} class="action-sheet-icon" />}
|
||||
{b.text}
|
||||
</span>
|
||||
{mode === 'md' && <ion-ripple-effect></ion-ripple-effect>}
|
||||
</button>
|
||||
))}
|
||||
{this.renderActionSheetButtons(buttons)}
|
||||
</div>
|
||||
|
||||
{cancelButton && (
|
||||
|
||||
@@ -134,3 +134,58 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* This behavior does not vary across modes/directions.
|
||||
*/
|
||||
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
|
||||
test.describe(title('action-sheet: radio buttons'), () => {
|
||||
test('should render action sheet with radio buttons correctly', async ({ page }) => {
|
||||
await page.goto(`/src/components/action-sheet/test/a11y`, config);
|
||||
|
||||
const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent');
|
||||
const button = page.locator('#radioButtons');
|
||||
|
||||
await button.click();
|
||||
await ionActionSheetDidPresent.next();
|
||||
|
||||
const actionSheet = page.locator('ion-action-sheet');
|
||||
|
||||
const radioButtons = actionSheet.locator('.action-sheet-button[role="radio"]');
|
||||
await expect(radioButtons).toHaveCount(2);
|
||||
});
|
||||
|
||||
test('should navigate radio buttons with keyboard', async ({ page, pageUtils }) => {
|
||||
await page.goto(`/src/components/action-sheet/test/a11y`, config);
|
||||
|
||||
const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent');
|
||||
const button = page.locator('#radioButtons');
|
||||
|
||||
await button.click();
|
||||
await ionActionSheetDidPresent.next();
|
||||
|
||||
// Focus on the radios
|
||||
await pageUtils.pressKeys('Tab');
|
||||
|
||||
// Verify the first focusable radio button is focused
|
||||
let focusedElement = await page.evaluate(() => document.activeElement?.textContent?.trim());
|
||||
expect(focusedElement).toBe('Option 2');
|
||||
|
||||
// Navigate to the next radio button
|
||||
await page.keyboard.press('ArrowDown');
|
||||
|
||||
// Verify the first radio button is focused again (wrap around)
|
||||
focusedElement = await page.evaluate(() => document.activeElement?.textContent?.trim());
|
||||
expect(focusedElement).toBe('Option 1');
|
||||
|
||||
// Navigate to the next radio button
|
||||
await page.keyboard.press('ArrowDown');
|
||||
|
||||
// Navigate to the cancel button
|
||||
await pageUtils.pressKeys('Tab');
|
||||
|
||||
focusedElement = await page.evaluate(() => document.activeElement?.textContent?.trim());
|
||||
expect(focusedElement).toBe('Cancel');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -27,6 +27,7 @@
|
||||
<button class="expand" id="ariaLabelCancelButton" onclick="presentAriaLabelCancelButton()">
|
||||
Aria Label Cancel Button
|
||||
</button>
|
||||
<button class="expand" id="radioButtons" onclick="presentRadioButtons()">Radio Buttons</button>
|
||||
</main>
|
||||
|
||||
<script>
|
||||
@@ -100,6 +101,32 @@
|
||||
],
|
||||
});
|
||||
}
|
||||
|
||||
function presentRadioButtons() {
|
||||
openActionSheet({
|
||||
header: 'Select an option',
|
||||
buttons: [
|
||||
{
|
||||
text: 'Option 1',
|
||||
htmlAttributes: {
|
||||
role: 'radio',
|
||||
'aria-checked': 'false',
|
||||
},
|
||||
},
|
||||
{
|
||||
text: 'Option 2',
|
||||
htmlAttributes: {
|
||||
role: 'radio',
|
||||
'aria-checked': 'true',
|
||||
},
|
||||
},
|
||||
{
|
||||
text: 'Cancel',
|
||||
role: 'cancel',
|
||||
},
|
||||
],
|
||||
});
|
||||
}
|
||||
</script>
|
||||
</body>
|
||||
</html>
|
||||
|
||||
@@ -556,14 +556,19 @@ export class Select implements ComponentInterface {
|
||||
.filter((cls) => cls !== 'hydrated')
|
||||
.join(' ');
|
||||
const optClass = `${OPTION_CLASS} ${copyClasses}`;
|
||||
const isSelected = isOptionSelected(selectValue, value, this.compareWith);
|
||||
|
||||
return {
|
||||
role: isOptionSelected(selectValue, value, this.compareWith) ? 'selected' : '',
|
||||
role: isSelected ? 'selected' : '',
|
||||
text: option.textContent,
|
||||
cssClass: optClass,
|
||||
handler: () => {
|
||||
this.setValue(value);
|
||||
},
|
||||
htmlAttributes: {
|
||||
'aria-checked': isSelected ? 'true' : 'false',
|
||||
role: 'radio',
|
||||
},
|
||||
} as ActionSheetButton;
|
||||
});
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@ import { expect } from '@playwright/test';
|
||||
import { configs, test } from '@utils/test/playwright';
|
||||
|
||||
configs({ directions: ['ltr'], palettes: ['light', 'dark'] }).forEach(({ title, config }) => {
|
||||
test.describe(title('textarea: a11y'), () => {
|
||||
test.describe(title('select: a11y'), () => {
|
||||
test('default layout should not have accessibility violations', async ({ page }) => {
|
||||
await page.setContent(
|
||||
`
|
||||
|
||||
Reference in New Issue
Block a user