Address feedback

Author: devversion

src/lib/button/button.ts

@@ -120,8 +120,8 @@ export class MdButton extends MdThemeable {
   get disabled() { return this._disabled; }
   set disabled(value: boolean) { this._disabled = coerceBooleanProperty(value) ? true : null; }
 
-  constructor(private _elementRef: ElementRef, private _renderer: Renderer) {
-    super(_renderer, _elementRef);
+  constructor(elementRef: ElementRef, renderer: Renderer) {
+    super(renderer, elementRef);
   }
 
   _setMousedown() {

src/lib/checkbox/checkbox.ts

@@ -173,10 +173,10 @@ export class MdCheckbox extends MdThemeable implements ControlValueAccessor {
 
   _hasFocus: boolean = false;
 
-  constructor(private _renderer: Renderer,
-              private _elementRef: ElementRef,
-              private _changeDetectorRef: ChangeDetectorRef) {
-    super(_renderer, _elementRef);
+  constructor(private _changeDetectorRef: ChangeDetectorRef,
+              renderer: Renderer,
+              elementRef: ElementRef) {
+    super(renderer, elementRef);
 
     // By default set the component color to accent.
     this.color = 'accent';

src/lib/chips/chip.ts

@@ -56,8 +56,8 @@ export class MdChip extends MdThemeable implements Focusable, OnInit, OnDestroy
   /** Emitted when the chip is destroyed. */
   @Output() destroy = new EventEmitter<MdChipEvent>();
 
-  constructor(protected _renderer: Renderer, protected _elementRef: ElementRef) {
-    super(_renderer, _elementRef);
+  constructor(renderer: Renderer, elementRef: ElementRef) {
+    super(renderer, elementRef);
 
     // By default the chip elements should use the primary palette.
     this.color = 'primary';

src/lib/core/style/themeable.spec.ts

@@ -44,6 +44,19 @@ describe('MdThemeable', () => {
     expect(themeableElement.classList).toContain('mat-accent');
     expect(themeableElement.classList).not.toContain('mat-warn');
     expect(themeableElement.classList).not.toContain('mat-primary');
+
+    testComponent.color = null;
+    fixture.detectChanges();
+
+    expect(themeableElement.classList).not.toContain('mat-accent');
+    expect(themeableElement.classList).not.toContain('mat-warn');
+    expect(themeableElement.classList).not.toContain('mat-primary');
+  });
+
+  it('should throw an error when using an invalid color', () => {
+    testComponent.color = 'Invalid';
+
+    expect(() => fixture.detectChanges()).toThrow();
   });
 
 });

src/lib/core/style/themeable.ts

@@ -1,30 +1,61 @@
 import {ElementRef, Input, Renderer} from '@angular/core';
+import {MdError} from '../errors/error';
 
-/** Material components can extend the MdThemeable class to add an Input that can
- * developers use to switch palettes on the components. */
+/** Possible color values for the color input. */
+export type MdThemeColor = 'primary' | 'accent' | 'warn';
+
+const VALID_COLOR_VALUES = ['primary', 'accent', 'warn'];
+
+/**
+ * Material components can extend the MdThemeable class to add an Input that can
+ * developers use to switch palettes on the components.
+ **/
 export class MdThemeable {
 
   /** Stored color for the themeable component. */
-  private _color: string;
+  private _color: MdThemeColor;
 
-  constructor(private renderer: Renderer, private elementRef: ElementRef) {}
+  // Constructor initializers need to have the `protected` modifier to avoid interferences.
+  // TypeScript throws if similar declarations, regardless of the modifier, have been
+  // found across the different classes. Because of that, the child classes should just use
+  // the protected properties from the superclass.
+  constructor(protected _renderer: Renderer, protected _elementRef: ElementRef) {}
 
   /** Color of the component. Values are primary, accent, or warn. */
   @Input()
-  get color(): string {
+  get color(): MdThemeColor {
     return this._color;
   }
-  set color(newColor: string) {
+  set color(newColor: MdThemeColor) {
+    this._validateColor(newColor);
+
     this._setElementColor(this._color, false);
     this._setElementColor(newColor, true);
     this._color = newColor;
   }
 
+  /** Validates the specified color value and throws an error if invalid. */
+  private _validateColor(color: string) {
+    if (color && VALID_COLOR_VALUES.indexOf(color) === -1) {
+      throw new MdInvalidColorValueError(color);
+    }
+  }
+
   /** Toggles a color class on the components host element. */
   private _setElementColor(color: string, isAdd: boolean) {
-    if (color != null && color != '') {
-      this.renderer.setElementClass(this.elementRef.nativeElement, `mat-${color}`, isAdd);
+    if (color) {
+      this._renderer.setElementClass(this._elementRef.nativeElement, `mat-${color}`, isAdd);
     }
   }
 
 }
+
+/** Error that will be thrown if the color input is set to an invalid value. */
+export class MdInvalidColorValueError extends MdError {
+  constructor(invalidColor: string) {
+    super(
+      `The color "${invalidColor}" for is not valid. ` +
+      `Possible values are: ${VALID_COLOR_VALUES.join(', ')} or null.`
+    );
+  }
+}

src/lib/icon/icon.ts

@@ -95,10 +95,10 @@ export class MdIcon extends MdThemeable implements OnChanges, OnInit, AfterViewC
   private _previousAriaLabel: string;
 
   constructor(
-      private _elementRef: ElementRef,
-      private _renderer: Renderer,
-      private _mdIconRegistry: MdIconRegistry) {
-    super(_renderer, _elementRef);
+      private _mdIconRegistry: MdIconRegistry,
+      elementRef: ElementRef,
+      renderer: Renderer) {
+    super(renderer, elementRef);
   }
 
   /**

src/lib/progress-spinner/progress-spinner.ts

@@ -153,8 +153,8 @@ export class MdProgressSpinner extends MdThemeable implements OnDestroy {
     this._mode = m;
   }
 
-  constructor(private _ngZone: NgZone, private _elementRef: ElementRef, renderer: Renderer) {
-    super(renderer, _elementRef);
+  constructor(private _ngZone: NgZone, elementRef: ElementRef, renderer: Renderer) {
+    super(renderer, elementRef);
   }
 
 

src/lib/slide-toggle/slide-toggle.ts

@@ -115,8 +115,8 @@ export class MdSlideToggle extends MdThemeable implements AfterContentInit, Cont
 
   @ViewChild('input') _inputElement: ElementRef;
 
-  constructor(private _elementRef: ElementRef, private _renderer: Renderer) {
-    super(_renderer, _elementRef);
+  constructor(elementRef: ElementRef, renderer: Renderer) {
+    super(renderer, elementRef);
   }
 
   ngAfterContentInit() {