Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(material/button-toggle): unable to tab into ngModel-based group on first render #30103

Merged
merged 1 commit into from
Dec 2, 2024
Merged

fix(material/button-toggle): unable to tab into ngModel-based group on first render #30103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 46 additions & 27 deletions src/material/button-toggle/button-toggle.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ describe('MatButtonToggle with forms', () => {
innerButtons = buttonToggleDebugElements.map(
debugEl => debugEl.query(By.css('button'))!.nativeElement,
);

fixture.detectChanges();
});

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

expect(instance.toggles.map(t => t.checked)).toEqual([true, false, false]);
});

it('should set the initial tabindex when using ngModel with a static list of options where none match the value', fakeAsync(() => {
const fixture = TestBed.createComponent(ButtonToggleGroupWithNgModelAndStaticOptions);
fixture.detectChanges();
tick();
const indexes = Array.from(
fixture.nativeElement.querySelectorAll('button'),
(button: HTMLElement) => button.getAttribute('tabindex'),
);

expect(indexes).toEqual(['0', '-1', '-1']);
}));
});

describe('MatButtonToggle without forms', () => {
Expand Down Expand Up @@ -341,7 +351,7 @@ describe('MatButtonToggle without forms', () => {
let groupNativeElement: HTMLElement;
let buttonToggleDebugElements: DebugElement[];
let buttonToggleNativeElements: HTMLElement[];
let buttonToggleLabelElements: HTMLLabelElement[];
let innerButtons: HTMLLabelElement[];
let groupInstance: MatButtonToggleGroup;
let buttonToggleInstances: MatButtonToggle[];
let testComponent: ButtonTogglesInsideButtonToggleGroup;
Expand All @@ -360,34 +370,28 @@ describe('MatButtonToggle without forms', () => {

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

buttonToggleLabelElements = fixture.debugElement
innerButtons = fixture.debugElement
.queryAll(By.css('button'))
.map(debugEl => debugEl.nativeElement);

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

it('should initialize the tab index correctly', () => {
buttonToggleLabelElements.forEach((buttonToggle, index) => {
if (index === 0) {
expect(buttonToggle.getAttribute('tabindex')).toBe('0');
} else {
expect(buttonToggle.getAttribute('tabindex')).toBe('-1');
}
});
expect(innerButtons.map(b => b.getAttribute('tabindex'))).toEqual(['0', '-1', '-1']);
});

it('should update the tab index correctly', () => {
buttonToggleLabelElements[1].click();
innerButtons[1].click();
fixture.detectChanges();

expect(buttonToggleLabelElements[0].getAttribute('tabindex')).toBe('-1');
expect(buttonToggleLabelElements[1].getAttribute('tabindex')).toBe('0');
expect(innerButtons[0].getAttribute('tabindex')).toBe('-1');
expect(innerButtons[1].getAttribute('tabindex')).toBe('0');
});

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

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

buttonToggleLabelElements[0].click();
innerButtons[0].click();
fixture.detectChanges();

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

it('should update the group value when one of the toggles changes', () => {
expect(groupInstance.value).toBeFalsy();
buttonToggleLabelElements[0].click();
innerButtons[0].click();
fixture.detectChanges();

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

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

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

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

expect(groupInstance.value).toBe('test1');
expect(groupInstance.selected).toBe(buttonToggleInstances[0]);
expect(buttonToggleInstances[0].checked).toBe(true);
expect(buttonToggleInstances[1].checked).toBe(false);

buttonToggleLabelElements[1].click();
innerButtons[1].click();
fixture.detectChanges();

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

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

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

buttonToggleLabelElements[0].click();
innerButtons[0].click();
fixture.detectChanges();
tick();
expect(changeSpy).toHaveBeenCalledTimes(1);

buttonToggleLabelElements[0].click();
innerButtons[0].click();
fixture.detectChanges();
tick();

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

buttonToggleLabelElements[0].click();
innerButtons[0].click();
fixture.detectChanges();
tick();
expect(changeSpy).toHaveBeenCalled();

buttonToggleLabelElements[1].click();
innerButtons[1].click();
fixture.detectChanges();
tick();
expect(changeSpy).toHaveBeenCalledTimes(2);
Expand Down Expand Up @@ -579,7 +583,7 @@ describe('MatButtonToggle without forms', () => {

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

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

it('should show checkmark indicator by default', () => {
buttonToggleLabelElements[0].click();
innerButtons[0].click();
fixture.detectChanges();

expect(
Expand Down Expand Up @@ -1310,3 +1314,18 @@ class ButtonToggleGroupWithFormControlAndDynamicButtons {
control = new FormControl('');
values = ['a', 'b', 'c'];
}

@Component({
template: `
<mat-button-toggle-group [(ngModel)]="value">
<mat-button-toggle value="1">One</mat-button-toggle>
<mat-button-toggle value="2">Two</mat-button-toggle>
<mat-button-toggle value="3">Three</mat-button-toggle>
</mat-button-toggle-group>
`,
standalone: true,
imports: [MatButtonToggleModule, FormsModule],
})
class ButtonToggleGroupWithNgModelAndStaticOptions {
value = '';
}
48 changes: 30 additions & 18 deletions src/material/button-toggle/button-toggle.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,16 +473,28 @@ export class MatButtonToggleGroup implements ControlValueAccessor, OnInit, After
return;
}

const toggles = this._buttonToggles.toArray();

if (this.multiple && value) {
if (!Array.isArray(value) && (typeof ngDevMode === 'undefined' || ngDevMode)) {
throw Error('Value must be an array in multiple-selection mode.');
}

this._clearSelection();
value.forEach((currentValue: any) => this._selectValue(currentValue));
value.forEach((currentValue: any) => this._selectValue(currentValue, toggles));
} else {
this._clearSelection();
this._selectValue(value);
this._selectValue(value, toggles);
}

// In single selection mode we need at least one enabled toggle to always be focusable.
if (!this.multiple && toggles.every(toggle => toggle.tabIndex === -1)) {
for (const toggle of toggles) {
if (!toggle.disabled) {
toggle.tabIndex = 0;
break;
}
}
}
}

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

/** Selects a value if there's a toggle that corresponds to it. */
private _selectValue(value: any) {
const correspondingOption = this._buttonToggles.find(toggle => {
return toggle.value != null && toggle.value === value;
});

if (correspondingOption) {
correspondingOption.checked = true;
this._selectionModel.select(correspondingOption);
if (!this.multiple) {
// If the button toggle is in single select mode, reset the tabIndex.
correspondingOption.tabIndex = 0;
private _selectValue(value: any, toggles: MatButtonToggle[]) {
for (const toggle of toggles) {
if (toggle.value != null && toggle.value === value) {
toggle.checked = true;
this._selectionModel.select(toggle);
if (!this.multiple) {
// If the button toggle is in single select mode, reset the tabIndex.
toggle.tabIndex = 0;
}
break;
}
}
}
Expand Down Expand Up @@ -601,8 +612,10 @@ export class MatButtonToggle implements OnInit, AfterViewInit, OnDestroy {
return this._tabIndex;
}
set tabIndex(value: number | null) {
this._tabIndex = value;
this._markForCheck();
if (value !== this._tabIndex) {
this._tabIndex = value;
this._markForCheck();
}
}
private _tabIndex: number | null;

Expand Down Expand Up @@ -668,14 +681,13 @@ export class MatButtonToggle implements OnInit, AfterViewInit, OnDestroy {
constructor() {
inject(_CdkPrivateStyleLoader).load(_StructuralStylesLoader);
const toggleGroup = inject<MatButtonToggleGroup>(MAT_BUTTON_TOGGLE_GROUP, {optional: true})!;
const defaultTabIndex = inject(new HostAttributeToken('tabindex'), {optional: true});
const defaultTabIndex = inject(new HostAttributeToken('tabindex'), {optional: true}) || '';
const defaultOptions = inject<MatButtonToggleDefaultOptions>(
MAT_BUTTON_TOGGLE_DEFAULT_OPTIONS,
{optional: true},
);

const parsedTabIndex = Number(defaultTabIndex);
this.tabIndex = parsedTabIndex || parsedTabIndex === 0 ? parsedTabIndex : null;
this._tabIndex = parseInt(defaultTabIndex) || 0;
this.buttonToggleGroup = toggleGroup;
this.appearance =
defaultOptions && defaultOptions.appearance ? defaultOptions.appearance : 'standard';
Expand Down
Loading