Skip to content

Commit d6a7b81

Browse files
ilhan007ilhan007
authored
fix(ui5-radiobutton): fix tab order within group (#2783)
We used to set tabindex="0" to the selected item and tabindex="-1" to the rest, which works fine in all cases, besides the one reported in the issue - when the application wants to define a group without preselected item. In this case, all the radio buttons in the group have tabindex="-1" and as a result the group is not accessible via the keyboard. Now, we calculate the tabindex for each item on every important action, such as: selection of item within the group, creating a new group, adding an item to the group or removal an item from the group. The logic considers the use-case from the issue as well - If the group does not have a selected item, the first (not disabled) item would have tabindex="0". In openui5, this is handled by the sap.m.RadioButtonGroup control, here it is implemented within the RadioButtonGroup util class. FIXES: #2774
1 parent ae6c7ad commit d6a7b81

File tree

4 files changed

+101
-22
lines changed

4 files changed

+101
-22
lines changed

‎packages/main/src/RadioButton.js

+14-2
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,12 @@ const metadata = {
161161
wrap: {
162162
type: Boolean,
163163
},
164+
165+
_tabIndex: {
166+
type: String,
167+
defaultValue: "-1",
168+
noAttribute: true,
169+
},
164170
},
165171
events: /** @lends sap.ui.webcomponents.main.RadioButton.prototype */ {
166172

@@ -240,13 +246,14 @@ class RadioButton extends UI5Element {
240246

241247
onBeforeRendering() {
242248
this.syncGroup();
243-
244249
this._enableFormSupport();
245250
}
246251

247252
syncGroup() {
248253
const oldGroup = this._name;
249254
const currentGroup = this.name;
255+
const oldSelected = this._selected;
256+
const currentSelected = this.selected;
250257

251258
if (currentGroup !== oldGroup) {
252259
if (oldGroup) {
@@ -262,7 +269,12 @@ class RadioButton extends UI5Element {
262269
RadioButtonGroup.enforceSingleSelection(this, currentGroup);
263270
}
264271

272+
if (this.name && currentSelected !== oldSelected) {
273+
RadioButtonGroup.updateTabOrder(this.name);
274+
}
275+
265276
this._name = this.name;
277+
this._selected = this.selected;
266278
}
267279

268280
_enableFormSupport() {
@@ -395,7 +407,7 @@ class RadioButton extends UI5Element {
395407
}
396408

397409
if (this.name) {
398-
return this.selected ? "0" : "-1";
410+
return this._tabIndex;
399411
}
400412

401413
return tabindex || "0";

‎packages/main/src/RadioButtonGroup.js

+24
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ class RadioButtonGroup {
2323
} else {
2424
this.createGroup(radioBtn, groupName);
2525
}
26+
27+
this.updateTabOrder(groupName);
2628
}
2729

2830
static removeFromGroup(radioBtn, groupName) {
@@ -48,6 +50,8 @@ class RadioButtonGroup {
4850
if (!group.length) {
4951
this.removeGroup(groupName);
5052
}
53+
54+
this.updateTabOrder(groupName);
5155
}
5256

5357
static createGroup(radioBtn, groupName) {
@@ -72,6 +76,23 @@ class RadioButtonGroup {
7276
this.updateSelectionInGroup(nextItemToSelect, groupName);
7377
}
7478

79+
static updateTabOrder(groupName) {
80+
if (!this.hasGroup(groupName)) {
81+
return;
82+
}
83+
84+
const group = this.getGroup(groupName);
85+
const hasSelectedRadio = group.some(radioBtn => radioBtn.selected);
86+
87+
group.filter(radioBtn => !radioBtn.disabled).forEach((radioBtn, idx) => {
88+
if (hasSelectedRadio) {
89+
radioBtn._tabIndex = radioBtn.selected ? "0" : "-1";
90+
} else {
91+
radioBtn._tabIndex = idx === 0 ? "0" : "-1";
92+
}
93+
});
94+
}
95+
7596
static selectPreviousItem(item, groupName) {
7697
const group = this.getGroup(groupName),
7798
groupLength = group.length,
@@ -88,6 +109,7 @@ class RadioButtonGroup {
88109

89110
static selectItem(item, groupName) {
90111
this.updateSelectionInGroup(item, groupName);
112+
this.updateTabOrder(groupName);
91113
}
92114

93115
static updateSelectionInGroup(radioBtnToSelect, groupName) {
@@ -161,6 +183,8 @@ class RadioButtonGroup {
161183
} else if (radioBtn === selectedRadio) {
162184
this.selectedRadios.set(groupName, null);
163185
}
186+
187+
this.updateTabOrder(groupName);
164188
}
165189

166190
static get groups() {

‎packages/main/test/pages/RadioButton.html

+38-19
Original file line numberDiff line numberDiff line change
@@ -17,50 +17,57 @@
1717
.radio-section {
1818
display: flex;
1919
flex-direction: column;
20+
margin-bottom: 2rem;
2021
}
2122
</style>
2223
</head>
2324

2425
<body style="background-color: var(--sapBackgroundColor);">
25-
<ui5-title level="H1">ui5-radiobutton</ui5-title>
26-
<ui5-radiobutton id="rb1"></ui5-radiobutton>
27-
<ui5-radiobutton id="rb2" text="Option B"></ui5-radiobutton>
28-
<ui5-radiobutton id="rb3" wrap text="Option C"></ui5-radiobutton>
29-
<ui5-radiobutton id="rb4" disabled text="Option D"></ui5-radiobutton>
30-
<ui5-radiobutton id="truncatingRb" text="Long long long text that should truncate at some point">></ui5-radiobutton>
31-
<br/>
32-
<ui5-radiobutton id="wrappingRb" wrap text="Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s." style="width: 300px"></ui5-radiobutton>
3326

34-
<ui5-input id="field"></ui5-input>
27+
<ui5-title level="H1">Radios within a group</ui5-title><br>
28+
<div class="radio-section">
29+
<ui5-title>Group1 - None pre-selection</ui5-title>
30+
<ui5-radiobutton id="testRbtn" text="N/A" disabled name="test"></ui5-radiobutton>
31+
<ui5-radiobutton id="testRbtn1" text="Male" name="test"></ui5-radiobutton>
32+
<ui5-radiobutton id="testRbtn2"text="Female" name="test"></ui5-radiobutton>
33+
</div>
34+
35+
<section class="radio-section">
36+
<ui5-title>Group 2</ui5-title>
37+
<ui5-radiobutton id="testRbtn11" name="test2" selected text="Selected A"></ui5-radiobutton>
38+
<ui5-radiobutton id="testRbtn12" name="test2" disabled text="Disabled B"></ui5-radiobutton>
39+
<ui5-radiobutton id="testRbtn13" name="test2" text="Standard C"></ui5-radiobutton>
40+
</section>
3541

3642
<section class="radio-section">
37-
<ui5-title>ui5-radiobutton in group</ui5-title>
43+
<ui5-title>Group 3</ui5-title>
44+
<ui5-input id="tabField"></ui5-input>
3845
<ui5-radiobutton id="groupRb1" name="a" selected text="Option A long long should shrink long long text text text text text text text text"></ui5-radiobutton>
3946
<ui5-radiobutton id="groupRb2" name="a" disabled text="Option C"></ui5-radiobutton>
40-
<ui5-radiobutton id="groupRbReadOnly" name="a" readonly text="Option E"></ui5-radiobutton>
4147
<ui5-radiobutton id="groupRb3" name="a" text="Option D"></ui5-radiobutton>
48+
<ui5-radiobutton id="groupRbReadOnly" name="a" readonly text="Option E"></ui5-radiobutton>
4249
</section>
4350

4451
<section class="radio-section">
45-
<ui5-title>ui5-radiobutton in group</ui5-title>
52+
<ui5-title>Group 4</ui5-title>
4653
<ui5-radiobutton id="groupRb4" name="b" selected text="Option A long long should shrink long long text text text text text text text text"></ui5-radiobutton>
4754
<ui5-radiobutton id="groupRb5" name="b" disabled text="Option C"></ui5-radiobutton>
4855
<ui5-radiobutton id="groupRb6" name="b" text="Option D"></ui5-radiobutton>
4956
</section>
5057

51-
<div id="radioGroup" class="radio-section">
52-
<ui5-title>Group of states</ui5-title>
53-
<ui5-label id="lblRadioGroup"></ui5-label>
54-
<ui5-label id="lblEventCounter"></ui5-label>
58+
<section id="radioGroup" class="radio-section">
59+
<ui5-title>Group 4 - Value states</ui5-title>
60+
<ui5-label id="lblRadioGroup">N/A</ui5-label>
61+
<ui5-label id="lblEventCounter">0</ui5-label>
5562
<ui5-radiobutton id="groupRb7" text="None selected" value-state="None" name="GroupB"></ui5-radiobutton>
5663
<ui5-radiobutton id="groupRb8" text="Warning" value-state="Warning" selected name="GroupB"></ui5-radiobutton>
5764
<ui5-radiobutton id="groupRb9"text="Error" value-state="Error" selected name="GroupB"></ui5-radiobutton>
5865
<ui5-radiobutton id="groupRb10" text="Default selected" value-state="None" selected name="GroupB"></ui5-radiobutton>
59-
<ui5-radiobutton id="groupRb11" text="Other group selected" value-state="None" selected name="GroupBB"></ui5-radiobutton>
60-
</div>
66+
<ui5-radiobutton id="groupRb11" text="Radio From Another Group" value-state="None" selected name="GroupBB"></ui5-radiobutton>
67+
</section>
6168

69+
<ui5-title level="H1">Standalone Radios Not Grouped</ui5-title>
6270
<section>
63-
<ui5-title>ui5-radiobutton states</ui5-title></p>
6471
<ui5-radiobutton id="myRb6" selected text="Default"></ui5-radiobutton>
6572
<ui5-radiobutton id="myRb7" readonly text="read only"></ui5-radiobutton>
6673
<ui5-radiobutton id="myRb8" disabled text="disabled"></ui5-radiobutton>
@@ -72,6 +79,18 @@
7279
<ui5-radiobutton id="myRb14" value-state="Error" selected text="error"></ui5-radiobutton>
7380
</section>
7481

82+
83+
<section class="radio-section">
84+
<ui5-radiobutton id="rb1"></ui5-radiobutton>
85+
<ui5-radiobutton id="rb2" text="Option B"></ui5-radiobutton>
86+
<ui5-radiobutton id="rb3" wrap text="Option C"></ui5-radiobutton>
87+
<ui5-radiobutton id="rb4" disabled text="Option D"></ui5-radiobutton>
88+
<ui5-radiobutton id="truncatingRb" text="Long long long text that should truncate at some point">></ui5-radiobutton>
89+
<br/>
90+
<ui5-radiobutton id="wrappingRb" wrap text="Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s." style="width: 300px"></ui5-radiobutton>
91+
<ui5-input id="field"></ui5-input>
92+
</section>
93+
7594
<p>*Params</p>
7695
<p>
7796
<ui5-label>- for compact add 'ui5-content-density-compact' class to any dom element</ui5-label>

‎packages/main/test/specs/RadioButton.spec.js

+25-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ describe("RadioButton general interaction", () => {
5656
});
5757

5858
it("tests radio buttons selection within group with ARROW-RIGHT key", () => {
59-
const field = browser.$("#field");
59+
const field = browser.$("#tabField");
6060
const radioButtonPreviouslySelected = browser.$("#groupRb1");
6161
const radioButtonToBeSelected = browser.$("#groupRb3");
6262

@@ -81,14 +81,38 @@ describe("RadioButton general interaction", () => {
8181
assert.ok(radioButtonToBeSelected.getProperty("selected"), "Pressing ArrowLeft selects the next (not disabled) radio in the group.");
8282
});
8383

84+
it("tests tabindex within group with selected item", () => {
85+
const selectedRadio = browser.$("#testRbtn11").shadow$(".ui5-radio-root");
86+
const disabledRadio = browser.$("#testRbtn12").shadow$(".ui5-radio-root");
87+
const radio = browser.$("#testRbtn13").shadow$(".ui5-radio-root");
88+
89+
assert.strictEqual(selectedRadio.getAttribute("tabindex"), "0", "The selected radio has tabindex = 0");
90+
assert.strictEqual(disabledRadio.getAttribute("tabindex"), "-1", "The disabled radio has tabindex = -1");
91+
assert.strictEqual(radio.getAttribute("tabindex"), "-1", "None selected item has tabindex = -1");
92+
});
93+
94+
it("tests tabindex within group with no selected item", () => {
95+
const radio1 = browser.$("#testRbtn1").shadow$(".ui5-radio-root");
96+
const radio2 = browser.$("#testRbtn2").shadow$(".ui5-radio-root");
97+
98+
assert.strictEqual(radio1.getAttribute("tabindex"), "0", "The first radio has tabindex = 0");
99+
assert.strictEqual(radio2.getAttribute("tabindex"), "-1", "The other radio has tabindex = -1");
100+
});
101+
84102
it("tests radio buttons selection within group by clicking", () => {
85103
const radioButtonPreviouslySelected = browser.$("#groupRb6");
104+
const radioButtonPreviouslySelectedRoot = browser.$("#groupRb6").shadow$(".ui5-radio-root");
105+
86106
const radioButtonToBeSelected = browser.$("#groupRb4");
107+
const radioButtonToBeSelectedRoot = browser.$("#groupRb4").shadow$(".ui5-radio-root");
87108

88109
radioButtonToBeSelected.click();
89110

90111
assert.ok(!radioButtonPreviouslySelected.getProperty("selected"), "Previously selected item has been de-selected.");
112+
assert.strictEqual(radioButtonPreviouslySelectedRoot.getAttribute("tabindex"), "-1", "The previously selected radio has tabindex = -1");
113+
91114
assert.ok(radioButtonToBeSelected.getProperty("selected"), "Pressing ArrowRight selects the next (not disabled) radio in the group.");
115+
assert.strictEqual(radioButtonToBeSelectedRoot.getAttribute("tabindex"), "0", "The newly selected radio has tabindex = 0");
92116
});
93117

94118
it("tests single selection within group, even if multiple radios are set as selected", () => {

0 commit comments

Comments
 (0)