Skip to content

Commit

Permalink
fix(selection-list): tabIndex should respect disabled state
Browse files Browse the repository at this point in the history
* Currently if the selection-list is disabled, the tabIndex may be still set to a valid value that allows tabbing to the element. The `mixinTabIndex` respects the disabled state of the selection list.
  • Loading branch information
devversion committed Sep 13, 2017
1 parent dcfe515 commit 30a6efc
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 13 deletions.
56 changes: 56 additions & 0 deletions src/lib/list/selection-list.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,49 @@ describe('MdSelectionList', () => {
});
});

describe('with tabindex', () => {

beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [MdListModule],
declarations: [
SelectionListWithTabindexAttr,
SelectionListWithTabindexBinding,
]
});

TestBed.compileComponents();
}));

it('should properly handle native tabindex attribute', () => {
const fixture = TestBed.createComponent(SelectionListWithTabindexAttr);
const selectionList = fixture.debugElement.query(By.directive(MdSelectionList));

expect(selectionList.componentInstance.tabIndex)
.toBe(5, 'Expected the selection-list tabindex to be set to the attribute value.');
});

it('should support changing the tabIndex through binding', () => {
const fixture = TestBed.createComponent(SelectionListWithTabindexBinding);
const selectionList = fixture.debugElement.query(By.directive(MdSelectionList));

expect(selectionList.componentInstance.tabIndex)
.toBe(0, 'Expected the tabIndex to be set to "0" by default.');

fixture.componentInstance.tabIndex = 3;
fixture.detectChanges();

expect(selectionList.componentInstance.tabIndex)
.toBe(3, 'Expected the tabIndex to updated through binding.');

fixture.componentInstance.disabled = true;
fixture.detectChanges();

expect(selectionList.componentInstance.tabIndex)
.toBe(-1, 'Expected the tabIndex to be set to "-1" if selection list is disabled.');
});
});

describe('with single option', () => {
let fixture: ComponentFixture<SelectionListWithOnlyOneOption>;
let listOption: DebugElement;
Expand Down Expand Up @@ -458,3 +501,16 @@ class SelectionListWithDisabledOption {
</mat-selection-list>`})
class SelectionListWithOnlyOneOption {
}

@Component({
template: `<mat-selection-list tabindex="5"></mat-selection-list>`
})
class SelectionListWithTabindexAttr {}

@Component({
template: `<mat-selection-list [tabIndex]="tabIndex" [disabled]="disabled"></mat-selection-list>`
})
class SelectionListWithTabindexBinding {
tabIndex: number;
disabled: boolean;
}
24 changes: 11 additions & 13 deletions src/lib/list/selection-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
OnDestroy,
forwardRef,
Inject,
Attribute,
} from '@angular/core';
import {coerceBooleanProperty, SelectionModel, MdLine, MdLineSetter} from '../core';
import {FocusKeyManager} from '../core/a11y/focus-key-manager';
Expand All @@ -34,11 +35,13 @@ import {RxChain, switchMap, startWith} from '../core/rxjs/index';
import {merge} from 'rxjs/observable/merge';
import {CanDisableRipple, mixinDisableRipple} from '../core/common-behaviors/disable-ripple';
import {MATERIAL_COMPATIBILITY_MODE} from '../core/compatibility/compatibility';
import {mixinTabIndex, HasTabIndex} from '../core/common-behaviors/tabindex';


/** @docs-private */
export class MdSelectionListBase {}
export const _MdSelectionListMixinBase = mixinDisableRipple(mixinDisabled(MdSelectionListBase));
export const _MdSelectionListMixinBase =
mixinTabIndex(mixinDisableRipple(mixinDisabled(MdSelectionListBase)));

/** @docs-private */
export class MdListOptionBase {}
Expand Down Expand Up @@ -182,10 +185,10 @@ export class MdListOption extends _MdListOptionMixinBase
@Component({
moduleId: module.id,
selector: 'md-selection-list, mat-selection-list',
inputs: ['disabled', 'disableRipple'],
inputs: ['disabled', 'disableRipple', 'tabIndex'],
host: {
'role': 'listbox',
'[attr.tabindex]': '_tabIndex',
'[tabIndex]': 'tabIndex',
'class': 'mat-selection-list',
'(focus)': 'focus()',
'(keydown)': '_keydown($event)',
Expand All @@ -195,11 +198,8 @@ export class MdListOption extends _MdListOptionMixinBase
encapsulation: ViewEncapsulation.None,
changeDetection: ChangeDetectionStrategy.OnPush
})
export class MdSelectionList extends _MdSelectionListMixinBase
implements FocusableOption, CanDisable, CanDisableRipple, AfterContentInit, OnDestroy {

/** Tab index for the selection-list. */
_tabIndex = 0;
export class MdSelectionList extends _MdSelectionListMixinBase implements FocusableOption,
CanDisable, CanDisableRipple, HasTabIndex, AfterContentInit, OnDestroy {

/** Subscription to all list options' onFocus events */
private _optionFocusSubscription = Subscription.EMPTY;
Expand All @@ -216,17 +216,15 @@ export class MdSelectionList extends _MdSelectionListMixinBase
/** The currently selected options. */
selectedOptions: SelectionModel<MdListOption> = new SelectionModel<MdListOption>(true);

constructor(private _element: ElementRef) {
constructor(private _element: ElementRef, @Attribute('tabindex') tabIndex: string) {
super();

this.tabIndex = parseInt(tabIndex) || 0;
}

ngAfterContentInit(): void {
this._keyManager = new FocusKeyManager<MdListOption>(this.options).withWrap();

if (this.disabled) {
this._tabIndex = -1;
}

this._optionFocusSubscription = this._onFocusSubscription();
this._optionDestroyStream = this._onDestroySubscription();
}
Expand Down

0 comments on commit 30a6efc

Please sign in to comment.