Skip to content

Commit

Permalink
fix(checkbox): no side margin if label has no content (#2121)
Browse files Browse the repository at this point in the history
* Remove side margin in checkbox if label has no content.
* Use `checkboxNativeElement` instead of `checkboxDebugElement.nativeElement` in expect statements.
* Fix typo.

Closes #2011
  • Loading branch information
belev authored and mmalerba committed Apr 28, 2017
1 parent 3560ada commit 4e8d806
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 11 deletions.
5 changes: 3 additions & 2 deletions src/lib/checkbox/checkbox.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<label class="mat-checkbox-layout" #label>
<div class="mat-checkbox-inner-container">
<div class="mat-checkbox-inner-container"
[class.mat-checkbox-inner-container-no-side-margin]="!_hasLabel()">
<input #input
class="mat-checkbox-input cdk-visually-hidden" type="checkbox"
[id]="inputId"
Expand Down Expand Up @@ -34,7 +35,7 @@
<div class="mat-checkbox-mixedmark"></div>
</div>
</div>
<span class="mat-checkbox-label">
<span class="mat-checkbox-label" #labelWrapper>
<ng-content></ng-content>
</span>
</label>
7 changes: 7 additions & 0 deletions src/lib/checkbox/checkbox.scss
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,13 @@ $_mat-checkbox-mark-stroke-size: 2 / 15 * $mat-checkbox-size !default;
}
}

.mat-checkbox-inner-container-no-side-margin {
margin: {
left: 0;
right: 0;
}
}

// TODO(kara): Remove this style when fixing vertical baseline
.mat-checkbox-layout .mat-checkbox-label {
line-height: 24px;
Expand Down
40 changes: 31 additions & 9 deletions src/lib/checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('MdCheckbox', () => {
CheckboxWithNameAttribute,
CheckboxWithChangeEvent,
CheckboxWithFormControl,
CheckboxWithoutLabel,
],
providers: [
{provide: ViewportRuler, useClass: FakeViewportRuler}
Expand Down Expand Up @@ -436,28 +437,28 @@ describe('MdCheckbox', () => {
it('should apply class based on color attribute', () => {
testComponent.checkboxColor = 'primary';
fixture.detectChanges();
expect(checkboxDebugElement.nativeElement.classList.contains('mat-primary')).toBe(true);
expect(checkboxNativeElement.classList.contains('mat-primary')).toBe(true);

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

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

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

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

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

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

});
});
Expand Down Expand Up @@ -730,7 +731,6 @@ describe('MdCheckbox', () => {
});
});


describe('with form control', () => {
let checkboxDebugElement: DebugElement;
let checkboxInstance: MdCheckbox;
Expand Down Expand Up @@ -763,6 +763,22 @@ describe('MdCheckbox', () => {
expect(inputElement.disabled).toBe(false);
});
});

describe('without label', () => {
let checkboxDebugElement: DebugElement;
let checkboxNativeElement: HTMLElement;

it('should add a css class to inner-container to remove side margin', () => {
fixture = TestBed.createComponent(CheckboxWithoutLabel);
fixture.detectChanges();
checkboxDebugElement = fixture.debugElement.query(By.directive(MdCheckbox));
checkboxNativeElement = checkboxDebugElement.nativeElement;

let checkboxInnerContainerWithoutMarginCount = checkboxNativeElement
.querySelectorAll('.mat-checkbox-inner-container-no-side-margin').length;
expect(checkboxInnerContainerWithoutMarginCount).toBe(1);
});
});
});

/** Simple component for testing a single checkbox. */
Expand Down Expand Up @@ -872,3 +888,9 @@ class CheckboxWithChangeEvent {
class CheckboxWithFormControl {
formControl = new FormControl();
}

/** Test component without label */
@Component({
template: `<md-checkbox></md-checkbox>`
})
class CheckboxWithoutLabel {}
9 changes: 9 additions & 0 deletions src/lib/checkbox/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,15 @@ export class MdCheckbox extends _MdCheckboxMixinBase
/** The native `<input type="checkbox"> element */
@ViewChild('input') _inputElement: ElementRef;

@ViewChild('labelWrapper') _labelWrapper: ElementRef;

/** Whether the checkbox has label */
_hasLabel(): boolean {
const labelText = this._labelWrapper.nativeElement.textContent || '';
return !!labelText.trim().length;
}

/** Called when the checkbox is blurred. Needed to properly implement ControlValueAccessor. */
@ViewChild(MdRipple) _ripple: MdRipple;

/**
Expand Down

0 comments on commit 4e8d806

Please sign in to comment.