From 28b3c0d41b2c5ff4c6f9a22ac19dc44ef98f2993 Mon Sep 17 00:00:00 2001 From: Jeremy Elbourn Date: Tue, 8 Nov 2016 17:03:36 -0800 Subject: [PATCH] fix: disable ripples when parent component is disabled --- src/lib/button/button.html | 2 +- src/lib/button/button.spec.ts | 43 ++++++++++++++++++++++++------- src/lib/button/button.ts | 24 +++++++---------- src/lib/checkbox/checkbox.html | 2 +- src/lib/checkbox/checkbox.spec.ts | 11 ++++++++ src/lib/checkbox/checkbox.ts | 4 +++ src/lib/radio/radio.html | 2 +- src/lib/radio/radio.spec.ts | 11 ++++++++ src/lib/radio/radio.ts | 4 +++ 9 files changed, 76 insertions(+), 27 deletions(-) diff --git a/src/lib/button/button.html b/src/lib/button/button.html index a1636be45494..99e784458c79 100644 --- a/src/lib/button/button.html +++ b/src/lib/button/button.html @@ -1,5 +1,5 @@ -
{ // Ripple tests. describe('button ripples', () => { - it('should remove ripple if md-ripple-disabled input is set', () => { - let fixture = TestBed.createComponent(TestApp); - let testComponent = fixture.debugElement.componentInstance; - let buttonDebugElement = fixture.debugElement.query(By.css('button')); + let fixture: ComponentFixture; + let testComponent: TestApp; + let buttonElement: HTMLButtonElement; + let anchorElement: HTMLAnchorElement; + beforeEach(() => { + fixture = TestBed.createComponent(TestApp); fixture.detectChanges(); - expect(buttonDebugElement.nativeElement.querySelectorAll('[md-ripple]').length).toBe(1); + + testComponent = fixture.componentInstance; + buttonElement = fixture.nativeElement.querySelector('button[md-button]'); + anchorElement = fixture.nativeElement.querySelector('a[md-button]'); + }); + + it('should remove ripple if md-ripple-disabled input is set', () => { + expect(buttonElement.querySelectorAll('[md-ripple]').length).toBe(1); testComponent.rippleDisabled = true; fixture.detectChanges(); - expect(buttonDebugElement.nativeElement.querySelectorAll('[md-ripple]').length).toBe(0); + expect(buttonElement.querySelectorAll('[md-ripple]').length).toBe(0); + }); + + it('should not have a ripple when the button is disabled', () => { + let buttonRipple = buttonElement.querySelector('[md-ripple]'); + let anchorRipple = anchorElement.querySelector('[md-ripple]'); + + expect(buttonRipple).toBeTruthy('Expected an enabled button[md-button] to have a ripple'); + expect(anchorRipple).toBeTruthy('Expected an enabled a[md-button] to have a ripple'); + + testComponent.isDisabled = true; + fixture.detectChanges(); + + buttonRipple = buttonElement.querySelector('button [md-ripple]'); + anchorRipple = anchorElement.querySelector('a [md-ripple]'); + + expect(buttonRipple).toBeFalsy('Expected a disabled button[md-button] not to have a ripple'); + expect(anchorRipple).toBeFalsy('Expected a disabled a[md-button] not to have a ripple'); }); }); }); diff --git a/src/lib/button/button.ts b/src/lib/button/button.ts index 9763d851a706..ef280cbc4afc 100644 --- a/src/lib/button/button.ts +++ b/src/lib/button/button.ts @@ -21,6 +21,7 @@ import {MdRippleModule, coerceBooleanProperty} from '../core'; selector: 'button[md-button], button[md-raised-button], button[md-icon-button], ' + 'button[md-fab], button[md-mini-fab]', host: { + '[attr.disabled]': 'disabled', '[class.md-button-focus]': '_isKeyboardFocused', '(mousedown)': '_setMousedown()', '(focus)': '_setKeyboardFocus()', @@ -42,11 +43,16 @@ export class MdButton { /** Whether the ripple effect on click should be disabled. */ private _disableRipple: boolean = false; + private _disabled: boolean = false; @Input() get disableRipple() { return this._disableRipple; } set disableRipple(v) { this._disableRipple = coerceBooleanProperty(v); } + @Input() + get disabled() { return this._disabled; } + set disabled(value: boolean) { this._disabled = coerceBooleanProperty(value); } + constructor(private _elementRef: ElementRef, private _renderer: Renderer) { } @Input() @@ -103,16 +109,17 @@ export class MdButton { el.hasAttribute('md-mini-fab'); } - isRippleEnabled() { - return !this.disableRipple; + _isRippleDisabled() { + return this.disableRipple || this.disabled; } } @Component({ moduleId: module.id, selector: 'a[md-button], a[md-raised-button], a[md-icon-button], a[md-fab], a[md-mini-fab]', - inputs: ['color'], + inputs: ['color', 'disabled', 'disableRipple'], host: { + '[attr.disabled]': 'disabled', '[class.md-button-focus]': '_isKeyboardFocused', '(mousedown)': '_setMousedown()', '(focus)': '_setKeyboardFocus()', @@ -124,8 +131,6 @@ export class MdButton { encapsulation: ViewEncapsulation.None }) export class MdAnchor extends MdButton { - _disabled: boolean = null; - constructor(elementRef: ElementRef, renderer: Renderer) { super(elementRef, renderer); } @@ -141,15 +146,6 @@ export class MdAnchor extends MdButton { return this.disabled ? 'true' : 'false'; } - @HostBinding('attr.disabled') - @Input('disabled') - get disabled() { return this._disabled; } - - set disabled(value: boolean) { - // The presence of *any* disabled value makes the component disabled, *except* for false. - this._disabled = (value != null && value != false) ? true : null; - } - _haltDisabledEvents(event: Event) { // A disabled button shouldn't apply any actions if (this.disabled) { diff --git a/src/lib/checkbox/checkbox.html b/src/lib/checkbox/checkbox.html index cccad19f1cac..c53bc700f0ae 100644 --- a/src/lib/checkbox/checkbox.html +++ b/src/lib/checkbox/checkbox.html @@ -14,7 +14,7 @@ (blur)="_onInputBlur()" (change)="_onInteractionEvent($event)" (click)="_onInputClick($event)"> -
{ expect(inputElement.disabled).toBe(false); }); + it('should not have a ripple when disabled', () => { + let rippleElement = checkboxNativeElement.querySelector('[md-ripple]'); + expect(rippleElement).toBeTruthy('Expected an enabled checkbox to have a ripple'); + + testComponent.isDisabled = true; + fixture.detectChanges(); + + rippleElement = checkboxNativeElement.querySelector('[md-ripple]'); + expect(rippleElement).toBeFalsy('Expected a disabled checkbox not to have a ripple'); + }); + it('should not toggle `checked` state upon interation while disabled', () => { testComponent.isDisabled = true; fixture.detectChanges(); diff --git a/src/lib/checkbox/checkbox.ts b/src/lib/checkbox/checkbox.ts index daa4143d7ffa..a477cd575618 100644 --- a/src/lib/checkbox/checkbox.ts +++ b/src/lib/checkbox/checkbox.ts @@ -217,6 +217,10 @@ export class MdCheckbox implements ControlValueAccessor { } } + _isRippleDisabled() { + return this.disableRipple || this.disabled; + } + /** * Implemented as part of ControlValueAccessor. * TODO: internal diff --git a/src/lib/radio/radio.html b/src/lib/radio/radio.html index f6ff6f3e15d5..9fdf1080dc68 100644 --- a/src/lib/radio/radio.html +++ b/src/lib/radio/radio.html @@ -5,7 +5,7 @@
-
{ expect(radioInstances.every(radio => !radio.checked)).toBe(true); }); + it('should not have a ripple on disabled radio buttons', () => { + let rippleElement = radioNativeElements[0].querySelector('[md-ripple]'); + expect(rippleElement).toBeTruthy('Expected an enabled radio button to have a ripple'); + + radioInstances[0].disabled = true; + fixture.detectChanges(); + + rippleElement = radioNativeElements[0].querySelector('[md-ripple]'); + expect(rippleElement).toBeFalsy('Expected a disabled radio button not to have a ripple'); + }); + it('should remove ripple if md-ripple-disabled input is set', async(() => { fixture.detectChanges(); for (let radioNativeElement of radioNativeElements) diff --git a/src/lib/radio/radio.ts b/src/lib/radio/radio.ts index ff922ca58468..6203a69aef27 100644 --- a/src/lib/radio/radio.ts +++ b/src/lib/radio/radio.ts @@ -374,6 +374,10 @@ export class MdRadioButton implements OnInit { this.change.emit(event); } + _isRippleDisabled() { + return this.disableRipple || this.disabled; + } + /** * We use a hidden native input field to handle changes to focus state via keyboard navigation, * with visual rendering done separately. The native element is kept in sync with the overall