Skip to content

Commit

Permalink
fix(material/list): fix tabindex="-1" not being maintained when disab…
Browse files Browse the repository at this point in the history
…led (#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.

(cherry picked from commit 8304a94)
  • Loading branch information
zarend committed Oct 28, 2022
1 parent 63853af commit 8baae73
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 3 deletions.
23 changes: 22 additions & 1 deletion src/material/list/selection-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -806,8 +806,9 @@ describe('MDC-based MatSelectionList without forms', () => {
});

// when the entire list is disabled, its listitems should always have tabindex="-1"
it('should not put listitems in the tab order', () => {
it('should remove all listitems from the tab order when disabled state is enabled', () => {
fixture.componentInstance.disabled = false;
fixture.detectChanges();
let testListItem = listOption[2].injector.get<MatListOption>(MatListOption);
testListItem.focus();
fixture.detectChanges();
Expand All @@ -827,6 +828,26 @@ describe('MDC-based MatSelectionList without forms', () => {
.withContext('Expected all list options to be excluded from the tab order')
.toBe(0);
});

it('should not allow focusin event to change the tabindex', () => {
fixture.componentInstance.disabled = true;
fixture.detectChanges();

expect(
listOption.filter(option => option.nativeElement.getAttribute('tabindex') !== '-1').length,
)
.withContext('Expected all list options to be excluded from the tab order')
.toBe(0);

listOption[1].triggerEventHandler('focusin', {target: listOption[1].nativeElement});
fixture.detectChanges();

expect(
listOption.filter(option => option.nativeElement.getAttribute('tabindex') !== '-1').length,
)
.withContext('Expected all list options to be excluded from the tab order')
.toBe(0);
});
});

describe('with checkbox position after', () => {
Expand Down
16 changes: 14 additions & 2 deletions src/material/list/selection-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,10 @@ export class MatSelectionList

/** Handles focusin events within the list. */
private _handleFocusin = (event: FocusEvent) => {
if (this.disabled) {
return;
}

const activeIndex = this._items
.toArray()
.findIndex(item => item._elementRef.nativeElement.contains(event.target as HTMLElement));
Expand Down Expand Up @@ -402,7 +406,7 @@ export class MatSelectionList
.withHomeAndEnd()
.withTypeAhead()
.withWrap()
.skipPredicate(() => false);
.skipPredicate(() => this.disabled);

// Set the initial focus.
this._resetActiveOption();
Expand All @@ -429,8 +433,16 @@ export class MatSelectionList
this._keyManager.updateActiveItem(index);
}

/** Resets the active option to the first selected option. */
/**
* Resets the active option. When the list is disabled, remove all options from to the tab order.
* Otherwise, focus the first selected option.
*/
private _resetActiveOption() {
if (this.disabled) {
this._setActiveOption(-1);
return;
}

const activeItem =
this._items.find(item => item.selected && !item.disabled) || this._items.first;
this._setActiveOption(activeItem ? this._items.toArray().indexOf(activeItem) : -1);
Expand Down

0 comments on commit 8baae73

Please sign in to comment.