From c110943532f9a1ccff5a73829cc892ac15c69e04 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Mon, 27 Aug 2018 20:23:30 +0200 Subject: [PATCH] feat(checkbox): align with 2018 material design spec Aligns the checkbox component with the latest Material design spec. --- e2e/components/checkbox-e2e.spec.ts | 6 --- src/lib/checkbox/_checkbox-theme.scss | 8 +-- src/lib/checkbox/checkbox.html | 3 +- src/lib/checkbox/checkbox.scss | 45 ++++++++++++---- src/lib/checkbox/checkbox.spec.ts | 42 +++++---------- src/lib/checkbox/checkbox.ts | 51 ++++++------------- .../_pseudo-checkbox-theme.scss | 3 +- .../pseudo-checkbox/pseudo-checkbox.scss | 9 +++- src/lib/core/style/_checkbox-common.scss | 2 +- src/lib/select/select.spec.ts | 4 +- src/lib/select/select.ts | 2 +- 11 files changed, 80 insertions(+), 95 deletions(-) diff --git a/e2e/components/checkbox-e2e.spec.ts b/e2e/components/checkbox-e2e.spec.ts index 1102a8412b08..f7a7a786ce87 100644 --- a/e2e/components/checkbox-e2e.spec.ts +++ b/e2e/components/checkbox-e2e.spec.ts @@ -14,16 +14,10 @@ describe('checkbox', () => { expect(inputEl.getAttribute('checked')) .toBeTruthy('Expect checkbox "checked" property to be true'); - await browser.wait(ExpectedConditions.not( - ExpectedConditions.presenceOf(element(by.css('div.mat-ripple-element'))))); - checkboxEl.click(); expect(inputEl.getAttribute('checked')) .toBeFalsy('Expect checkbox "checked" property to be false'); - - await browser.wait(ExpectedConditions.not( - ExpectedConditions.presenceOf(element(by.css('div.mat-ripple-element'))))); }); it('should toggle the checkbox when pressing space', () => { diff --git a/src/lib/checkbox/_checkbox-theme.scss b/src/lib/checkbox/_checkbox-theme.scss index d13c92e0e97a..581df9a775d9 100644 --- a/src/lib/checkbox/_checkbox-theme.scss +++ b/src/lib/checkbox/_checkbox-theme.scss @@ -60,7 +60,7 @@ } .mat-checkbox-disabled { - &.mat-checkbox-checked, &.mat-checkbox-indeterminate { + &.mat-checkbox-checked:not(.mat-checkbox-indeterminate) { .mat-checkbox-background { background-color: $disabled-color; } @@ -92,15 +92,15 @@ .mat-checkbox:not(.mat-checkbox-disabled) { &.mat-primary .mat-checkbox-ripple .mat-ripple-element { - background-color: mat-color($primary, 0.26); + background-color: mat-color($primary); } &.mat-accent .mat-checkbox-ripple .mat-ripple-element { - background-color: mat-color($accent, 0.26); + background-color: mat-color($accent); } &.mat-warn .mat-checkbox-ripple .mat-ripple-element { - background-color: mat-color($warn, 0.26); + background-color: mat-color($warn); } } } diff --git a/src/lib/checkbox/checkbox.html b/src/lib/checkbox/checkbox.html index c29c859785a1..0c23dae408e6 100644 --- a/src/lib/checkbox/checkbox.html +++ b/src/lib/checkbox/checkbox.html @@ -19,9 +19,10 @@
+
diff --git a/src/lib/checkbox/checkbox.scss b/src/lib/checkbox/checkbox.scss index 8329c241652f..bc37ff2d669c 100644 --- a/src/lib/checkbox/checkbox.scss +++ b/src/lib/checkbox/checkbox.scss @@ -11,7 +11,7 @@ $_mat-checkbox-mark-path-length: 22.910259; $_mat-checkbox-indeterminate-checked-easing-function: cubic-bezier(0.14, 0, 0, 1); // The ripple size of the checkbox -$_mat-checkbox-ripple-radius: 25px; +$_mat-checkbox-ripple-radius: 20px; // The amount of spacing between the checkbox and its label. $_mat-checkbox-item-spacing: $mat-toggle-padding; @@ -164,13 +164,6 @@ $_mat-checkbox-mark-stroke-size: 2 / 15 * $mat-checkbox-size !default; } } -// Applied to elements that are considered "marks" within the checkbox, e.g. the checkmark and -// the mixedmark. -%mat-checkbox-mark { - $width-padding-inset: 2 * $mat-checkbox-border-width; - width: calc(100% - #{$width-padding-inset}); -} - // Applied to elements that appear to make up the outer box of the checkmark, such as the frame // that contains the border and the actual background element that contains the marks. %mat-checkbox-outer-box { @@ -189,6 +182,10 @@ $_mat-checkbox-mark-stroke-size: 2 / 15 * $mat-checkbox-size !default; cursor: pointer; -webkit-tap-highlight-color: transparent; + + .mat-ripple-element:not(.mat-checkbox-persistent-ripple) { + opacity: 0.16; + } } .mat-checkbox-layout { @@ -260,10 +257,29 @@ $_mat-checkbox-mark-stroke-size: 2 / 15 * $mat-checkbox-size !default; } } +.mat-checkbox-persistent-ripple { + width: 100%; + height: 100%; + transform: none; + + .mat-checkbox-inner-container:hover & { + opacity: 0.04; + } + + .mat-checkbox.cdk-focused & { + opacity: 0.12; + } + + // We do this here, rather than having a `:not(.mat-checkbox-disabled)` + // above in the `:hover`, because the `:not` will bump the specificity + // a lot and will cause it to overide the focus styles. + &, .mat-checkbox.mat-disabled .mat-checkbox-inner-container:hover & { + opacity: 0; + } +} + .mat-checkbox-checkmark { @include mat-fill; - @extend %mat-checkbox-mark; - width: 100%; } @@ -278,10 +294,11 @@ $_mat-checkbox-mark-stroke-size: 2 / 15 * $mat-checkbox-size !default; .mat-checkbox-mixedmark { $height: floor($_mat-checkbox-mark-stroke-size); - @extend %mat-checkbox-mark; + width: calc(100% - 6px); height: $height; opacity: 0; transform: scaleX(0) rotate(0deg); + border-radius: 2px; @include cdk-high-contrast { height: 0; @@ -335,6 +352,12 @@ $_mat-checkbox-mark-stroke-size: 2 / 15 * $mat-checkbox-size !default; opacity: 1; transform: scaleX(1) rotate(0deg); } + + &.mat-checkbox-disabled { + .mat-checkbox-inner-container { + opacity: 0.5; + } + } } diff --git a/src/lib/checkbox/checkbox.spec.ts b/src/lib/checkbox/checkbox.spec.ts index 1ae96a96d582..71602a6b51cb 100644 --- a/src/lib/checkbox/checkbox.spec.ts +++ b/src/lib/checkbox/checkbox.spec.ts @@ -2,7 +2,6 @@ import { ComponentFixture, fakeAsync, TestBed, - tick, flush, flushMicrotasks, } from '@angular/core/testing'; @@ -11,7 +10,6 @@ import {Component, DebugElement, ViewChild, Type} from '@angular/core'; import {By} from '@angular/platform-browser'; import {dispatchFakeEvent} from '@angular/cdk/testing'; import {MatCheckbox, MatCheckboxChange, MatCheckboxModule} from './index'; -import {defaultRippleAnimationConfig} from '@angular/material/core'; import {MAT_CHECKBOX_CLICK_ACTION} from './checkbox-config'; import {MutationObserverFactory} from '@angular/cdk/observers'; @@ -379,25 +377,6 @@ describe('MatCheckbox', () => { expect(inputElement.value).toBe('basic_checkbox'); }); - it('should show a ripple when focused by a keyboard action', fakeAsync(() => { - expect(fixture.nativeElement.querySelectorAll('.mat-ripple-element').length) - .toBe(0, 'Expected no ripples on load.'); - - dispatchFakeEvent(inputElement, 'keydown'); - dispatchFakeEvent(inputElement, 'focus'); - - tick(defaultRippleAnimationConfig.enterDuration); - - expect(fixture.nativeElement.querySelectorAll('.mat-ripple-element').length) - .toBe(1, 'Expected ripple after element is focused.'); - - dispatchFakeEvent(checkboxInstance._inputElement.nativeElement, 'blur'); - tick(defaultRippleAnimationConfig.exitDuration); - - expect(fixture.nativeElement.querySelectorAll('.mat-ripple-element').length) - .toBe(0, 'Expected no ripple after element is blurred.'); - })); - it('should remove the SVG checkmark from the tab order', () => { expect(checkboxNativeElement.querySelector('svg')!.getAttribute('focusable')).toBe('false'); }); @@ -405,22 +384,25 @@ describe('MatCheckbox', () => { describe('ripple elements', () => { it('should show ripples on label mousedown', () => { - expect(checkboxNativeElement.querySelector('.mat-ripple-element')).toBeFalsy(); + const rippleSelector = '.mat-ripple-element:not(.mat-checkbox-persistent-ripple)'; + + expect(checkboxNativeElement.querySelector(rippleSelector)).toBeFalsy(); dispatchFakeEvent(labelElement, 'mousedown'); dispatchFakeEvent(labelElement, 'mouseup'); - expect(checkboxNativeElement.querySelectorAll('.mat-ripple-element').length).toBe(1); + expect(checkboxNativeElement.querySelectorAll(rippleSelector).length).toBe(1); }); it('should not show ripples when disabled', () => { + const rippleSelector = '.mat-ripple-element:not(.mat-checkbox-persistent-ripple)'; testComponent.isDisabled = true; fixture.detectChanges(); dispatchFakeEvent(labelElement, 'mousedown'); dispatchFakeEvent(labelElement, 'mouseup'); - expect(checkboxNativeElement.querySelectorAll('.mat-ripple-element').length).toBe(0); + expect(checkboxNativeElement.querySelectorAll(rippleSelector).length).toBe(0); testComponent.isDisabled = false; fixture.detectChanges(); @@ -428,17 +410,18 @@ describe('MatCheckbox', () => { dispatchFakeEvent(labelElement, 'mousedown'); dispatchFakeEvent(labelElement, 'mouseup'); - expect(checkboxNativeElement.querySelectorAll('.mat-ripple-element').length).toBe(1); + expect(checkboxNativeElement.querySelectorAll(rippleSelector).length).toBe(1); }); it('should remove ripple if matRippleDisabled input is set', () => { + const rippleSelector = '.mat-ripple-element:not(.mat-checkbox-persistent-ripple)'; testComponent.disableRipple = true; fixture.detectChanges(); dispatchFakeEvent(labelElement, 'mousedown'); dispatchFakeEvent(labelElement, 'mouseup'); - expect(checkboxNativeElement.querySelectorAll('.mat-ripple-element').length).toBe(0); + expect(checkboxNativeElement.querySelectorAll(rippleSelector).length).toBe(0); testComponent.disableRipple = false; fixture.detectChanges(); @@ -446,7 +429,7 @@ describe('MatCheckbox', () => { dispatchFakeEvent(labelElement, 'mousedown'); dispatchFakeEvent(labelElement, 'mouseup'); - expect(checkboxNativeElement.querySelectorAll('.mat-ripple-element').length).toBe(1); + expect(checkboxNativeElement.querySelectorAll(rippleSelector).length).toBe(1); }); }); @@ -842,19 +825,20 @@ describe('MatCheckbox', () => { }); it('should toggle checkbox ripple disabledness correctly', () => { + const rippleSelector = '.mat-ripple-element:not(.mat-checkbox-persistent-ripple)'; const labelElement = checkboxNativeElement.querySelector('label') as HTMLLabelElement; testComponent.isDisabled = true; fixture.detectChanges(); dispatchFakeEvent(labelElement, 'mousedown'); dispatchFakeEvent(labelElement, 'mouseup'); - expect(checkboxNativeElement.querySelectorAll('.mat-ripple-element').length).toBe(0); + expect(checkboxNativeElement.querySelectorAll(rippleSelector).length).toBe(0); testComponent.isDisabled = false; fixture.detectChanges(); dispatchFakeEvent(labelElement, 'mousedown'); dispatchFakeEvent(labelElement, 'mouseup'); - expect(checkboxNativeElement.querySelectorAll('.mat-ripple-element').length).toBe(1); + expect(checkboxNativeElement.querySelectorAll(rippleSelector).length).toBe(1); }); }); diff --git a/src/lib/checkbox/checkbox.ts b/src/lib/checkbox/checkbox.ts index d36ee8599663..c4c52f0e7f51 100644 --- a/src/lib/checkbox/checkbox.ts +++ b/src/lib/checkbox/checkbox.ts @@ -6,11 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ -import {FocusMonitor, FocusOrigin} from '@angular/cdk/a11y'; +import {FocusMonitor} from '@angular/cdk/a11y'; import {coerceBooleanProperty} from '@angular/cdk/coercion'; import { - AfterViewChecked, - AfterViewInit, Attribute, ChangeDetectionStrategy, ChangeDetectorRef, @@ -26,6 +24,7 @@ import { Output, ViewChild, ViewEncapsulation, + AfterViewChecked, } from '@angular/core'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms'; import { @@ -42,7 +41,6 @@ import { mixinDisabled, mixinDisableRipple, mixinTabIndex, - RippleRef, } from '@angular/material/core'; import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations'; import {MAT_CHECKBOX_CLICK_ACTION, MatCheckboxClickAction} from './checkbox-config'; @@ -133,8 +131,7 @@ export const _MatCheckboxMixinBase: changeDetection: ChangeDetectionStrategy.OnPush }) export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAccessor, - AfterViewChecked, AfterViewInit, OnDestroy, CanColor, CanDisable, HasTabIndex, - CanDisableRipple { + AfterViewChecked, OnDestroy, CanColor, CanDisable, HasTabIndex, CanDisableRipple { /** * Attached to the aria-label attribute of the host element. In most cases, arial-labelledby will @@ -195,9 +192,6 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc private _controlValueAccessorChangeFn: (value: any) => void = () => {}; - /** Reference to the focused state ripple. */ - private _focusRipple: RippleRef | null; - constructor(elementRef: ElementRef, private _changeDetectorRef: ChangeDetectorRef, private _focusMonitor: FocusMonitor, @@ -209,12 +203,17 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc super(elementRef); this.tabIndex = parseInt(tabIndex) || 0; - } - ngAfterViewInit() { - this._focusMonitor - .monitor(this._inputElement) - .subscribe(focusOrigin => this._onInputFocusChange(focusOrigin)); + this._focusMonitor.monitor(elementRef, true).subscribe(focusOrigin => { + if (!focusOrigin) { + // When a focused element becomes disabled, the browser *immediately* fires a blur event. + // Angular does not expect events to be raised during change detection, so any state change + // (such as a form control's 'ng-touched') will cause a changed-after-checked error. + // See https://github.com/angular/angular/issues/17793. To work around this, we defer + // telling the form control it has been touched until the next tick. + Promise.resolve().then(() => this._onTouched()); + } + }); } ngAfterViewChecked() { @@ -222,7 +221,7 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc } ngOnDestroy() { - this._focusMonitor.stopMonitoring(this._inputElement); + this._focusMonitor.stopMonitoring(this._elementRef); } /** @@ -342,7 +341,7 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc } private _emitChangeEvent() { - let event = new MatCheckboxChange(); + const event = new MatCheckboxChange(); event.source = this; event.checked = this.checked; @@ -350,26 +349,6 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc this.change.emit(event); } - /** Function is called whenever the focus changes for the input element. */ - private _onInputFocusChange(focusOrigin: FocusOrigin) { - // TODO(paul): support `program`. See https://github.com/angular/material2/issues/9889 - if (!this._focusRipple && focusOrigin === 'keyboard') { - this._focusRipple = this.ripple.launch(0, 0, {persistent: true}); - } else if (!focusOrigin) { - if (this._focusRipple) { - this._focusRipple.fadeOut(); - this._focusRipple = null; - } - - // When a focused element becomes disabled, the browser *immediately* fires a blur event. - // Angular does not expect events to be raised during change detection, so any state change - // (such as a form control's 'ng-touched') will cause a changed-after-checked error. - // See https://github.com/angular/angular/issues/17793. To work around this, we defer telling - // the form control it has been touched until the next tick. - Promise.resolve().then(() => this._onTouched()); - } - } - /** Toggles the `checked` state of the checkbox. */ toggle(): void { this.checked = !this.checked; diff --git a/src/lib/core/selection/pseudo-checkbox/_pseudo-checkbox-theme.scss b/src/lib/core/selection/pseudo-checkbox/_pseudo-checkbox-theme.scss index 871fadce6f6b..eb38e33a8f12 100644 --- a/src/lib/core/selection/pseudo-checkbox/_pseudo-checkbox-theme.scss +++ b/src/lib/core/selection/pseudo-checkbox/_pseudo-checkbox-theme.scss @@ -43,8 +43,7 @@ background: mat-color(map-get($theme, warn)); } - .mat-pseudo-checkbox-checked, - .mat-pseudo-checkbox-indeterminate { + .mat-pseudo-checkbox-checked { &.mat-pseudo-checkbox-disabled { background: $disabled-color; } diff --git a/src/lib/core/selection/pseudo-checkbox/pseudo-checkbox.scss b/src/lib/core/selection/pseudo-checkbox/pseudo-checkbox.scss index 073d2c52880b..cbb6e725e6a0 100644 --- a/src/lib/core/selection/pseudo-checkbox/pseudo-checkbox.scss +++ b/src/lib/core/selection/pseudo-checkbox/pseudo-checkbox.scss @@ -45,13 +45,18 @@ $_mat-pseudo-checkmark-size: $mat-checkbox-size - (2 * $_mat-pseudo-checkbox-pad .mat-pseudo-checkbox-disabled { cursor: default; + + &.mat-pseudo-checkbox-indeterminate { + opacity: 0.5; + } } .mat-pseudo-checkbox-indeterminate::after { top: ($mat-checkbox-size - $mat-checkbox-border-width) / 2 - $mat-checkbox-border-width; - left: 0; - width: $mat-checkbox-size - ($mat-checkbox-border-width * 2); + left: $mat-checkbox-border-width / 2; + width: $mat-checkbox-size - 6px; opacity: 1; + border-radius: 2px; } .mat-pseudo-checkbox-checked::after { diff --git a/src/lib/core/style/_checkbox-common.scss b/src/lib/core/style/_checkbox-common.scss index e5a916908ccb..d921d9c972d0 100644 --- a/src/lib/core/style/_checkbox-common.scss +++ b/src/lib/core/style/_checkbox-common.scss @@ -1,7 +1,7 @@ @import './variables'; // The width/height of the checkbox element. -$mat-checkbox-size: $mat-toggle-size !default; +$mat-checkbox-size: 16px !default; // The width of the checkbox border shown when the checkbox is unchecked. $mat-checkbox-border-width: 2px; diff --git a/src/lib/select/select.spec.ts b/src/lib/select/select.spec.ts index a9d573ad77ac..d2badc3099ec 100644 --- a/src/lib/select/select.spec.ts +++ b/src/lib/select/select.spec.ts @@ -3569,7 +3569,7 @@ describe('MatSelect', () => { // 44px accounts for the checkbox size, margin and the panel's padding. expect(Math.floor(firstOptionLeft)) - .toEqual(Math.floor(triggerLeft - 44), + .toEqual(Math.floor(triggerLeft - 40), `Expected trigger label to align along x-axis, accounting for the checkbox.`); })); @@ -3585,7 +3585,7 @@ describe('MatSelect', () => { // 44px accounts for the checkbox size, margin and the panel's padding. expect(Math.floor(firstOptionRight)) - .toEqual(Math.floor(triggerRight + 44), + .toEqual(Math.floor(triggerRight + 40), `Expected trigger label to align along x-axis, accounting for the checkbox.`); })); }); diff --git a/src/lib/select/select.ts b/src/lib/select/select.ts index c49930c449c3..1d37781bd82b 100644 --- a/src/lib/select/select.ts +++ b/src/lib/select/select.ts @@ -125,7 +125,7 @@ export const SELECT_ITEM_HEIGHT_EM = 3; * Calculated as: * (SELECT_PANEL_PADDING_X * 1.5) + 20 = 44 * The padding is multiplied by 1.5 because the checkbox's margin is half the padding. - * The checkbox width is 20px. + * The checkbox width is 16px. */ export let SELECT_MULTIPLE_PANEL_PADDING_X = 0;