Skip to content

Commit

Permalink
fix(ui5-radio-button): fix aria-disabled and focus of the read-only r…
Browse files Browse the repository at this point in the history 
…adio buttons (#10111)

* fix(ui5-radio-button): fix aria-disabled and focus of the read-only radio buttons

This pull request fixes two issues:
1. radio buttons cannot be focused using the keyboard
2. radio buttons lack the aria-disabled attribute
and the screen reader doesn't announce that they
can't be selected

fixes: #10025
  • Loading branch information
@LidiyaGeorgieva
LidiyaGeorgieva authored Nov 26, 2024
1 parent 3bad6ed commit 3bcfd1d
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 39 deletions.
2 changes: 1 addition & 1 deletion packages/main/src/RadioButton.hbs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<circle part="outer-ring" class="ui5-radio-svg-outer" cx="50%" cy="50%" r="50%" />
<circle part="inner-ring" class="ui5-radio-svg-inner" cx="50%" cy="50%" />
</svg>
<input type='radio' ?required="{{required}}" ?checked="{{checked}}" ?readonly="{{readonly}}" ?disabled="{{effectiveAriaDisabled}}" name="{{name}}" data-sap-no-tab-ref/>
<input type='radio' ?required="{{required}}" ?checked="{{checked}}" ?readonly="{{readonly}}" ?disabled="{{disabled}}" name="{{name}}" data-sap-no-tab-ref/>
</div>

{{#if text}}
Expand Down
9 changes: 6 additions & 3 deletions packages/main/src/RadioButton.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ class RadioButton extends UI5Element implements IFormInputElement {
/**
* Defines whether the component is read-only.
*
* **Note:** A read-only component is not editable,
* but still provides visual feedback upon user interaction.
* **Note:** A read-only component isn't editable or selectable.
* However, because it's focusable, it still provides visual feedback upon user interaction.
* @default false
* @public
*/
Expand All @@ -125,6 +125,9 @@ class RadioButton extends UI5Element implements IFormInputElement {
* **Note:** The property value can be changed with user interaction,
* either by clicking/tapping on the component,
* or by using the Space or Enter key.
*
* **Note:** Only enabled radio buttons can be checked.
* Read-only radio buttons are not selectable, and therefore are always unchecked.
* @default false
* @formEvents change
* @formProperty
Expand Down Expand Up @@ -406,7 +409,7 @@ class RadioButton extends UI5Element implements IFormInputElement {
}

get effectiveAriaDisabled() {
return this.disabled ? "true" : null;
return (this.disabled || this.readonly) ? "true" : null;
}

get ariaLabelText() {
Expand Down
91 changes: 57 additions & 34 deletions packages/main/src/RadioButtonGroup.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import getActiveElement from "@ui5/webcomponents-base/dist/util/getActiveElement.js";
import type RadioButton from "./RadioButton.js";

class RadioButtonGroup {
Expand Down Expand Up @@ -83,12 +84,12 @@ class RadioButtonGroup {
return;
}

const nextItemToSelect = this._nextSelectable(currentItemPosition, group);
if (!nextItemToSelect) {
const nextItemToFocus = this._nextFocusable(currentItemPosition, group);
if (!nextItemToFocus) {
return;
}

this.updateSelectionInGroup(nextItemToSelect, groupName);
this.updateSelectionInGroup(nextItemToFocus, groupName);
}

static updateFormValidity(groupName: string) {
Expand Down Expand Up @@ -117,8 +118,21 @@ class RadioButtonGroup {
const hasCheckedRadio = group.some(radioBtn => radioBtn.checked);

group.filter(radioBtn => !radioBtn.disabled).forEach((radioBtn, idx) => {
let activeElement: Element | RadioButton = getActiveElement()!;

if (activeElement.classList.contains("ui5-radio-root")) {
activeElement = activeElement.getRootNode() as Element;
if (activeElement instanceof ShadowRoot) {
activeElement = activeElement.host;
}
}

if (hasCheckedRadio) {
radioBtn._tabIndex = radioBtn.checked ? "0" : "-1";
if (activeElement.hasAttribute("ui5-radio-button") && (activeElement as RadioButton).readonly) {
radioBtn._tabIndex = activeElement === radioBtn && radioBtn.readonly ? "0" : "-1";
} else {
radioBtn._tabIndex = radioBtn.checked ? "0" : "-1";
}
} else {
radioBtn._tabIndex = idx === 0 ? "0" : "-1";
}
Expand All @@ -139,12 +153,12 @@ class RadioButtonGroup {
return;
}

const previousItemToSelect = this._previousSelectable(currentItemPosition, group);
if (!previousItemToSelect) {
const previousItemToFocus = this._previousFocusable(currentItemPosition, group);
if (!previousItemToFocus) {
return;
}

this.updateSelectionInGroup(previousItemToSelect, groupName);
this.updateSelectionInGroup(previousItemToFocus, groupName);
}

static selectItem(item: RadioButton, groupName: string) {
Expand All @@ -155,12 +169,24 @@ class RadioButtonGroup {
static updateSelectionInGroup(radioBtnToSelect: RadioButton, groupName: string) {
const checkedRadio = this.getCheckedRadioFromGroup(groupName);

if (checkedRadio) {
if (checkedRadio && !radioBtnToSelect.readonly) {
this._deselectRadio(checkedRadio);
this.checkedRadios.set(groupName, radioBtnToSelect);
}

this._selectRadio(radioBtnToSelect);
this.checkedRadios.set(groupName, radioBtnToSelect);
// the focusable radio buttons are the enabled and the read-only ones, but only the enabled are selectable
if (radioBtnToSelect) {
radioBtnToSelect.focus();

if (!radioBtnToSelect.readonly) {
this._selectRadio(radioBtnToSelect);
} else {
// Ensure updateTabOrder is called after focus
setTimeout(() => {
this.updateTabOrder(groupName);
}, 0);
}
}
}

static _deselectRadio(radioBtn: RadioButton) {
Expand All @@ -170,51 +196,48 @@ class RadioButtonGroup {
}

static _selectRadio(radioBtn: RadioButton) {
if (radioBtn) {
radioBtn.focus();
radioBtn.checked = true;
radioBtn._checked = true;
radioBtn.fireDecoratorEvent("change");
}
radioBtn.checked = true;
radioBtn._checked = true;
radioBtn.fireDecoratorEvent("change");
}

static _nextSelectable(pos: number, group: RadioButton[]): RadioButton | null {
static _nextFocusable(pos: number, group: RadioButton[]): RadioButton | null {
if (!group) {
return null;
}

const groupLength = group.length;
let nextRadioToSelect = null;
let nextRadioToFocus = null;

if (pos === groupLength - 1) {
if (group[0].disabled || group[0].readonly) {
return this._nextSelectable(1, group);
if (group[0].disabled) {
return this._nextFocusable(1, group);
}
nextRadioToSelect = group[0];
} else if (group[pos + 1].disabled || group[pos + 1].readonly) {
return this._nextSelectable(pos + 1, group);
nextRadioToFocus = group[0];
} else if (group[pos + 1].disabled) {
return this._nextFocusable(pos + 1, group);
} else {
nextRadioToSelect = group[pos + 1];
nextRadioToFocus = group[pos + 1];
}

return nextRadioToSelect;
return nextRadioToFocus;
}

static _previousSelectable(pos: number, group: RadioButton[]): RadioButton | null {
static _previousFocusable(pos: number, group: RadioButton[]): RadioButton | null {
const groupLength = group.length;
let previousRadioToSelect = null;
let previousRadioToFocus = null;
if (pos === 0) {
if (group[groupLength - 1].disabled || group[groupLength - 1].readonly) {
return this._previousSelectable(groupLength - 1, group);
if (group[groupLength - 1].disabled) {
return this._previousFocusable(groupLength - 1, group);
}
previousRadioToSelect = group[groupLength - 1];
} else if (group[pos - 1].disabled || group[pos - 1].readonly) {
return this._previousSelectable(pos - 1, group);
previousRadioToFocus = group[groupLength - 1];
} else if (group[pos - 1].disabled) {
return this._previousFocusable(pos - 1, group);
} else {
previousRadioToSelect = group[pos - 1];
previousRadioToFocus = group[pos - 1];
}

return previousRadioToSelect;
return previousRadioToFocus;
}

static enforceSingleSelection(radioBtn: RadioButton, groupName: string) {
Expand Down
5 changes: 5 additions & 0 deletions packages/main/test/pages/RadioButton.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@
<ui5-radio-button id="testRbtn11" name="test2" checked text="Selected A"></ui5-radio-button>
<ui5-radio-button id="testRbtn12" name="test2" disabled text="Disabled B"></ui5-radio-button>
<ui5-radio-button id="testRbtn13" name="test2" text="Standard C"></ui5-radio-button>
<ui5-radio-button name="test2" readonly text="Read-Only D"></ui5-radio-button>
<ui5-radio-button name="test2" readonly text="Read-Only E"></ui5-radio-button>
<ui5-radio-button name="test2" text="Standard F"></ui5-radio-button>
<ui5-radio-button name="test2" readonly text="Read-Only G"></ui5-radio-button>
<ui5-radio-button name="test2" text="Standard H"></ui5-radio-button>
</div>

<div aria-labelledby="radioGroupTitle3" role="radiogroup" class="radio-section">
Expand Down
11 changes: 10 additions & 1 deletion packages/main/test/specs/RadioButton.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe("Rendering", () => {

assert.notOk(await readOnlyRadioButtonRoot.getAttribute("aria-readonly"), "aria-readonly is not set to the root level");
assert.strictEqual(await readOnlyRadioButtonInput.getAttribute("readonly"), "true", "internal input is readonly");
assert.strictEqual(await readOnlyRadioButtonRoot.getAttribute("aria-disabled"), "true", "aria-disabled = true");
});
});

Expand Down Expand Up @@ -87,6 +88,7 @@ describe("RadioButton general interaction", () => {
const field = await browser.$("#tabField");
const radioButtonPreviouslySelected = await browser.$("#groupRb1");
const radioButtonToBeSelected = await browser.$("#groupRb3");
const radioButtonToBeSelectedNext = await browser.$("#groupRbReadOnly");

await field.click();
await field.keys("Tab");
Expand All @@ -96,7 +98,14 @@ describe("RadioButton general interaction", () => {
assert.notOk(await radioButtonPreviouslySelected.getProperty("checked"), "Previously selected item has been de-selected.");
assert.ok(await radioButtonToBeSelected.getProperty("checked"), "Pressing ArrowRight selects the next (not disabled) radio in the group.");

await radioButtonToBeSelected.keys("Tab");
await radioButtonToBeSelected.keys("ArrowRight");
const activeElementId = await browser.$(await browser.getActiveElement()).getAttribute("id");

assert.ok(await radioButtonToBeSelected.getProperty("checked"), "Previously selected item is still selected");
assert.notOk(await radioButtonToBeSelectedNext.getProperty("checked"), "Read-only radio button is not selected.");
assert.ok(activeElementId === "groupRbReadOnly", " Focus is on the last radio button, which is read-only");

await radioButtonToBeSelectedNext.keys("Tab");
});

it("tests radio buttons selection within group with ARROW-LEFT key", async () => {
Expand Down

0 comments on commit 3bcfd1d

Please sign in to comment.