Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(checkbox): Add color attribute. #1463

Merged
merged 2 commits into from
Oct 25, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/demo-app/checkbox/checkbox-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ <h1>md-checkbox: Basic Example</h1>
<form>
<md-checkbox [(ngModel)]="isChecked"
name="cb"
[color]="checkboxColor()"
(change)="isIndeterminate = false"
[indeterminate]="isIndeterminate"
[disabled]="isDisabled"
Expand All @@ -18,6 +19,8 @@ <h1>md-checkbox: Basic Example</h1>
<label for="indeterminate-toggle">Toggle Indeterminate</label>
<input id="disabled-toggle" type="checkbox" [(ngModel)]="isDisabled">
<label for="disabled-toggle">Toggle Disabled</label>
<input id="color-toggle" type="checkbox" [(ngModel)]="useAlternativeColor">
<label for="color-toggle">Toggle Color</label>
</div>
<div>
<p>Alignment:</p>
Expand All @@ -41,4 +44,4 @@ <h1>md-checkbox: Basic Example</h1>
</div>

<h1>Nested Checklist</h1>
<md-checkbox-demo-nested-checklist></md-checkbox-demo-nested-checklist>
<md-checkbox-demo-nested-checklist></md-checkbox-demo-nested-checklist>
5 changes: 5 additions & 0 deletions src/demo-app/checkbox/checkbox-demo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,16 @@ export class CheckboxDemo {
isChecked: boolean = false;
isDisabled: boolean = false;
alignment: string = 'start';
useAlternativeColor: boolean = false;

printResult() {
if (this.isIndeterminate) {
return 'Maybe!';
}
return this.isChecked ? 'Yes!' : 'No!';
}

checkboxColor() {
return this.useAlternativeColor ? 'primary' : 'accent';
}
}
14 changes: 14 additions & 0 deletions src/lib/checkbox/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,17 @@ checkbox if you do not wish to have any label text.
```html
<md-checkbox [checked]="false" aria-label="My standalone checkbox"></md-checkbox>
```

### Theming

The color of a `md-checkbox` can be changed by using the `color` attribute.
The value `accent` is default and will correspond to your theme accent color.
Alternatively, you can specify `primary` or `warn`.

Example:

```html
<md-checkbox [checked]="true" color="primary">
I come after my label.
</md-checkbox>
```
43 changes: 38 additions & 5 deletions src/lib/checkbox/_checkbox-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
$is-dark-theme: map-get($theme, is-dark);
$primary: map-get($theme, primary);
$accent: map-get($theme, accent);
$warn: map-get($theme, warn);
$background: map-get($theme, background);


Expand All @@ -15,7 +16,9 @@
$checkbox-mark-color: md-color($background, background);

// The color that is used as the checkbox background when it is checked.
$checkbox-background-color: md-color($accent, 500);
$checkbox-background-color-primary: md-color($primary, 500);
$checkbox-background-color-accent: md-color($accent, 500);
$checkbox-background-color-warn: md-color($warn, 500);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think these variables are necessary; you can use md-color directly where its needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


// NOTE(traviskaufman): While the spec calls for translucent blacks/whites for disabled colors,
// this does not work well with elements layered on top of one another. To get around this we
Expand Down Expand Up @@ -43,8 +46,22 @@
}

.md-checkbox-indeterminate, .md-checkbox-checked {
.md-checkbox-background {
background-color: $checkbox-background-color;
&.md-primary {
.md-checkbox-background {
background-color: $checkbox-background-color-primary;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be a little more concise as

.md-checkbox-indeterminate, .md-checkbox-checked {
  &.primary .md-checkbox-background {
    background: md-color($primary, 500);
  }

  ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done if you meant &.md-primary instead of &.primary.
MdButton also uses the CSS classes md-primary, md-accent and md-warn.


&.md-accent {
.md-checkbox-background {
background-color: $checkbox-background-color-accent;
}
}

&.md-warn {
.md-checkbox-background {
background-color: $checkbox-background-color-warn;
}
}
}

Expand All @@ -63,7 +80,23 @@
}

// TODO(jelbourn): remove style for temporary ripple once the real ripple is applied.
.md-checkbox-focused .md-ink-ripple {
background-color: md-color($accent, 0.26);
.md-checkbox-focused {
&.md-primary {
.md-ink-ripple {
background-color: md-color($primary, 0.26);
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would also be a little more concise as

.md-checkbox-focused {
  &.primary .md-ink-ripple {
    background: md-color($primary, 0.26);
  }
  ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done if you meant &.md-primary instead of &.primary.
MdButton also uses the CSS classes md-primary, md-accent and md-warn.


&.md-accent {
.md-ink-ripple {
background-color: md-color($accent, 0.26);
}
}

&.md-warn {
.md-ink-ripple {
background-color: md-color($warn, 0.26);
}
}
}
}
5 changes: 5 additions & 0 deletions src/lib/checkbox/checkbox.scss
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
@import '../core/theming/theming';
@import '../core/style/elevation';
@import '../core/style/variables';
@import '../core/ripple/ripple';

Expand Down Expand Up @@ -189,6 +190,10 @@ $_md-checkbox-indeterminate-checked-easing-function: cubic-bezier(0.14, 0, 0, 1)

md-checkbox {
cursor: pointer;

// Animation
transition: background $swift-ease-out-duration $swift-ease-out-timing-function,
md-elevation-transition-property-value();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is md-elevation-transition-property-value here? This was only meant to be used with elevation

Copy link
Contributor Author

@mathebox mathebox Oct 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it's used in the same way. _button-base.scss

}

.md-checkbox-layout {
Expand Down
32 changes: 32 additions & 0 deletions src/lib/checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,36 @@ describe('MdCheckbox', () => {
expect(inputElement.required).toBe(false);
});

describe('color behaviour', () => {
it('should apply class based on color attribute', () => {
testComponent.checkboxColor = 'primary';
fixture.detectChanges();
expect(checkboxDebugElement.nativeElement.classList.contains('md-primary')).toBe(true);

testComponent.checkboxColor = 'accent';
fixture.detectChanges();
expect(checkboxDebugElement.nativeElement.classList.contains('md-accent')).toBe(true);
});

it('should should not clear previous defined classes', () => {
checkboxDebugElement.nativeElement.classList.add('custom-class');

testComponent.checkboxColor = 'primary';
fixture.detectChanges();

expect(checkboxDebugElement.nativeElement.classList.contains('md-primary')).toBe(true);
expect(checkboxDebugElement.nativeElement.classList.contains('custom-class')).toBe(true);

testComponent.checkboxColor = 'accent';
fixture.detectChanges();

expect(checkboxDebugElement.nativeElement.classList.contains('md-primary')).toBe(false);
expect(checkboxDebugElement.nativeElement.classList.contains('md-accent')).toBe(true);
expect(checkboxDebugElement.nativeElement.classList.contains('custom-class')).toBe(true);

});
});

describe('state transition css classes', () => {
it('should transition unchecked -> checked -> unchecked', () => {
testComponent.isChecked = true;
Expand Down Expand Up @@ -519,6 +549,7 @@ describe('MdCheckbox', () => {
[checked]="isChecked"
[indeterminate]="isIndeterminate"
[disabled]="isDisabled"
[color]="checkboxColor"
(change)="changeCount = changeCount + 1"
(click)="onCheckboxClick($event)"
(change)="onCheckboxChange($event)">
Expand All @@ -536,6 +567,7 @@ class SingleCheckbox {
parentElementKeyedUp: boolean = false;
lastKeydownEvent: Event = null;
changeCount: number = 0;
checkboxColor: string = 'primary';

onCheckboxClick(event: Event) {}
onCheckboxChange(event: MdCheckboxChange) {}
Expand Down
28 changes: 27 additions & 1 deletion src/lib/checkbox/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,15 @@ export class MdCheckbox implements ControlValueAccessor {

private _indeterminate: boolean = false;

private _color: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just say private _color: string = 'accent';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately that alone won't do the magic. The md-checkbox DOM element has to get the class md-accent. To achieve this you have the following options:

  1. Add one of these lines to the constructor (All are somehow weird calls):
    • this.color = 'accent';
    • this._updateColor(this._color)
    • this._setElementColor(this._color, true)
    • this._renderer.setElementClass(this._elementRef.nativeElement, md-${this._color}, true);
  2. Alternative way: Modify the theme css like this (Introduce a "base" color):
.md-checkbox-indeterminate, .md-checkbox-checked {
  .md-checkbox-background {
    background-color: md-color($accent, 500);
  }
  &.md-primary .md-checkbox-background {
    background-color: md-color($primary, 500);
  }
  ...
}

Which option do you think is the best? Have a missed an option?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jelbourn Any thoughts on this?


private _controlValueAccessorChangeFn: (value: any) => void = (value) => {};

hasFocus: boolean = false;

constructor(private _renderer: Renderer, private _elementRef: ElementRef) {}
constructor(private _renderer: Renderer, private _elementRef: ElementRef) {
this.color = 'accent';
}

/**
* Whether the checkbox is checked. Note that setting `checked` will immediately set
Expand Down Expand Up @@ -174,6 +178,28 @@ export class MdCheckbox implements ControlValueAccessor {
}
}

/** Sets the color of the checkbox */
@Input()
get color(): string {
return this._color;
}

set color(value: string) {
this._updateColor(value);
}

_updateColor(newColor: string) {
this._setElementColor(this._color, false);
this._setElementColor(newColor, true);
this._color = newColor;
}

_setElementColor(color: string, isAdd: boolean) {
if (color != null && color != '') {
this._renderer.setElementClass(this._elementRef.nativeElement, `md-${color}`, isAdd);
}
}

/**
* Implemented as part of ControlValueAccessor.
* TODO: internal
Expand Down