Skip to content

Commit 5e330bb

Browse files
abhiomkarcopybara-github
authored andcommitted
fix(radio): Fix issue where ripple stays active when radio label is clicked
Also fixed keyboard focus issue. Tab focus now lands on selected radio after fix. PiperOrigin-RevId: 334685489
1 parent 0361d2d commit 5e330bb

File tree

2 files changed

+63
-40
lines changed

2 files changed

+63
-40
lines changed

packages/radio/mwc-radio-base.ts

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,19 @@ export class RadioBase extends FormElement {
6464
*/
6565
set checked(isChecked: boolean) {
6666
const oldValue = this._checked;
67-
if (!!isChecked === !!oldValue) {
67+
if (isChecked === oldValue) {
6868
return;
6969
}
7070
this._checked = isChecked;
7171
if (this.formElement) {
7272
this.formElement.checked = isChecked;
7373
}
74-
if (this._selectionController !== undefined) {
75-
this._selectionController.update(this);
74+
this._selectionController?.update(this);
75+
76+
if (isChecked === false) {
77+
// Remove focus ring when unchecked on other radio programmatically.
78+
// Blur on input since this determines the focus style.
79+
this.formElement?.blur();
7680
}
7781
this.requestUpdate('checked', oldValue);
7882

@@ -115,6 +119,12 @@ export class RadioBase extends FormElement {
115119

116120
private _selectionController?: SingleSelectionController;
117121

122+
/**
123+
* input's tabindex is updated based on checked status.
124+
* Tab navigation will be removed from unchecked radios.
125+
*/
126+
@property({type: Number}) formElementTabIndex = 0;
127+
118128
@internalProperty() protected shouldRenderRipple = false;
119129

120130
@queryAsync('mwc-ripple') ripple!: Promise<Ripple|null>;
@@ -149,17 +159,20 @@ export class RadioBase extends FormElement {
149159
// radio's shadow root. For example, if we're stamping in a lit-html map
150160
// or repeat, then we'll be constructed before we're added to a root node.
151161
//
152-
// Also note if we aren't using native shadow DOM, then we don't technically
153-
// need a SelectionController, because our inputs will share document-scoped
154-
// native selection groups. However, it simplifies implementation and
155-
// testing to use one in all cases. In particular, it means we correctly
156-
// manage groups before the first update stamps the native input.
162+
// Also note if we aren't using native shadow DOM, we still need a
163+
// SelectionController, because we should update checked status of other
164+
// radios in the group when selection changes. It also simplifies
165+
// implementation and testing to use one in all cases.
157166
//
158167
// eslint-disable-next-line @typescript-eslint/no-use-before-define
159168
this._selectionController = SingleSelectionController.getController(this);
160169
this._selectionController.register(this);
161-
// With native <input type="radio">, when a checked radio is added to the
162-
// root, then it wins. Immediately update to emulate this behavior.
170+
171+
// Radios maybe checked before connected, update selection as soon it is
172+
// connected to DOM. Last checked radio button in the DOM will be selected.
173+
//
174+
// NOTE: If we update selection only after firstUpdate() we might mistakenly
175+
// update checked status before other radios are rendered.
163176
this._selectionController.update(this);
164177
}
165178

@@ -172,10 +185,6 @@ export class RadioBase extends FormElement {
172185
}
173186

174187
focus() {
175-
this.focusNative();
176-
}
177-
178-
focusNative() {
179188
this.formElement.focus();
180189
}
181190

@@ -189,10 +198,7 @@ export class RadioBase extends FormElement {
189198
}
190199

191200
private handleFocus() {
192-
if (this._selectionController !== undefined) {
193-
this._selectionController.focus(this);
194-
this.handleRippleFocus();
195-
}
201+
this.handleRippleFocus();
196202
}
197203

198204
private handleClick() {
@@ -202,7 +208,7 @@ export class RadioBase extends FormElement {
202208

203209
private handleBlur() {
204210
this.formElement.blur();
205-
this.handleRippleBlur();
211+
this.rippleHandlers.endFocus();
206212
}
207213

208214
/**
@@ -220,6 +226,7 @@ export class RadioBase extends FormElement {
220226
return html`
221227
<div class="mdc-radio ${classMap(classes)}">
222228
<input
229+
tabindex="${this.formElementTabIndex}"
223230
class="mdc-radio__native-control"
224231
type="radio"
225232
name="${this.name}"
@@ -244,16 +251,6 @@ export class RadioBase extends FormElement {
244251
</div>`;
245252
}
246253

247-
protected firstUpdated() {
248-
super.firstUpdated();
249-
// We might not have been able to synchronize this from the checked setter
250-
// earlier, if checked was set before the input was stamped.
251-
this.formElement.checked = this.checked;
252-
if (this._selectionController !== undefined) {
253-
this._selectionController.update(this);
254-
}
255-
}
256-
257254
protected handleRippleMouseDown(event: Event) {
258255
const onUp = () => {
259256
window.removeEventListener('mouseup', onUp);
@@ -286,10 +283,6 @@ export class RadioBase extends FormElement {
286283
this.rippleHandlers.startFocus();
287284
}
288285

289-
protected handleRippleBlur() {
290-
this.rippleHandlers.endFocus();
291-
}
292-
293286
protected changeHandler() {
294287
this.checked = this.formElement.checked;
295288
}

packages/radio/single-selection-controller.ts

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,13 @@ export class SingleSelectionSet {
3030
}
3131

3232
/**
33-
* Element with `checked` and `name` properties consumed by
33+
* Element that is checkable consumed by
3434
* `SingleSelectionController` and `SingleSelectionSet`
3535
*/
3636
export type CheckableElement = HTMLElement&{
3737
name: string;
3838
checked: boolean;
39+
formElementTabIndex?: number;
3940
};
4041

4142
/**
@@ -49,6 +50,8 @@ export type CheckableElement = HTMLElement&{
4950
* - Grouping of checkable elements by name
5051
* - Defaults grouping scope to host shadow root
5152
* - Document-wide scoping enabled
53+
* - Land focus only on checked element. Focuses leading element when none
54+
* checked.
5255
*
5356
* Intended Usage:
5457
*
@@ -84,10 +87,6 @@ export type CheckableElement = HTMLElement&{
8487
* this.selectionController!.unregister(this);
8588
* this.selectionController = null;
8689
* }
87-
*
88-
* focus() {
89-
* // focus native radio element
90-
* }
9190
* }
9291
* ```
9392
*/
@@ -212,6 +211,8 @@ export class SingleSelectionController {
212211
*
213212
* @param element Element from which selection set is derived and subsequently
214213
* focused.
214+
* @deprecated update() method now handles focus management by setting
215+
* appropriate tabindex to form element.
215216
*/
216217
focus(element: CheckableElement) {
217218
// Only manage focus state when using keyboard
@@ -226,6 +227,21 @@ export class SingleSelectionController {
226227
}
227228
}
228229

230+
/**
231+
* @return Returns true if atleast one radio is selected in the radio group.
232+
*/
233+
isAnySelected(element: CheckableElement): boolean {
234+
const set = this.getSet(element.name);
235+
236+
for (const e of set.set) {
237+
if (e.checked) {
238+
return true;
239+
}
240+
}
241+
242+
return false;
243+
}
244+
229245
/**
230246
* Returns the elements in the given element's selection set in document
231247
* position order.
@@ -301,13 +317,27 @@ export class SingleSelectionController {
301317
return;
302318
}
303319
this.updating = true;
320+
const set = this.getSet(element.name);
304321
if (element.checked) {
305-
const set = this.getSet(element.name);
306322
for (const e of set.set) {
307-
e.checked = (e == element);
323+
if (e == element) {
324+
continue;
325+
}
326+
e.checked = false;
308327
}
309328
set.selected = element;
310329
}
330+
331+
// When tabbing through land focus on the checked radio in the group.
332+
if (this.isAnySelected(element)) {
333+
for (const e of set.set) {
334+
if (e.formElementTabIndex === undefined) {
335+
break;
336+
}
337+
338+
e.formElementTabIndex = e.checked ? 0 : -1;
339+
}
340+
}
311341
this.updating = false;
312342
}
313343
}

0 commit comments

Comments
 (0)