Skip to content

Commit fcee873

Browse files
committed
fix(checkbox): Cancel spacebar keydown events when component focused
This commit ensures that when a checkbox element is focused, spacebar `keydown` events will not bubble up to window, causing side effects like page scrolling. This commit also fixes an issue as part of this change where the component could still be focused even when it was disabled. Finally, some of the test code has been cleaned up and corrected. Fixes #162
1 parent 548b5b5 commit fcee873

File tree

2 files changed

+139
-9
lines changed

2 files changed

+139
-9
lines changed

src/components/checkbox/checkbox.spec.ts

Lines changed: 103 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -289,9 +289,9 @@ export function main() {
289289
expect(el.nativeElement.className).toContain('md-checkbox-disabled');
290290
});
291291

292-
it('sets the tabindex to -1 on the host element', function() {
292+
it('removes the tabindex attribute from the host element', function() {
293293
let el = fixture.debugElement.query(By.css('.md-checkbox'));
294-
expect(el.nativeElement.getAttribute('tabindex')).toEqual('-1');
294+
expect(el.nativeElement.hasAttribute('tabindex')).toBe(false);
295295
});
296296

297297
it('sets "aria-disabled" to "true" on the host element', function() {
@@ -307,7 +307,7 @@ export function main() {
307307
tabindexController.isDisabled = true;
308308
fixture.detectChanges();
309309
let el = fixture.debugElement.query(By.css('.md-checkbox'));
310-
expect(el.nativeElement.getAttribute('tabindex')).toEqual('-1');
310+
expect(el.nativeElement.hasAttribute('tabindex')).toBe(false);
311311

312312
tabindexController.isDisabled = false;
313313
fixture.detectChanges();
@@ -334,9 +334,9 @@ export function main() {
334334
}).then(done).catch(done);
335335
});
336336

337-
it('keeps the tabindex at -1', function() {
337+
it('keeps the tabindex removed from the host', function() {
338338
let el = fixture.debugElement.query(By.css('.md-checkbox'));
339-
expect(el.nativeElement.getAttribute('tabindex')).toEqual('-1');
339+
expect(el.nativeElement.hasAttribute('tabindex')).toBe(false);
340340
});
341341

342342
it('uses the newly changed tabindex when re-enabled', function() {
@@ -394,6 +394,102 @@ export function main() {
394394
});
395395
});
396396

397+
describe('when the checkbox is focused', function() {
398+
var browserSupportsDom3KeyProp: boolean = ({}).hasOwnProperty.call(
399+
KeyboardEvent.prototype, 'key');
400+
var fixture: ComponentFixture;
401+
var controller: CheckboxController;
402+
var el: DebugElement;
403+
var windowListenerSpy: jasmine.Spy;
404+
var windowKeydownListeners: EventListener[];
405+
406+
function keydown(target: EventTarget, key: string, keyCode: number): Event {
407+
var evt: Event;
408+
if (BROWSER_SUPPORTS_EVENT_CONSTRUCTORS && browserSupportsDom3KeyProp) {
409+
evt = new KeyboardEvent('keydown', {key});
410+
} else {
411+
// NOTE(traviskaufman): Chrome/Webkit-based browsers do not support any mechanism for
412+
// creating keyboard events with the correct keyCode. Therefore, the object.defineProperty
413+
// hack is the only way to make this work correctly.
414+
// see: https://bugs.webkit.org/show_bug.cgi?id=16735
415+
// see: https://bugs.chromium.org/p/chromium/issues/detail?id=227231
416+
// see: http://goo.gl/vrh534 (stackoverflow post on the topic).
417+
evt = document.createEvent('Event');
418+
evt.initEvent('keydown', true, true);
419+
Object.defineProperty(evt, 'keyCode', {
420+
value: keyCode,
421+
enumerable: true,
422+
writable: false,
423+
configurable: true
424+
});
425+
}
426+
spyOn(evt, 'preventDefault').and.callThrough();
427+
target.dispatchEvent(evt);
428+
return evt;
429+
};
430+
431+
function dispatchUIEventOnEl(evtName: string) {
432+
var evt: Event;
433+
if (BROWSER_SUPPORTS_EVENT_CONSTRUCTORS) {
434+
evt = new Event(evtName);
435+
} else {
436+
evt = document.createEvent('Event');
437+
evt.initEvent(evtName, true, true);
438+
}
439+
el.nativeElement.dispatchEvent(evt);
440+
fixture.detectChanges();
441+
}
442+
443+
beforeEach(function(done: () => void) {
444+
var windowAddEventListener = window.addEventListener.bind(window);
445+
windowKeydownListeners = [];
446+
windowListenerSpy = spyOn(window, 'addEventListener').and.callFake(
447+
function(name: string, listener: EventListener) {
448+
if (name === 'keydown') {
449+
windowKeydownListeners.push(listener);
450+
}
451+
windowAddEventListener(name, listener);
452+
});
453+
454+
builder.createAsync(CheckboxController).then(function(f) {
455+
fixture = f;
456+
controller = fixture.componentInstance;
457+
458+
fixture.detectChanges();
459+
el = fixture.debugElement.query(By.css('.md-checkbox'));
460+
}).then(done).catch(done);
461+
});
462+
463+
afterEach(function() {
464+
windowKeydownListeners.forEach((l) => window.removeEventListener('keydown', l));
465+
});
466+
467+
it('blocks spacebar keydown events from performing their default behavior', function() {
468+
dispatchUIEventOnEl('focus');
469+
470+
var evt = keydown(window, ' ', 32);
471+
expect(evt.preventDefault).toHaveBeenCalled();
472+
});
473+
474+
it('does not block other keyboard events from performing their default behavior', function() {
475+
dispatchUIEventOnEl('focus');
476+
477+
var evt = keydown(window, 'Tab', 9);
478+
expect(evt.preventDefault).not.toHaveBeenCalled();
479+
});
480+
481+
it('stops blocking spacebar keydown events when un-focused', function() {
482+
dispatchUIEventOnEl('focus');
483+
// sanity check.
484+
var evt = keydown(window, ' ', 32);
485+
expect(evt.preventDefault).toHaveBeenCalled();
486+
487+
dispatchUIEventOnEl('blur');
488+
evt = keydown(window, ' ', 32);
489+
expect(evt.preventDefault).not.toHaveBeenCalled();
490+
});
491+
});
492+
397493
describe('when a spacebar press occurs on the checkbox', function() {
398494
var fixture: ComponentFixture;
399495
var controller: CheckboxController;
@@ -602,10 +698,11 @@ function keyup(el: DebugElement, key: string, fixture: ComponentFixture) {
602698
if (BROWSER_SUPPORTS_EVENT_CONSTRUCTORS) {
603699
kbdEvent = new KeyboardEvent('keyup');
604700
} else {
605-
kbdEvent = document.createEvent('Events');
701+
kbdEvent = document.createEvent('Event');
606702
kbdEvent.initEvent('keyup', true, true);
607703
}
608704
// Hack DOM Level 3 Events "key" prop into keyboard event.
705+
// See note above for explanation of this defineProperty hack.
609706
Object.defineProperty(kbdEvent, 'key', {
610707
value: ' ',
611708
enumerable: false,

src/components/checkbox/checkbox.ts

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,18 @@ enum TransitionCheckState {
6868
'[class.md-checkbox-checked]': 'checked',
6969
'[class.md-checkbox-disabled]': 'disabled',
7070
'[class.md-checkbox-align-end]': 'align == "end"',
71-
'[tabindex]': 'disabled ? -1 : tabindex',
71+
'[attr.tabindex]': 'disabled ? null : tabindex',
7272
'[attr.aria-label]': 'ariaLabel',
7373
'[attr.aria-labelledby]': 'labelId',
7474
'[attr.aria-checked]': 'getAriaChecked()',
7575
'[attr.aria-disabled]': 'disabled',
7676
'(click)': 'onInteractionEvent($event)',
7777
'(keyup.space)': 'onInteractionEvent($event)',
78-
'(blur)': 'onTouched()'
78+
'(focus)': 'onFocus($event)',
79+
'(blur)': 'onBlur()',
80+
// TODO(traviskaufman): Use (window:keydown.space) if/when it's supported.
81+
// See: https://github.com/angular/angular/issues/7308
82+
'(window:keydown)': 'onWindowKeyDown($event)'
7983
},
8084
providers: [MD_CHECKBOX_CONTROL_VALUE_ACCESSOR],
8185
encapsulation: ViewEncapsulation.None,
@@ -102,7 +106,7 @@ export class MdCheckbox implements ControlValueAccessor {
102106

103107
/**
104108
* The tabindex attribute for the checkbox. Note that when the checkbox is disabled, the attribute
105-
* on the host element will be set to -1, regardless of the actual tabindex value.
109+
* on the host element will be removed. It will be placed back when the checkbox is re-enabled.
106110
*/
107111
@Input() tabindex: number = 0;
108112

@@ -120,6 +124,8 @@ export class MdCheckbox implements ControlValueAccessor {
120124

121125
private _indeterminate: boolean = false;
122126

127+
private _isFocused: boolean = false;
128+
123129
private _changeSubscription: {unsubscribe: () => any} = null;
124130

125131
constructor(private _renderer: Renderer, private _elementRef: ElementRef) {}
@@ -192,6 +198,33 @@ export class MdCheckbox implements ControlValueAccessor {
192198
this.toggle();
193199
}
194200

201+
/**
202+
* Event handler used for (focus) events.
203+
*/
204+
onFocus(evt: Event) {
205+
this._isFocused = true;
206+
}
207+
208+
/**
209+
* Event handler used for (blur) events.
210+
*/
211+
onBlur() {
212+
this._isFocused = false;
213+
this.onTouched();
214+
}
215+
216+
/**
217+
* Event handler used for (keydown.space) events on the window. Used to prevent spacebar events
218+
* from bubbling when the component is focused, which prevents side effects like page scrolling
219+
* from happening.
220+
*/
221+
onWindowKeyDown(evt: KeyboardEvent) {
222+
let wasSpacePressed = evt.key && evt.key === ' ' || evt.keyCode === 32;
223+
if (this._isFocused && wasSpacePressed) {
224+
evt.preventDefault();
225+
}
226+
}
227+
195228
/** Implemented as part of ControlValueAccessor. */
196229
writeValue(value: any) {
197230
this.checked = !!value;

0 commit comments

Comments
 (0)