Skip to content

fix(chips): unable to tab out of chip list #4605

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.

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

Merged
merged 1 commit into from
May 19, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 16 additions & 5 deletions src/lib/chips/chip-list.spec.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import {async, ComponentFixture, TestBed} from '@angular/core/testing';
import {async, ComponentFixture, TestBed, fakeAsync, tick} from '@angular/core/testing';
import {Component, DebugElement, QueryList} from '@angular/core';
import {By} from '@angular/platform-browser';
import {MdChip, MdChipList, MdChipsModule} from './index';
import {FocusKeyManager} from '../core/a11y/focus-key-manager';
import {FakeEvent} from '../core/a11y/list-key-manager.spec';
import {SPACE, LEFT_ARROW, RIGHT_ARROW} from '../core/keyboard/keycodes';
import {SPACE, LEFT_ARROW, RIGHT_ARROW, TAB} from '../core/keyboard/keycodes';
import {createKeyboardEvent} from '../core/testing/event-objects';


class FakeKeyboardEvent extends FakeEvent {
constructor(keyCode: number, protected target: HTMLElement) {
Expand All @@ -26,9 +28,7 @@ describe('MdChipList', () => {
beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [MdChipsModule],
declarations: [
StaticChipList
]
declarations: [StaticChipList]
});

TestBed.compileComponents();
Expand Down Expand Up @@ -189,6 +189,17 @@ describe('MdChipList', () => {
expect(testComponent.chipDeselect).toHaveBeenCalledTimes(1);
expect(testComponent.chipDeselect).toHaveBeenCalledWith(0);
});

it('allow focus to escape when tabbing away', fakeAsync(() => {
chipListInstance._keyManager.onKeydown(createKeyboardEvent('keydown', TAB));

expect(chipListInstance._tabIndex)
.toBe(-1, 'Expected tabIndex to be set to -1 temporarily.');

tick();

expect(chipListInstance._tabIndex).toBe(0, 'Expected tabIndex to be reset back to 0');
}));
});

describe('when selectable is false', () => {
Expand Down
36 changes: 26 additions & 10 deletions src/lib/chips/chip-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@ import {
ChangeDetectionStrategy,
Component,
ContentChildren,
ElementRef,
Input,
QueryList,
ViewEncapsulation
ViewEncapsulation,
OnDestroy,
} from '@angular/core';

import {MdChip} from './chip';
import {FocusKeyManager} from '../core/a11y/focus-key-manager';
import {coerceBooleanProperty} from '../core/coercion/boolean-property';
import {SPACE, LEFT_ARROW, RIGHT_ARROW} from '../core/keyboard/keycodes';
import {SPACE, LEFT_ARROW, RIGHT_ARROW, TAB} from '../core/keyboard/keycodes';
import {Subscription} from 'rxjs/Subscription';

/**
* A material design chips component (named ChipList for it's similarity to the List component).
Expand All @@ -30,7 +31,7 @@ import {SPACE, LEFT_ARROW, RIGHT_ARROW} from '../core/keyboard/keycodes';
template: `<div class="mat-chip-list-wrapper"><ng-content></ng-content></div>`,
host: {
// Properties
'tabindex': '0',
'[attr.tabindex]': '_tabIndex',
'role': 'listbox',
'[class.mat-chip-list]': 'true',

Expand All @@ -45,11 +46,14 @@ import {SPACE, LEFT_ARROW, RIGHT_ARROW} from '../core/keyboard/keycodes';
encapsulation: ViewEncapsulation.None,
changeDetection: ChangeDetectionStrategy.OnPush
})
export class MdChipList implements AfterContentInit {
export class MdChipList implements AfterContentInit, OnDestroy {

/** Track which chips we're listening to for focus/destruction. */
private _subscribed: WeakMap<MdChip, boolean> = new WeakMap();

/** Subscription to tabbing out from the chip list. */
private _tabOutSubscription: Subscription;

/** Whether or not the chip is selectable. */
protected _selectable: boolean = true;

Expand All @@ -59,11 +63,19 @@ export class MdChipList implements AfterContentInit {
/** The chip components contained within this chip list. */
chips: QueryList<MdChip>;

constructor(private _elementRef: ElementRef) { }
/** Tab index for the chip list. */
_tabIndex = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why Shift+tab doesn't work on tabindex=0?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because the browsers determine that the previous tabbable element is the parent (since the chips are tabindex="-1" and the chip list is tabindex="0"). When the parent receives focus, it forwards it back to the first chip. As an aside, Firefox also tabs into the parent when tabbing forward.


ngAfterContentInit(): void {
this._keyManager = new FocusKeyManager(this.chips).withWrap();

// Prevents the chip list from capturing focus and redirecting
// it back to the first chip when the user tabs out.
this._tabOutSubscription = this._keyManager.tabOut.subscribe(() => {
this._tabIndex = -1;
setTimeout(() => this._tabIndex = 0);
});

// Go ahead and subscribe all of the initial chips
this._subscribeChips(this.chips);

Expand All @@ -73,14 +85,18 @@ export class MdChipList implements AfterContentInit {
});
}

ngOnDestroy(): void {
if (this._tabOutSubscription) {
this._tabOutSubscription.unsubscribe();
}
}

/**
* Whether or not this chip is selectable. When a chip is not selectable,
* it's selected state is always ignored.
*/
@Input() get selectable(): boolean {
return this._selectable;
}

@Input()
get selectable(): boolean { return this._selectable; }
set selectable(value: boolean) {
this._selectable = coerceBooleanProperty(value);
}
Expand Down