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.

Sign up for GitHub

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): update component specs #846

Merged
merged 1 commit into from
Mar 15, 2021
Merged

feat(checkbox): update component specs #846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 9 additions & 2 deletions docs/components/checkbox/_spec.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
@include hxCheckboxControl($pseudo-state: hover);

~ label > hx-checkbox {
@include hxCheckboxFacade($pseudo-state: hover);
@include hxCheckboxFacade($pseudo-state: null);

border-color: $blue-700;
}
}

Expand All @@ -43,7 +45,12 @@
@include hxCheckboxControl(disabled, hover);

~ label > hx-checkbox {
@include hxCheckboxFacade(disabled, hover);
@include hxCheckboxFacade(disabled, null);

border-color: $gray-500;
border-color: var(--hxCheckbox-disabled-checked-label-bordercolor, $gray-500);
color: $gray-0;
color: var(--hxCheckbox-disabled-checked-label-color, $gray-0);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/scss/_vars.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ $font-monospace: "Open Sans Condensed", "Roboto Mono", monospace;
// ==== FOCUS STATE =====
$focus-glow: 0 0 4px rgba($blue-700, 0.5);
$focus-glow-disabled: $focus-glow;
$focus-glow-invalid: 0 0 4px rgba($red-status-900, 0.5);
$focus-glow-invalid: 0 0 4px rgba($red-status-500, 0.5);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good to me. However, just make sure design includes this in their docs.
The focus states were left out of the checkbox states and missed. I think they were missed for radios and some other components as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

@danielmdesigns, can you provide design recommendations for required and invalid states. These states could occur during validation phase (for example, form validation).

Copy link
Member Author

Choose a reason for hiding this comment

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

@ssalinas24 @danielmdesigns we received confirmation from design to keep required and invalid states. I updated these states to match our v2.0 Checkbox component styled specifications.


$app-nav-width: 15rem;
//$corner-radius: 2px; // Deprecated for Button- v2.0 Specs
Expand Down
44 changes: 40 additions & 4 deletions src/scss/components/checkbox/_index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,12 @@ hx-checkbox-control {

// ========== PRISTINE ==========
hx-checkbox {
background-color: $gray-0;
background-color: var(--hxCheckbox-bgcolor, $gray-0);
border-radius: 2px;
border: 1px solid var(--hxCheckbox-border, $gray-500);
border-color: $gray-500;
border-color: var(--hxCheckbox-border, $gray-500);
border-style: solid;
border-width: 1px;
}

hx-checkbox-control {
Expand Down Expand Up @@ -125,17 +128,22 @@ hx-checkbox-control {

&:checked,
&:indeterminate {
color: $blue-700;
color: var(--hxCheckbox-indeterminate-color, $blue-700);

~ label > hx-checkbox {
background-color: $blue-700;
Copy link
Contributor

Choose a reason for hiding this comment

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

@100stacks so i'm just asking in order to gain knowledge. I see we sometimes duplicate properties. So in this case we have the color variable here, and then the next line we have the same property, but with the CSS variable. This is required for browser compatibility and fallback, correct? So if the browser doesn't support CSS variables, then it will at least have the usual color var, correct?

Copy link
Member Author

@100stacks 100stacks Mar 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@ssalinas24, yes that's correct. We're using duplicate properties for IE11 support fallback. It's cleaner and simpler than using a library to provide polyfills for IE11 CSS variables/custom property support.

background-color: var(--hxCheckbox-indeterminate-label-bgcolor, $blue-700);
border-color: $blue-700;
border-color: var(--hxCheckbox-indeterminate-label-bordercolor, $blue-700);
color: var(--hxCheckbox-indeterminate-label-color, $blue-700);
color: $gray-0;
color: var(--hxCheckbox-indeterminate-label-color, $gray-0);
}
}

&:valid {
~ label > hx-checkbox:hover {
@include hxCheckboxFacade($pseudo-state: hover);
@include hxCheckboxFacade($pseudo-state: null); // v2.0 specs
Copy link
Contributor

Choose a reason for hiding this comment

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

we didn't still need the hover here? I guess because hover technically is supposed to look the same as the checked state, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we need clarification from design on hover while the checkbox is checked/selected state.

Copy link
Member Author

Choose a reason for hiding this comment

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

@danielmdesigns, can you please provide design recommendations for hover state on a checked/selected checkbox.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ssalinas24, I'm leaving the hover state construct in place for now.

}
}
}
Expand Down Expand Up @@ -165,6 +173,20 @@ hx-checkbox-control.hxInvalid > input[type="checkbox"]:enabled {
@include hxCheckboxFacade(invalid, hover);
}
}

// v2.0 specs
&:checked,
&:indeterminate {
color: $red-status-500;
color: var(--hxCheckbox-invalid-checked-color, $red-status-500);

~ label > hx-checkbox {
background-color: $red-status-100;
background-color: var(--hxCheckbox-invalid-checked-bgcolor, $red-status-100);
border-color: $red-status-500;
border-color: var(--hxCheckbox-invalid-checked-label-bordercolor, $red-status-500);
}
}
}

// ========== DISABLED ==========
Expand All @@ -184,6 +206,7 @@ hx-checkbox-control > input[type="checkbox"]:disabled {
}

~ label {
color: $gray-600;
color: var(--hxCheckbox-disabled-label-color, $gray-600);

> hx-checkbox {
Expand All @@ -197,6 +220,19 @@ hx-checkbox-control > input[type="checkbox"]:disabled {
}
}

// v2.0 specs
hx-checkbox-control > input[type="checkbox"]:indeterminate:disabled,
hx-checkbox-control > input[type="checkbox"]:checked:disabled {
~ label > hx-checkbox {
background-color: $gray-500;
background-color: var(--hxCheckbox-disabled-checked-label-bgcolor, $gray-500);
border-color: $gray-500;
border-color: var(--hxCheckbox-disabled-checked-label-bordercolor, $gray-500);
color: $gray-0;
color: var(--hxCheckbox-disabled-checked-label-color, $gray-0);
}
}

// ----- Modern Browsers -----
@supports (--modern: true) {
hx-checkbox-control {
Expand Down
38 changes: 24 additions & 14 deletions src/scss/components/checkbox/mixins/_hxCheckboxControl.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,33 @@
//
// ===== Mixins for <hx-checkbox-control> states =====
//
// =================================================== //
// ===== Mixins for <hx-checkbox-control> states ===== //
// =================================================== //

// ========== PRISTINE ==========
@mixin __checkboxControl($pseudo-state: null) {
@if $pseudo-state == focus {
box-shadow: $focus-glow;

&::-ms-check {
border-color: $blue-700;
border-color: var(--hxCheckbox-focus-bordercolor, $blue-700);
}
}
@else if $pseudo-state == hover {
color: $blue-700;
color: var(--hxCheckbox-hover-color, $blue-700);

&::-ms-check {
background-color: var(--hxCheckbox-hover-bgcolor, $gray-0);
color: var(--hxCheckbox-hover-ie-color, $blue-700);
background-color: $gray-0;
border-color: $blue-700;
color: $blue-700;
}
}
@else {
color: $gray-500;
color: var(--hxCheckbox-color, $gray-500);

&::-ms-check {
background-color: $gray-0;
background-color: var(--hxCheckbox-bgcolor, $gray-0);
border-color: currentColor;
border-style: solid;
Expand All @@ -38,21 +43,24 @@
box-shadow: $focus-glow-invalid;

&::-ms-check {
border-color: var(--hxCheckbox-invalid-bordercolor, $red-status-900);
border-color: $red-status-500;
border-color: var(--hxCheckbox-invalid-bordercolor, $red-status-500);
Copy link
Contributor

Choose a reason for hiding this comment

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

similar setup here. Let me know if this is correct, and I'm just wrong in my prior comment in first file.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's right. This is our pattern to support IE11 fallbacks.

}
}
@else if $pseudo-state == hover {
&::-ms-check {
background-color: var(--hxCheckbox-invalid-hover-bgcolor, $red-status-100);
// v2.0 specs
}
}
@else {
color: var(--hxCheckbox-invalid-color, $red-status-900);
color: $red-status-500;
color: var(--hxCheckbox-invalid-color, $red-status-500);
background-color: $red-status-100;

&::-ms-check {
border-width: 1px;
border-color: $red-status-900 !important;
color: $red-status-900 !important;
border-color: $red-status-500 !important;
color: $red-status-500 !important;
}
}
}
Expand All @@ -61,24 +69,26 @@
@mixin __checkboxControl--disabled($pseudo-state: null) {
@if $pseudo-state == focus {
&::-ms-check {
border-color: $gray-500;
border-color: var(--hxCheckbox-disabled-bordercolor, $gray-500);
box-shadow: $focus-glow-disabled;
}
}
@else if $pseudo-state == hover {
&::-ms-check {
background-color: var(--hxCheckbox-disabled-hover-bgcolor, $gray-0);
border-color: var(--hxCheckbox-disabled-hover-bordercolor, $gray-500);
color: var(--hxCheckbox-disabled-hover-color, $gray-500);
background-color: $gray-0;
border-color: $gray-500;
color: $gray-500;
}
}
@else {
color: var(--hxCheckbox-disabled-color, $gray-500);
cursor: not-allowed;

&::-ms-check {
background-color: $gray-0 !important; // simplest way of styling
border-color: $gray-500;
border-width: 1px;
color: $gray-500;
}
}
}
Expand Down
27 changes: 17 additions & 10 deletions src/scss/components/checkbox/mixins/_hxCheckboxFacade.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
//
// ===== Mixins for <hx-checkbox> facade states =====
//
// ================================================== //
// ===== Mixins for <hx-checkbox> facade states ===== //
// ================================================== //

// ========== PRISTINE ==========
@mixin __checkboxFacade($pseudo-state: null) {
@if $pseudo-state == focus {
border-color: $blue-900;
border-color: var(--hxCheckbox-bordercolor-focus, $blue-900);
box-shadow: $focus-glow;
}
Expand All @@ -18,29 +19,35 @@
// ========== INVALID ==========
@mixin __checkboxFacade--invalid($pseudo-state: null) {
@if $pseudo-state == focus {
border-color: var(--hxCheckbox-invalid-bordercolor, $red-status-900);
border-color: $red-status-500;
border-color: var(--hxCheckbox-invalid-bordercolor, $red-status-500);
box-shadow: $focus-glow-invalid;
}
@else if $pseudo-state == hover {
background-color: var(--hxCheckbox-invalid-hover-bgcolor, $red-status-100);
border-color: var(--hxCheckbox-invalid-hover-bordercolor, $red-status-900);
color: var(--hxCheckbox-invalid-hover-color, $red-status-900);
border-color: $red-status-500;
border-color: var(--hxCheckbox-invalid-hover-bordercolor, $red-status-500);
color: $red-status-500;
color: var(--hxCheckbox-invalid-hover-color, $red-status-500);
}
@else {
border: 1px solid var(--hxCheckbox-invalid-border, $red-status-900);
color: var(--hxCheckbox-invalid-color, $red-status-900);
border: 1px solid $red-status-500;
border: 1px solid var(--hxCheckbox-invalid-border, $red-status-500);
color: $red-status-500;
color: var(--hxCheckbox-invalid-color, $red-status-500);
}
}

// ========== DISABLED ==========
@mixin __checkboxFacade--disabled($pseudo-state: null) {
@if $pseudo-state == focus {
border-color: $gray-500;
border-color: var(--hxCheckbox-disabled-bordercolor, $gray-500);
}
@else if $pseudo-state == hover {
background-color: $gray-0;
background-color: var(--hxCheckbox-disabled-hover-bgcolor, $gray-0);
border-color: $gray-500;
border-color: var(--hxCheckbox-disabled-hover-bordercolor, $gray-500);
color: var(--hxCheckbox-disabled-color, $gray-500);
}
@else {
border-width: 1px;
Expand Down