Skip to content

Commit

Permalink
fix(checkbox): no side margin if label has no content
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 angular#2011
  • Loading branch information
belev committed Dec 8, 2016
1 parent 84323a8 commit 276a705
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 12 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="md-checkbox-layout">
<div class="md-checkbox-inner-container">
<div class="md-checkbox-inner-container"
[class.md-checkbox-inner-container-no-side-margin]="!hasLabel">
<input #input
class="md-checkbox-input md-visually-hidden" type="checkbox"
[id]="inputId"
Expand Down Expand Up @@ -36,7 +37,7 @@
<div class="md-checkbox-mixedmark"></div>
</div>
</div>
<span class="md-checkbox-label">
<span class="md-checkbox-label" #labelWrapper>
<ng-content></ng-content>
</span>
</label>
14 changes: 14 additions & 0 deletions src/lib/checkbox/checkbox.scss
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,13 @@ md-checkbox {
right: auto;
}
}

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

// TODO(kara): Remove this style when fixing vertical baseline
Expand Down Expand Up @@ -292,6 +299,13 @@ md-checkbox {
right: $md-checkbox-item-spacing;
}
}

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

Expand Down
40 changes: 30 additions & 10 deletions src/lib/checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe('MdCheckbox', () => {
CheckboxWithNameAttribute,
CheckboxWithChangeEvent,
CheckboxWithFormControl,
CheckboxWithoutLabel,
],
providers: [
{provide: ViewportRuler, useClass: FakeViewportRuler},
Expand Down Expand Up @@ -181,7 +182,7 @@ describe('MdCheckbox', () => {
expect(rippleElement).toBeFalsy('Expected a disabled checkbox not to have a ripple');
});

it('should not toggle `checked` state upon interation while disabled', () => {
it('should not toggle `checked` state upon interaction while disabled', () => {
testComponent.isDisabled = true;
fixture.detectChanges();

Expand Down Expand Up @@ -308,28 +309,28 @@ describe('MdCheckbox', () => {
it('should apply class based on color attribute', () => {
testComponent.checkboxColor = 'primary';
fixture.detectChanges();
expect(checkboxDebugElement.nativeElement.classList.contains('md-primary')).toBe(true);
expect(checkboxNativeElement.classList.contains('md-primary')).toBe(true);

testComponent.checkboxColor = 'accent';
fixture.detectChanges();
expect(checkboxDebugElement.nativeElement.classList.contains('md-accent')).toBe(true);
expect(checkboxNativeElement.classList.contains('md-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('md-primary')).toBe(true);
expect(checkboxDebugElement.nativeElement.classList.contains('custom-class')).toBe(true);
expect(checkboxNativeElement.classList.contains('md-primary')).toBe(true);
expect(checkboxNativeElement.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);
expect(checkboxNativeElement.classList.contains('md-primary')).toBe(false);
expect(checkboxNativeElement.classList.contains('md-accent')).toBe(true);
expect(checkboxNativeElement.classList.contains('custom-class')).toBe(true);

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


describe('with form control', () => {
let checkboxDebugElement: DebugElement;
let checkboxInstance: MdCheckbox;
Expand Down Expand Up @@ -617,6 +617,20 @@ describe('MdCheckbox', () => {
expect(checkboxInstance.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;

expect(checkboxNativeElement.querySelectorAll('.md-checkbox-inner-container-no-side-margin').length).toBe(1);
});
});
});

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

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

@ViewChild('labelWrapper') _labelWrapper: ElementRef;

get hasLabel(): boolean {
const labelWrapperTextContent = this._labelWrapper.nativeElement.textContent;
return labelWrapperTextContent && labelWrapperTextContent.trim().length > 0;
}

/** Called when the checkbox is blurred. Needed to properly implement ControlValueAccessor. */
onTouched: () => any = () => {};

Expand Down

0 comments on commit 276a705

Please sign in to comment.