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.

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

USWDS - Checkbox + Radio: create background color variable. #4199

Merged
merged 18 commits into from Jun 7, 2021

Conversation

mejiaj
Copy link
Contributor

@mejiaj mejiaj commented May 17, 2021

Description

Builds on the great work from @aduth on #4178.

Additional information

  • Added variable to _settings-components.scss and _uswds-theme-components.scss
  • Use set-text-and-bg() in checkbox and radio tiles to set accessible color on background

Examples

Default (white)
image

Base darker
image

Green 60v
image


Before you hit Submit, make sure you’ve done whichever of these applies to you:

  • Follow the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run npm test and make sure the tests for the files you have changed have passed.
  • Run your code through HTML_CodeSniffer and make sure it’s error free.
  • Title your pull request using this format: [Website] - [UI component]: Brief statement describing what this pull request solves.

@mejiaj mejiaj marked this pull request as ready for review May 17, 2021 20:46
@mejiaj mejiaj added this to Development review in USWDS Sprint Board via automation May 17, 2021
@mejiaj mejiaj requested a review from thisisdano May 17, 2021 20:47
@@ -1,3 +1,131 @@
@mixin format-input {
Copy link
Member

Choose a reason for hiding this comment

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

Added these two mixins to improve the readability of the component code.

Before:

.usa-checkbox__input:disabled + .usa-checkbox__label::before,
.usa-radio__input:disabled + .usa-radio__label::before {
  background: color("disabled-light");
  box-shadow: 0 0 0 units($theme-input-select-border-width) color("disabled");
  cursor: not-allowed;
}

After:

.usa-checkbox__input,
.usa-radio__input {
  &:disabled {
    @include format-input {
        background: color("disabled-light");
        box-shadow: 0 0 0 units($theme-input-select-border-width) color("disabled");
        cursor: not-allowed;
    }
  }
}

}
}

@mixin checkbox-and-radio-colors(
Copy link
Member

Choose a reason for hiding this comment

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

This long mixin allows users to write scoped colors.

It takes two color token inputs, $bg-color and checked-color. Neither are required and will assume the values in settings if omitted. It returns all the code necessary to set checkbox and radio inputs for the desired scope.

For instance, if you wanted to change the color of all checkbox and radio elements within a .checkbox-tests container, you could do something like the following

.checkbox-tests {
  @include set-text-and-bg("green-80");
  @include checkbox-and-radio-colors("green-80", "green-warm-10v");
  padding: units(2);
  border-radius: radius("md");
}

}

@mixin checkbox-and-radio-colors(
$bg-color: $theme-input-background-color,
Copy link
Member

Choose a reason for hiding this comment

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

The bg-color default the component bg color from settings...


@mixin checkbox-and-radio-colors(
$bg-color: $theme-input-background-color,
$checked-color: "default"
Copy link
Member

Choose a reason for hiding this comment

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

...and the default checked color is either the dark or light link color set in settings.

@@ -11,18 +139,65 @@
}
}

@include checkbox-and-radio-colors;
Copy link
Member

Choose a reason for hiding this comment

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

Here, I'm using the mixin to set the component's default colors.

.usa-checkbox__label-description,
.usa-radio__label-description {
display: block;
font-size: size("ui", "2xs");
margin-top: units(1);
text-indent: 0;
}

// Test code for scoped custom colors
Copy link
Member

Choose a reason for hiding this comment

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

This test code is used to power the example here: https://federalist-3b6ba08e-0df4-44c9-ac73-6fc193b0e19c.app.cloud.gov/preview/uswds/uswds/jm-tile-add-bg/components/preview/checkbox--test.html

We should comment it out before going into production

@@ -80,16 +80,14 @@ $theme-footer-max-width: "desktop" !default;
// Form and input
$theme-checkbox-border-radius: "sm" !default;
$theme-form-font-family: "ui" !default;
$theme-input-background-color: default !default;
Copy link
Member

Choose a reason for hiding this comment

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

I changed this from a tile-specific background color to an input background color since this will now apply to original checkbox and radios as well

$theme-input-line-height: 3 !default;
$theme-input-max-width: "mobile-lg" !default;
$theme-input-select-border-width: 2px !default;
$theme-input-select-size: 2.5 !default;
$theme-input-state-border-width: 0.5 !default;
$theme-input-tile-background-color-selected: "primary-lighter" !default;
Copy link
Member

Choose a reason for hiding this comment

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

The system now sets this automatically based on the background color. The tile background color is a more transparent version of the calculated text/border color

$theme-input-tile-border-radius: "md" !default;
$theme-input-tile-border-width: 2px !default;
$theme-input-tile-border-color: "base-lighter" !default;
Copy link
Member

Choose a reason for hiding this comment

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

The system now sets this automatically as a transparent version of the calculated text color

$theme-input-tile-border-radius: "md" !default;
$theme-input-tile-border-width: 2px !default;
$theme-input-tile-border-color: "base-lighter" !default;
$theme-input-tile-border-color-selected: "primary-light" !default;
Copy link
Member

Choose a reason for hiding this comment

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

The system now uses the standard or reverse link color as the selected color instead of a special new color.

@thisisdano
Copy link
Member

What I did

This new work allows users to change the output colors of default and tile checkboxes and radio buttons. The system will calculate the proper colors for all elements based on the set background and the values of standard and reverse text and link colors.

It also includes a new mixin, checkbox-and-radio-colors($bg-color, $selected-color) that allows developers to write scoped colors for checkboxes and radios that contained within custom-color elements:

.checkbox-tests {
  @include set-text-and-bg("green-80");
  @include checkbox-and-radio-colors("green-80", "green-warm-10v");
  padding: units(2);
  border-radius: radius("md");
}

This allows something like what we see below, where the top section uses colors derived solely from settings, and the bottom section includes new scoped color rules generated by the mixin:

Screen Shot 2021-06-02 at 3 46 24 PM

Further notes in the code comments.

What to check:

  • Change the value of component settings like $theme-input-background-color
  • Change the value of color settings like:
    • $theme-body-background-color
    • $theme-text-color
    • $theme-text-reverse-color
    • $theme-link-color
    • $theme-link-reverse-color

Copy link
Contributor Author

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

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

This is good!

Noticed that changing $theme-input-background-color gives odd results.


$theme-input-background-color: "magenta-40";

image
image
image


$theme-input-background-color: "primary-light";
image
image
image
image


$theme-input-background-color: "primary-dark"

image

@thisisdano
Copy link
Member

thisisdano commented Jun 3, 2021

That is the expected result for using that setting (as built). If you change the value of $theme-input-background-color to a value that is not the actual background color for the component, you'll get the kind of results you're seeing. $theme-input-background-color does not actually set the background color, it adapts the colors of the component to a specified background color.

I updated the code to give the selected background color to usa-checkbox and usa-radio elements, but the general point still stands: designers will need to assign the desired background color to the input's containing element independent of this setting.

Note: This code is not meant to let designers give their tiles a separate background color independent from the form's background, like to give the tiles a yellow tile background on a white form background. We don't want the tiles to have a dramatically different look from the their surroundings.

@mejiaj
Copy link
Contributor Author

mejiaj commented Jun 3, 2021

I see, thanks for the clarification.

@thisisdano thisisdano merged commit 5147766 into develop Jun 7, 2021
USWDS Sprint Board automation moved this from Development review to Done Jun 7, 2021
@thisisdano thisisdano deleted the jm-tile-add-bg branch June 7, 2021 22:25
@thisisdano thisisdano mentioned this pull request Jun 16, 2021
danielnaab added a commit to 18F/fedramp-automation that referenced this pull request Aug 17, 2021
danielnaab added a commit to 18F/fedramp-automation that referenced this pull request Aug 17, 2021
aduth added a commit to 18F/identity-design-system that referenced this pull request Aug 25, 2021
**Why:** Avoid future maintenance overhead incurred by version drift, incorporate latest features and updates, including upstream contributions previously implemented as overrides.

Changelog:
- https://github.com/uswds/uswds/releases/tag/v2.12.1
- https://github.com/uswds/uswds/releases/tag/v2.12.0

Incorporated revisions:
- uswds/uswds#4199
- uswds/uswds#4286
aduth added a commit to 18F/identity-design-system that referenced this pull request Aug 27, 2021
**Why:** Avoid future maintenance overhead incurred by version drift, incorporate latest features and updates, including upstream contributions previously implemented as overrides.

Changelog:
- https://github.com/uswds/uswds/releases/tag/v2.12.1
- https://github.com/uswds/uswds/releases/tag/v2.12.0

Incorporated revisions:
- uswds/uswds#4199
- uswds/uswds#4286
aduth added a commit to 18F/identity-design-system that referenced this pull request Aug 27, 2021
**Why:** Avoid future maintenance overhead incurred by version drift, incorporate latest features and updates, including upstream contributions previously implemented as overrides.

Changelog:
- https://github.com/uswds/uswds/releases/tag/v2.12.1
- https://github.com/uswds/uswds/releases/tag/v2.12.0

Incorporated revisions:
- uswds/uswds#4199
- uswds/uswds#4286
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants