From 6b9e11cfd7700718016c444c651b9aa7e5222904 Mon Sep 17 00:00:00 2001 From: Jeremy Elbourn <jelbourn@gmail.com> Date: Tue, 8 Nov 2016 17:39:55 -0800 Subject: [PATCH] fix: disable ripples when parent component is disabled (#1778) --- 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 @@ <span class="md-button-wrapper"><ng-content></ng-content></span> -<div md-ripple *ngIf="isRippleEnabled()" class="md-button-ripple" +<div md-ripple *ngIf="!_isRippleDisabled()" class="md-button-ripple" [class.md-button-ripple-round]="isRoundButton()" [md-ripple-trigger]="getHostElement()" [md-ripple-color]="isRoundButton() ? 'rgba(255, 255, 255, 0.2)' : ''" diff --git a/src/lib/button/button.spec.ts b/src/lib/button/button.spec.ts index e2bec8b31473..0144fcf7843d 100644 --- a/src/lib/button/button.spec.ts +++ b/src/lib/button/button.spec.ts @@ -1,7 +1,4 @@ -import { - async, - TestBed, -} from '@angular/core/testing'; +import {async, TestBed, ComponentFixture} from '@angular/core/testing'; import {Component} from '@angular/core'; import {By} from '@angular/platform-browser'; import {MdButtonModule} from './button'; @@ -124,17 +121,43 @@ describe('MdButton', () => { // 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<TestApp>; + 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)"> - <div md-ripple *ngIf="!disableRipple" class="md-checkbox-ripple" + <div md-ripple *ngIf="!_isRippleDisabled()" class="md-checkbox-ripple" [md-ripple-trigger]="getHostElement()" [md-ripple-centered]="true" [md-ripple-speed-factor]="0.3" diff --git a/src/lib/checkbox/checkbox.spec.ts b/src/lib/checkbox/checkbox.spec.ts index 7fd7c478c6fa..342c4032bed2 100644 --- a/src/lib/checkbox/checkbox.spec.ts +++ b/src/lib/checkbox/checkbox.spec.ts @@ -153,6 +153,17 @@ describe('MdCheckbox', () => { 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 @@ <div class="md-radio-container"> <div class="md-radio-outer-circle"></div> <div class="md-radio-inner-circle"></div> - <div md-ripple *ngIf="!disableRipple" class="md-radio-ripple" + <div md-ripple *ngIf="!_isRippleDisabled()" class="md-radio-ripple" [md-ripple-trigger]="getHostElement()" [md-ripple-centered]="true" [md-ripple-speed-factor]="0.3" diff --git a/src/lib/radio/radio.spec.ts b/src/lib/radio/radio.spec.ts index 6fa1eb6642fe..1925c4efac01 100644 --- a/src/lib/radio/radio.spec.ts +++ b/src/lib/radio/radio.spec.ts @@ -218,6 +218,17 @@ describe('MdRadio', () => { 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