Skip to content

Commit 8304a94

Browse files
authored
fix(material/list): fix tabindex="-1" not being maintained when disabled (#25860)
When entire list is disabled, avoid adding list options to the tab order in the following situations: - focusin event is triggered on an option. This can happen when the end user clicks on a list option. - ngOnInit happens when list is disabled - updating selection state When list is disabled tabindex of every option should always be -1. Tabindex was set to 0 mainly because `_resetActionOption` only considered when list options where disabled, but didn't consider when the list itself is disabled. Fix issues by setting tabindex to -1 when disabled and making `_handleFocusin` a noop when disabled.
1 parent b1b720c commit 8304a94

File tree

2 files changed

+36
-3
lines changed

2 files changed

+36
-3
lines changed

src/material/list/selection-list.spec.ts

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -806,8 +806,9 @@ describe('MDC-based MatSelectionList without forms', () => {
806806
});
807807

808808
// when the entire list is disabled, its listitems should always have tabindex="-1"
809-
it('should not put listitems in the tab order', () => {
809+
it('should remove all listitems from the tab order when disabled state is enabled', () => {
810810
fixture.componentInstance.disabled = false;
811+
fixture.detectChanges();
811812
let testListItem = listOption[2].injector.get<MatListOption>(MatListOption);
812813
testListItem.focus();
813814
fixture.detectChanges();
@@ -827,6 +828,26 @@ describe('MDC-based MatSelectionList without forms', () => {
827828
.withContext('Expected all list options to be excluded from the tab order')
828829
.toBe(0);
829830
});
831+
832+
it('should not allow focusin event to change the tabindex', () => {
833+
fixture.componentInstance.disabled = true;
834+
fixture.detectChanges();
835+
836+
expect(
837+
listOption.filter(option => option.nativeElement.getAttribute('tabindex') !== '-1').length,
838+
)
839+
.withContext('Expected all list options to be excluded from the tab order')
840+
.toBe(0);
841+
842+
listOption[1].triggerEventHandler('focusin', {target: listOption[1].nativeElement});
843+
fixture.detectChanges();
844+
845+
expect(
846+
listOption.filter(option => option.nativeElement.getAttribute('tabindex') !== '-1').length,
847+
)
848+
.withContext('Expected all list options to be excluded from the tab order')
849+
.toBe(0);
850+
});
830851
});
831852

832853
describe('with checkbox position after', () => {

src/material/list/selection-list.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,10 @@ export class MatSelectionList
373373

374374
/** Handles focusin events within the list. */
375375
private _handleFocusin = (event: FocusEvent) => {
376+
if (this.disabled) {
377+
return;
378+
}
379+
376380
const activeIndex = this._items
377381
.toArray()
378382
.findIndex(item => item._elementRef.nativeElement.contains(event.target as HTMLElement));
@@ -402,7 +406,7 @@ export class MatSelectionList
402406
.withHomeAndEnd()
403407
.withTypeAhead()
404408
.withWrap()
405-
.skipPredicate(() => false);
409+
.skipPredicate(() => this.disabled);
406410

407411
// Set the initial focus.
408412
this._resetActiveOption();
@@ -429,8 +433,16 @@ export class MatSelectionList
429433
this._keyManager.updateActiveItem(index);
430434
}
431435

432-
/** Resets the active option to the first selected option. */
436+
/**
437+
* Resets the active option. When the list is disabled, remove all options from to the tab order.
438+
* Otherwise, focus the first selected option.
439+
*/
433440
private _resetActiveOption() {
441+
if (this.disabled) {
442+
this._setActiveOption(-1);
443+
return;
444+
}
445+
434446
const activeItem =
435447
this._items.find(item => item.selected && !item.disabled) || this._items.first;
436448
this._setActiveOption(activeItem ? this._items.toArray().indexOf(activeItem) : -1);

0 commit comments

Comments
 (0)