Skip to content

Commit

Permalink
fix(checkbox)!: remove error property
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: Design is still exploring error-state checkboxes. If needed, theme with an error color and set aria-invalid.

PiperOrigin-RevId: 552914338
  • Loading branch information
@asyncLiz
asyncLiz authored and Copybara-Service committed Aug 1, 2023
1 parent ad01113 commit ce248dc
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 133 deletions.
1 change: 0 additions & 1 deletion checkbox/demo/demo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const collection =
new MaterialCollection<KnobTypesToKnobs<StoryKnobs>>('Checkbox', [
new Knob('checked', {defaultValue: false, ui: boolInput()}),
new Knob('indeterminate', {defaultValue: false, ui: boolInput()}),
new Knob('error', {defaultValue: false, ui: boolInput()}),
new Knob('disabled', {defaultValue: false, ui: boolInput()}),
]);

Expand Down
9 changes: 2 additions & 7 deletions checkbox/demo/stories.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,17 @@ import {css, html} from 'lit';
export interface StoryKnobs {
checked: boolean;
disabled: boolean;
error: boolean;
indeterminate: boolean;
}

const checkbox: MaterialStoryInit<StoryKnobs> = {
name: 'Checkbox',
render({checked, disabled, error, indeterminate}) {
render({checked, disabled, indeterminate}) {
return html`
<md-checkbox
aria-label="An example checkbox"
?checked=${checked}
?disabled=${disabled}
?error=${error}
?indeterminate=${indeterminate}
></md-checkbox>
`;
Expand All @@ -47,13 +45,12 @@ const withLabels: MaterialStoryInit<StoryKnobs> = {
}
`,
],
render({disabled, error}) {
render({disabled}) {
return html`
<div class="column" role="group" aria-label="Animals">
<label role="presentation">
<md-checkbox
?disabled=${disabled}
?error=${error}
aria-label="Cats"
></md-checkbox>
<span aria-hidden="true">Cats</span>
Expand All @@ -62,7 +59,6 @@ const withLabels: MaterialStoryInit<StoryKnobs> = {
<md-checkbox
checked
?disabled=${disabled}
?error=${error}
aria-label="dogs"
></md-checkbox>
<span aria-hidden="true">Dogs</span>
Expand All @@ -71,7 +67,6 @@ const withLabels: MaterialStoryInit<StoryKnobs> = {
<md-checkbox
indeterminate
?disabled=${disabled}
?error=${error}
aria-label="Birds"
></md-checkbox>
<span aria-hidden="true">Birds</span>
Expand Down
61 changes: 0 additions & 61 deletions checkbox/internal/_checkbox.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -195,17 +195,6 @@ $_checkmark-bottom-left: 7px, -14px;
);
}

.error md-ripple {
@include ripple.theme(
(
hover-color: var(--_error-hover-state-layer-color),
hover-opacity: var(--_error-hover-state-layer-opacity),
pressed-color: var(--_error-pressed-state-layer-color),
pressed-opacity: var(--_error-pressed-state-layer-opacity),
)
);
}

// Icon and icon marks

.icon {
Expand Down Expand Up @@ -325,20 +314,6 @@ $_checkmark-bottom-left: 7px, -14px;

// States

:where(.error) .outline {
border-color: var(--_error-outline-color);
// TODO(b/262410085): add once token is added
// border-width: var(--_error-outline-width);
}

:where(.error) .background {
background: var(--_selected-error-container-color);
}

:where(.error) .icon {
fill: var(--_selected-error-icon-color);
}

:where(:hover) .outline {
border-color: var(--_hover-outline-color);
border-width: var(--_hover-outline-width);
Expand All @@ -352,18 +327,6 @@ $_checkmark-bottom-left: 7px, -14px;
fill: var(--_selected-hover-icon-color);
}

:where(.error:hover) .outline {
border-color: var(--_error-hover-outline-color);
}

:where(.error:hover) .background {
background: var(--_selected-error-hover-container-color);
}

:where(.error:hover) .icon {
fill: var(--_selected-error-hover-icon-color);
}

:where(:focus-within) .outline {
border-color: var(--_focus-outline-color);
border-width: var(--_focus-outline-width);
Expand All @@ -377,18 +340,6 @@ $_checkmark-bottom-left: 7px, -14px;
fill: var(--_selected-focus-icon-color);
}

:where(.error:focus-within) .outline {
border-color: var(--_error-focus-outline-color);
}

:where(.error:focus-within) .background {
background: var(--_selected-error-focus-container-color);
}

:where(.error:focus-within) .icon {
fill: var(--_selected-error-focus-icon-color);
}

:where(:active) .outline {
border-color: var(--_pressed-outline-color);
border-width: var(--_pressed-outline-width);
Expand All @@ -402,18 +353,6 @@ $_checkmark-bottom-left: 7px, -14px;
fill: var(--_selected-pressed-icon-color);
}

:where(.error:active) .outline {
border-color: var(--_error-pressed-outline-color);
}

:where(.error:active) .background {
background: var(--_selected-error-pressed-container-color);
}

:where(.error:active) .icon {
fill: var(--_selected-error-pressed-icon-color);
}

// Don't animate to/from disabled states because the outline is hidden when
// selected. Without this, there'd be a FOUC if the checkbox state is
// programmatically changed while disabled.
Expand Down
27 changes: 2 additions & 25 deletions checkbox/internal/checkbox.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@ export class Checkbox extends LitElement {
*/
@property({type: Boolean, reflect: true}) disabled = false;

/**
* Whether or not the checkbox is invalid.
*/
@property({type: Boolean}) error = false;

/**
* Whether or not the checkbox is indeterminate.
*
Expand Down Expand Up @@ -182,23 +177,6 @@ export class Checkbox extends LitElement {
return this.internals.reportValidity();
}

/**
* Checks the checkbox's native validation and returns whether or not the
* element is valid.
*
* If invalid, this method will dispatch the `invalid` event.
*
* The checkbox's `error` state will be set to true if invalid, or false if
* valid.
*
* @return true if the checkbox is valid, or false if not.
*/
showValidity() {
const isValid = this.checkValidity();
this.error = !isValid;
return isValid;
}

/**
* Sets the checkbox's native validation error message. This is used to
* customize `validationMessage`.
Expand Down Expand Up @@ -242,15 +220,14 @@ export class Checkbox extends LitElement {
'unselected': !isChecked && !isIndeterminate,
'checked': isChecked,
'indeterminate': isIndeterminate,
'error': this.error && !this.disabled,
'prev-unselected': prevNone,
'prev-checked': prevChecked,
'prev-indeterminate': prevIndeterminate,
'prev-disabled': this.prevDisabled,
});

// Needed for closure conformance
const {ariaLabel} = this as ARIAMixinStrict;
const {ariaLabel, ariaInvalid} = this as ARIAMixinStrict;
return html`
<div class="container ${containerClasses}">
<div class="outline"></div>
Expand All @@ -266,7 +243,7 @@ export class Checkbox extends LitElement {
id="input"
aria-checked=${isIndeterminate ? 'mixed' : nothing}
aria-label=${ariaLabel || nothing}
aria-invalid=${this.error || nothing}
aria-invalid=${ariaInvalid || nothing}
?disabled=${this.disabled}
?required=${this.required}
.indeterminate=${this.indeterminate}
Expand Down
23 changes: 0 additions & 23 deletions checkbox/internal/checkbox_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ describe('checkbox', () => {
expect(harness.element.checked).toEqual(false);
expect(harness.element.indeterminate).toEqual(false);
expect(harness.element.disabled).toEqual(false);
expect(harness.element.error).toEqual(false);
expect(harness.element.value).toEqual('on');
});

Expand Down Expand Up @@ -245,27 +244,5 @@ describe('checkbox', () => {
.withContext('checkbox.validity.valueMissing')
.toBeTrue();
});

it('should set error to true when showValidity() is called and checkbox is invalid',
async () => {
const {harness} = await setupTest();
harness.element.required = true;

harness.element.showValidity();
expect(harness.element.error).withContext('checkbox.error').toBeTrue();
});

it('should set error to false when showValidity() is called and checkbox is valid',
async () => {
const {harness} = await setupTest();
harness.element.required = true;
harness.element.error = true;
harness.element.checked = true;

harness.element.showValidity();
expect(harness.element.error)
.withContext('checkbox.error')
.toBeFalse();
});
});
});
32 changes: 16 additions & 16 deletions tokens/_md-comp-checkbox.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,6 @@ $supported-tokens: (
'disabled-container-opacity',
'disabled-outline-color',
'disabled-outline-width',
'error-focus-outline-color',
'error-hover-outline-color',
'error-hover-state-layer-color',
'error-hover-state-layer-opacity',
'error-outline-color',
'error-pressed-outline-color',
'error-pressed-state-layer-color',
'error-pressed-state-layer-opacity',
'focus-outline-color',
'focus-outline-width',
'hover-outline-color',
Expand All @@ -43,14 +35,6 @@ $supported-tokens: (
'selected-disabled-container-color',
'selected-disabled-container-opacity',
'selected-disabled-icon-color',
'selected-error-container-color',
'selected-error-focus-container-color',
'selected-error-focus-icon-color',
'selected-error-hover-container-color',
'selected-error-hover-icon-color',
'selected-error-icon-color',
'selected-error-pressed-container-color',
'selected-error-pressed-icon-color',
'selected-focus-container-color',
'selected-focus-icon-color',
'selected-hover-container-color',
Expand All @@ -69,11 +53,27 @@ $supported-tokens: (

$unsupported-tokens: (
// go/keep-sorted start
'error-focus-outline-color',
'error-focus-state-layer-color',
'error-focus-state-layer-opacity',
'error-hover-outline-color',
'error-hover-state-layer-color',
'error-hover-state-layer-opacity',
'error-outline-color',
'error-pressed-outline-color',
'error-pressed-state-layer-color',
'error-pressed-state-layer-opacity',
'focus-state-layer-color',
'focus-state-layer-opacity',
'selected-disabled-container-outline-width',
'selected-error-container-color',
'selected-error-focus-container-color',
'selected-error-focus-icon-color',
'selected-error-hover-container-color',
'selected-error-hover-icon-color',
'selected-error-icon-color',
'selected-error-pressed-container-color',
'selected-error-pressed-icon-color',
'selected-focus-outline-width',
'selected-focus-state-layer-color',
'selected-focus-state-layer-opacity',
Expand Down

0 comments on commit ce248dc

Please sign in to comment.