Skip to content

Commit f0a767c

Browse files
crisbetocrisbeto
authored
fix(material/button-toggle): unable to tab into ngModel-based group on first render (#30103)
Fixes an issue where the button toggle group would reset all the buttons back to `tabindex="-1"` if they're all static (e.g. not in a loop) and none of them match the value. Fixes #30097.
1 parent de6c491 commit f0a767c

File tree

2 files changed

+76
-45
lines changed

2 files changed

+76
-45
lines changed

‎src/material/button-toggle/button-toggle.spec.ts‎

Lines changed: 46 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,6 @@ describe('MatButtonToggle with forms', () => {
9696
innerButtons = buttonToggleDebugElements.map(
9797
debugEl => debugEl.query(By.css('button'))!.nativeElement,
9898
);
99-
100-
fixture.detectChanges();
10199
});
102100

103101
it('should update the model before firing change event', fakeAsync(() => {
@@ -312,6 +310,18 @@ describe('MatButtonToggle with forms', () => {
312310

313311
expect(instance.toggles.map(t => t.checked)).toEqual([true, false, false]);
314312
});
313+
314+
it('should set the initial tabindex when using ngModel with a static list of options where none match the value', fakeAsync(() => {
315+
const fixture = TestBed.createComponent(ButtonToggleGroupWithNgModelAndStaticOptions);
316+
fixture.detectChanges();
317+
tick();
318+
const indexes = Array.from(
319+
fixture.nativeElement.querySelectorAll('button'),
320+
(button: HTMLElement) => button.getAttribute('tabindex'),
321+
);
322+
323+
expect(indexes).toEqual(['0', '-1', '-1']);
324+
}));
315325
});
316326

317327
describe('MatButtonToggle without forms', () => {
@@ -341,7 +351,7 @@ describe('MatButtonToggle without forms', () => {
341351
let groupNativeElement: HTMLElement;
342352
let buttonToggleDebugElements: DebugElement[];
343353
let buttonToggleNativeElements: HTMLElement[];
344-
let buttonToggleLabelElements: HTMLLabelElement[];
354+
let innerButtons: HTMLLabelElement[];
345355
let groupInstance: MatButtonToggleGroup;
346356
let buttonToggleInstances: MatButtonToggle[];
347357
let testComponent: ButtonTogglesInsideButtonToggleGroup;
@@ -360,34 +370,28 @@ describe('MatButtonToggle without forms', () => {
360370

361371
buttonToggleNativeElements = buttonToggleDebugElements.map(debugEl => debugEl.nativeElement);
362372

363-
buttonToggleLabelElements = fixture.debugElement
373+
innerButtons = fixture.debugElement
364374
.queryAll(By.css('button'))
365375
.map(debugEl => debugEl.nativeElement);
366376

367377
buttonToggleInstances = buttonToggleDebugElements.map(debugEl => debugEl.componentInstance);
368378
});
369379

370380
it('should initialize the tab index correctly', () => {
371-
buttonToggleLabelElements.forEach((buttonToggle, index) => {
372-
if (index === 0) {
373-
expect(buttonToggle.getAttribute('tabindex')).toBe('0');
374-
} else {
375-
expect(buttonToggle.getAttribute('tabindex')).toBe('-1');
376-
}
377-
});
381+
expect(innerButtons.map(b => b.getAttribute('tabindex'))).toEqual(['0', '-1', '-1']);
378382
});
379383

380384
it('should update the tab index correctly', () => {
381-
buttonToggleLabelElements[1].click();
385+
innerButtons[1].click();
382386
fixture.detectChanges();
383387

384-
expect(buttonToggleLabelElements[0].getAttribute('tabindex')).toBe('-1');
385-
expect(buttonToggleLabelElements[1].getAttribute('tabindex')).toBe('0');
388+
expect(innerButtons[0].getAttribute('tabindex')).toBe('-1');
389+
expect(innerButtons[1].getAttribute('tabindex')).toBe('0');
386390
});
387391

388392
it('should set individual button toggle names based on the group name', () => {
389393
expect(groupInstance.name).toBeTruthy();
390-
for (let buttonToggle of buttonToggleLabelElements) {
394+
for (let buttonToggle of innerButtons) {
391395
expect(buttonToggle.getAttribute('name')).toBe(groupInstance.name);
392396
}
393397
});
@@ -407,7 +411,7 @@ describe('MatButtonToggle without forms', () => {
407411

408412
expect(buttonToggleInstances[0].disabled).toBe(false);
409413

410-
buttonToggleLabelElements[0].click();
414+
innerButtons[0].click();
411415
fixture.detectChanges();
412416

413417
expect(buttonToggleInstances[0].checked).toBe(true);
@@ -456,7 +460,7 @@ describe('MatButtonToggle without forms', () => {
456460

457461
it('should update the group value when one of the toggles changes', () => {
458462
expect(groupInstance.value).toBeFalsy();
459-
buttonToggleLabelElements[0].click();
463+
innerButtons[0].click();
460464
fixture.detectChanges();
461465

462466
expect(groupInstance.value).toBe('test1');
@@ -465,7 +469,7 @@ describe('MatButtonToggle without forms', () => {
465469

466470
it('should propagate the value change back up via a two-way binding', () => {
467471
expect(groupInstance.value).toBeFalsy();
468-
buttonToggleLabelElements[0].click();
472+
innerButtons[0].click();
469473
fixture.detectChanges();
470474

471475
expect(groupInstance.value).toBe('test1');
@@ -474,15 +478,15 @@ describe('MatButtonToggle without forms', () => {
474478

475479
it('should update the group and toggles when one of the button toggles is clicked', () => {
476480
expect(groupInstance.value).toBeFalsy();
477-
buttonToggleLabelElements[0].click();
481+
innerButtons[0].click();
478482
fixture.detectChanges();
479483

480484
expect(groupInstance.value).toBe('test1');
481485
expect(groupInstance.selected).toBe(buttonToggleInstances[0]);
482486
expect(buttonToggleInstances[0].checked).toBe(true);
483487
expect(buttonToggleInstances[1].checked).toBe(false);
484488

485-
buttonToggleLabelElements[1].click();
489+
innerButtons[1].click();
486490
fixture.detectChanges();
487491

488492
expect(groupInstance.value).toBe('test2');
@@ -492,7 +496,7 @@ describe('MatButtonToggle without forms', () => {
492496
});
493497

494498
it('should check a button toggle upon interaction with underlying native radio button', () => {
495-
buttonToggleLabelElements[0].click();
499+
innerButtons[0].click();
496500
fixture.detectChanges();
497501

498502
expect(buttonToggleInstances[0].checked).toBe(true);
@@ -515,12 +519,12 @@ describe('MatButtonToggle without forms', () => {
515519
const changeSpy = jasmine.createSpy('button-toggle change listener');
516520
buttonToggleInstances[0].change.subscribe(changeSpy);
517521

518-
buttonToggleLabelElements[0].click();
522+
innerButtons[0].click();
519523
fixture.detectChanges();
520524
tick();
521525
expect(changeSpy).toHaveBeenCalledTimes(1);
522526

523-
buttonToggleLabelElements[0].click();
527+
innerButtons[0].click();
524528
fixture.detectChanges();
525529
tick();
526530

@@ -534,12 +538,12 @@ describe('MatButtonToggle without forms', () => {
534538
const changeSpy = jasmine.createSpy('button-toggle-group change listener');
535539
groupInstance.change.subscribe(changeSpy);
536540

537-
buttonToggleLabelElements[0].click();
541+
innerButtons[0].click();
538542
fixture.detectChanges();
539543
tick();
540544
expect(changeSpy).toHaveBeenCalled();
541545

542-
buttonToggleLabelElements[1].click();
546+
innerButtons[1].click();
543547
fixture.detectChanges();
544548
tick();
545549
expect(changeSpy).toHaveBeenCalledTimes(2);
@@ -579,7 +583,7 @@ describe('MatButtonToggle without forms', () => {
579583

580584
it('should update the model if a selected toggle is removed', fakeAsync(() => {
581585
expect(groupInstance.value).toBeFalsy();
582-
buttonToggleLabelElements[0].click();
586+
innerButtons[0].click();
583587
fixture.detectChanges();
584588

585589
expect(groupInstance.value).toBe('test1');
@@ -595,7 +599,7 @@ describe('MatButtonToggle without forms', () => {
595599
}));
596600

597601
it('should show checkmark indicator by default', () => {
598-
buttonToggleLabelElements[0].click();
602+
innerButtons[0].click();
599603
fixture.detectChanges();
600604

601605
expect(
@@ -1310,3 +1314,18 @@ class ButtonToggleGroupWithFormControlAndDynamicButtons {
13101314
control = new FormControl('');
13111315
values = ['a', 'b', 'c'];
13121316
}
1317+
1318+
@Component({
1319+
template: `
1320+
<mat-button-toggle-group [(ngModel)]="value">
1321+
<mat-button-toggle value="1">One</mat-button-toggle>
1322+
<mat-button-toggle value="2">Two</mat-button-toggle>
1323+
<mat-button-toggle value="3">Three</mat-button-toggle>
1324+
</mat-button-toggle-group>
1325+
`,
1326+
standalone: true,
1327+
imports: [MatButtonToggleModule, FormsModule],
1328+
})
1329+
class ButtonToggleGroupWithNgModelAndStaticOptions {
1330+
value = '';
1331+
}

‎src/material/button-toggle/button-toggle.ts‎

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -473,16 +473,28 @@ export class MatButtonToggleGroup implements ControlValueAccessor, OnInit, After
473473
return;
474474
}
475475

476+
const toggles = this._buttonToggles.toArray();
477+
476478
if (this.multiple && value) {
477479
if (!Array.isArray(value) && (typeof ngDevMode === 'undefined' || ngDevMode)) {
478480
throw Error('Value must be an array in multiple-selection mode.');
479481
}
480482

481483
this._clearSelection();
482-
value.forEach((currentValue: any) => this._selectValue(currentValue));
484+
value.forEach((currentValue: any) => this._selectValue(currentValue, toggles));
483485
} else {
484486
this._clearSelection();
485-
this._selectValue(value);
487+
this._selectValue(value, toggles);
488+
}
489+
490+
// In single selection mode we need at least one enabled toggle to always be focusable.
491+
if (!this.multiple && toggles.every(toggle => toggle.tabIndex === -1)) {
492+
for (const toggle of toggles) {
493+
if (!toggle.disabled) {
494+
toggle.tabIndex = 0;
495+
break;
496+
}
497+
}
486498
}
487499
}
488500

@@ -499,17 +511,16 @@ export class MatButtonToggleGroup implements ControlValueAccessor, OnInit, After
499511
}
500512

501513
/** Selects a value if there's a toggle that corresponds to it. */
502-
private _selectValue(value: any) {
503-
const correspondingOption = this._buttonToggles.find(toggle => {
504-
return toggle.value != null && toggle.value === value;
505-
});
506-
507-
if (correspondingOption) {
508-
correspondingOption.checked = true;
509-
this._selectionModel.select(correspondingOption);
510-
if (!this.multiple) {
511-
// If the button toggle is in single select mode, reset the tabIndex.
512-
correspondingOption.tabIndex = 0;
514+
private _selectValue(value: any, toggles: MatButtonToggle[]) {
515+
for (const toggle of toggles) {
516+
if (toggle.value != null && toggle.value === value) {
517+
toggle.checked = true;
518+
this._selectionModel.select(toggle);
519+
if (!this.multiple) {
520+
// If the button toggle is in single select mode, reset the tabIndex.
521+
toggle.tabIndex = 0;
522+
}
523+
break;
513524
}
514525
}
515526
}
@@ -601,8 +612,10 @@ export class MatButtonToggle implements OnInit, AfterViewInit, OnDestroy {
601612
return this._tabIndex;
602613
}
603614
set tabIndex(value: number | null) {
604-
this._tabIndex = value;
605-
this._markForCheck();
615+
if (value !== this._tabIndex) {
616+
this._tabIndex = value;
617+
this._markForCheck();
618+
}
606619
}
607620
private _tabIndex: number | null;
608621

@@ -668,14 +681,13 @@ export class MatButtonToggle implements OnInit, AfterViewInit, OnDestroy {
668681
constructor() {
669682
inject(_CdkPrivateStyleLoader).load(_StructuralStylesLoader);
670683
const toggleGroup = inject<MatButtonToggleGroup>(MAT_BUTTON_TOGGLE_GROUP, {optional: true})!;
671-
const defaultTabIndex = inject(new HostAttributeToken('tabindex'), {optional: true});
684+
const defaultTabIndex = inject(new HostAttributeToken('tabindex'), {optional: true}) || '';
672685
const defaultOptions = inject<MatButtonToggleDefaultOptions>(
673686
MAT_BUTTON_TOGGLE_DEFAULT_OPTIONS,
674687
{optional: true},
675688
);
676689

677-
const parsedTabIndex = Number(defaultTabIndex);
678-
this.tabIndex = parsedTabIndex || parsedTabIndex === 0 ? parsedTabIndex : null;
690+
this._tabIndex = parseInt(defaultTabIndex) || 0;
679691
this.buttonToggleGroup = toggleGroup;
680692
this.appearance =
681693
defaultOptions && defaultOptions.appearance ? defaultOptions.appearance : 'standard';

0 commit comments

Comments
 (0)