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

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 committed Oct 30, 2024
1 parent 16c6f42 commit ce04924
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 11 deletions.
2 changes: 1 addition & 1 deletion packages/main/src/RadioButton.hbs
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
2 changes: 1 addition & 1 deletion packages/main/src/RadioButton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,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
19 changes: 11 additions & 8 deletions packages/main/src/RadioButtonGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ class RadioButtonGroup {
static updateSelectionInGroup(radioBtnToSelect: RadioButton, groupName: string) {
const checkedRadio = this.getCheckedRadioFromGroup(groupName);

if (checkedRadio) {
// if there is radio button, that is already checked or the new radio button is not readonly
if (checkedRadio && !radioBtnToSelect.readonly) {
this._deselectRadio(checkedRadio);
}

Expand All @@ -172,9 +173,11 @@ class RadioButtonGroup {
static _selectRadio(radioBtn: RadioButton) {
if (radioBtn) {
radioBtn.focus();
radioBtn.checked = true;
radioBtn._checked = true;
radioBtn.fireDecoratorEvent("change");
if (!radioBtn.readonly) {
radioBtn.checked = true;
radioBtn._checked = true;
radioBtn.fireDecoratorEvent("change");
}
}
}

Expand All @@ -187,11 +190,11 @@ class RadioButtonGroup {
let nextRadioToSelect = null;

if (pos === groupLength - 1) {
if (group[0].disabled || group[0].readonly) {
if (group[0].disabled) {
return this._nextSelectable(1, group);
}
nextRadioToSelect = group[0];
} else if (group[pos + 1].disabled || group[pos + 1].readonly) {
} else if (group[pos + 1].disabled) {
return this._nextSelectable(pos + 1, group);
} else {
nextRadioToSelect = group[pos + 1];
Expand All @@ -204,11 +207,11 @@ class RadioButtonGroup {
const groupLength = group.length;
let previousRadioToSelect = null;
if (pos === 0) {
if (group[groupLength - 1].disabled || group[groupLength - 1].readonly) {
if (group[groupLength - 1].disabled) {
return this._previousSelectable(groupLength - 1, group);
}
previousRadioToSelect = group[groupLength - 1];
} else if (group[pos - 1].disabled || group[pos - 1].readonly) {
} else if (group[pos - 1].disabled) {
return this._previousSelectable(pos - 1, group);
} else {
previousRadioToSelect = group[pos - 1];
Expand Down
11 changes: 10 additions & 1 deletion packages/main/test/specs/RadioButton.spec.js
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 ce04924

Please sign in to comment.