From ac84e25b618e860e91d9b694918becac16492373 Mon Sep 17 00:00:00 2001 From: Travis Kaufman Date: Wed, 16 Mar 2016 11:36:40 -0400 Subject: [PATCH] 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 --- src/components/checkbox/checkbox.spec.ts | 71 ++++++++++++++++++++---- src/components/checkbox/checkbox.ts | 13 ++++- 2 files changed, 72 insertions(+), 12 deletions(-) diff --git a/src/components/checkbox/checkbox.spec.ts b/src/components/checkbox/checkbox.spec.ts index 3abe395b2356..f25d382c0323 100644 --- a/src/components/checkbox/checkbox.spec.ts +++ b/src/components/checkbox/checkbox.spec.ts @@ -289,9 +289,9 @@ export function main() { expect(el.nativeElement.className).toContain('md-checkbox-disabled'); }); - it('sets the tabindex to -1 on the host element', function() { + it('removes the tabindex attribute from the host element', function() { let el = fixture.debugElement.query(By.css('.md-checkbox')); - expect(el.nativeElement.getAttribute('tabindex')).toEqual('-1'); + expect(el.nativeElement.hasAttribute('tabindex')).toBe(false); }); it('sets "aria-disabled" to "true" on the host element', function() { @@ -307,7 +307,7 @@ export function main() { tabindexController.isDisabled = true; fixture.detectChanges(); let el = fixture.debugElement.query(By.css('.md-checkbox')); - expect(el.nativeElement.getAttribute('tabindex')).toEqual('-1'); + expect(el.nativeElement.hasAttribute('tabindex')).toBe(false); tabindexController.isDisabled = false; fixture.detectChanges(); @@ -334,9 +334,9 @@ export function main() { }).then(done).catch(done); }); - it('keeps the tabindex at -1', function() { + it('keeps the tabindex removed from the host', function() { let el = fixture.debugElement.query(By.css('.md-checkbox')); - expect(el.nativeElement.getAttribute('tabindex')).toEqual('-1'); + expect(el.nativeElement.hasAttribute('tabindex')).toBe(false); }); it('uses the newly changed tabindex when re-enabled', function() { @@ -394,6 +394,48 @@ export function main() { }); }); + describe('when the checkbox is focused', function() { + var fixture: ComponentFixture; + var controller: CheckboxController; + var el: DebugElement; + + function dispatchUIEventOnEl(evtName: string) { + var evt: Event; + if (BROWSER_SUPPORTS_EVENT_CONSTRUCTORS) { + evt = new Event(evtName); + } else { + evt = document.createEvent('Event'); + evt.initEvent(evtName, true, true); + } + el.nativeElement.dispatchEvent(evt); + fixture.detectChanges(); + } + + beforeEach(function(done: () => void) { + builder.createAsync(CheckboxController).then(function(f) { + fixture = f; + controller = fixture.componentInstance; + + fixture.detectChanges(); + el = fixture.debugElement.query(By.css('.md-checkbox')); + }).then(done).catch(done); + }); + + it('blocks spacebar keydown events from performing their default behavior', function() { + dispatchUIEventOnEl('focus'); + + var evt = keydown(el, ' ', fixture); + expect(evt.preventDefault).toHaveBeenCalled(); + }); + + it('does not block other keyboard events from performing their default behavior', function() { + dispatchUIEventOnEl('focus'); + + var evt = keydown(el, 'Tab', fixture); + expect(evt.preventDefault).not.toHaveBeenCalled(); + }); + }); + describe('when a spacebar press occurs on the checkbox', function() { var fixture: ComponentFixture; var controller: CheckboxController; @@ -597,27 +639,36 @@ function click(el: DebugElement, fixture: ComponentFixture) { } // TODO(traviskaufman): Reinvestigate implementation of this method once tests in Dart begin to run. -function keyup(el: DebugElement, key: string, fixture: ComponentFixture) { +function keyEvent(name: string, el: DebugElement, key: string, fixture: ComponentFixture): Event { var kbdEvent: Event; if (BROWSER_SUPPORTS_EVENT_CONSTRUCTORS) { - kbdEvent = new KeyboardEvent('keyup'); + kbdEvent = new KeyboardEvent(name); } else { - kbdEvent = document.createEvent('Events'); - kbdEvent.initEvent('keyup', true, true); + kbdEvent = document.createEvent('Event'); + kbdEvent.initEvent(name, true, true); } // Hack DOM Level 3 Events "key" prop into keyboard event. Object.defineProperty(kbdEvent, 'key', { - value: ' ', + value: key, enumerable: false, writable: false, configurable: true }); + spyOn(kbdEvent, 'preventDefault').and.callThrough(); spyOn(kbdEvent, 'stopPropagation').and.callThrough(); el.nativeElement.dispatchEvent(kbdEvent); fixture.detectChanges(); return kbdEvent; } +function keydown(el: DebugElement, key: string, fixture: ComponentFixture): Event { + return keyEvent('keydown', el, key, fixture); +} + +function keyup(el: DebugElement, key: string, fixture: ComponentFixture): Event { + return keyEvent('keyup', el, key, fixture); +} + @Component({ selector: 'checkbox-controller', template: ` diff --git a/src/components/checkbox/checkbox.ts b/src/components/checkbox/checkbox.ts index 0c8c06e6d100..0ddead691e23 100644 --- a/src/components/checkbox/checkbox.ts +++ b/src/components/checkbox/checkbox.ts @@ -68,12 +68,13 @@ enum TransitionCheckState { '[class.md-checkbox-checked]': 'checked', '[class.md-checkbox-disabled]': 'disabled', '[class.md-checkbox-align-end]': 'align == "end"', - '[tabindex]': 'disabled ? -1 : tabindex', + '[attr.tabindex]': 'disabled ? null : tabindex', '[attr.aria-label]': 'ariaLabel', '[attr.aria-labelledby]': 'labelId', '[attr.aria-checked]': 'getAriaChecked()', '[attr.aria-disabled]': 'disabled', '(click)': 'onInteractionEvent($event)', + '(keydown.space)': 'onSpaceDown($event)', '(keyup.space)': 'onInteractionEvent($event)', '(blur)': 'onTouched()' }, @@ -102,7 +103,7 @@ export class MdCheckbox implements ControlValueAccessor { /** * The tabindex attribute for the checkbox. Note that when the checkbox is disabled, the attribute - * on the host element will be set to -1, regardless of the actual tabindex value. + * on the host element will be removed. It will be placed back when the checkbox is re-enabled. */ @Input() tabindex: number = 0; @@ -192,6 +193,14 @@ export class MdCheckbox implements ControlValueAccessor { this.toggle(); } + /** + * Event handler used for (keydown.space) events. Used to prevent spacebar events from bubbling + * when the component is focused, which prevents side effects like page scrolling from happening. + */ + onSpaceDown(evt: Event) { + evt.preventDefault(); + } + /** Implemented as part of ControlValueAccessor. */ writeValue(value: any) { this.checked = !!value;