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

changelog: improvements, add style and markdown for checked and disab… #313

Merged
merged 13 commits into from
May 16, 2022

Conversation

SammySteiner
Copy link
Contributor

…led radios and checkboxes, LG-6230

What

Adding styles and markdown content for displaying variations on radios and checkboxes that are both checked and disabled.

Why

Needed for LG-6230 when users see a form they previously submitted and can select more options but not unselect the one they previously selected. Those options are checked but they must also be disabled.

Example:
Screen Shot 2022-05-13 at 10 37 01 AM

Comment on lines 73 to 76

.usa-checkbox__image {
filter: invert(45%) sepia(1%) saturate(0%) hue-rotate(232deg) brightness(100%) contrast(80%);
}
Copy link
Member

Choose a reason for hiding this comment

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

Are these styles necessary, since it appears they're the same as what would be applied below on lines 144-150?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not necessary, removing now. Thanks for catching.

src/scss/components/_inputs.scss Outdated Show resolved Hide resolved
@aduth aduth requested a review from nickttng May 13, 2022 14:53
@nickttng
Copy link
Member

Is there a way that the checkmark could be more visible? The checkmark from the current example is barely visible (white foreground on light gray background, so, likely, the users won't be able to perceive what it represents.

In Mostyn's example, the checkbox has a darker background so that the white checkmark is more prominent.
Screen Shot 2022-05-13 at 10 46 05 AM

SammySteiner and others added 3 commits May 13, 2022 12:02
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
…ty-style-guide into checkbox-disabled-and-checked
@SammySteiner
Copy link
Contributor Author

Is there a way that the checkmark could be more visible? The checkmark from the current example is barely visible (white foreground on light gray background, so, likely, the users won't be able to perceive what it represents.

In Mostyn's example, the checkbox has a darker background so that the white checkmark is more prominent. Screen Shot 2022-05-13 at 10 46 05 AM

Good catch, change incoming to fix.

Comment on lines 87 to 89
background-color: color('disabled');
background-size: units(1) auto;
box-shadow: inset 0 0 0 units($border-width) color('disabled');
Copy link
Member

Choose a reason for hiding this comment

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

The background color and box shadow styling here should be limited to disabled fields. Currently, it's applying to all:

image

Comment on lines 87 to 89
background-color: color('disabled');
background-size: units(1) auto;
box-shadow: inset 0 0 0 units($border-width) color('disabled');
Copy link
Member

Choose a reason for hiding this comment

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

Also, these are currently only applying to checkboxes. Radio fields look significantly lighter in the current version of the branch. Is that intended?

image

@@ -76,15 +76,19 @@ $input-select-margin-right: 1;
border-radius: 50%;
}

.usa-radio__input:disabled + .usa-radio__label::before,
Copy link
Member

Choose a reason for hiding this comment

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

Should we want this to apply to radio buttons as well? As implemented, the border of the unselected, disabled radio is thicker than the unselected, enabled radio and lighter than the disabled checkbox.

image

image

image

Keeping this selector makes it look a bit more expected:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I added that back in. Ty

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

LGTM 👍

src/scss/components/_inputs.scss Outdated Show resolved Hide resolved
Co-authored-by: Andrew Duthie <andrew.duthie@gsa.gov>
@SammySteiner SammySteiner merged commit d682568 into main May 16, 2022
@SammySteiner SammySteiner deleted the checkbox-disabled-and-checked branch May 16, 2022 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants