Skip to content

Add better support for custom icon colors #4079

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

Merged
mejiaj merged 13 commits into from
Mar 12, 2021
Merged

Add better support for custom icon colors #4079

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1048f9f
Add alert icons that respond to theme colors
thisisdano Mar 5, 2021
4e421c0
Remove __icon styles (not used) and table-oriented styles
thisisdano Mar 5, 2021
8439f3b
Rebuild slim variant
thisisdano Mar 5, 2021
9f123d2
Use smaller optical factor
thisisdano Mar 5, 2021
973981a
Add padding-y theme variable
thisisdano Mar 5, 2021
e049585
Add proper variants for ie--white icon fallbacks
thisisdano Mar 5, 2021
4b4cba4
Revert colors
thisisdano Mar 5, 2021
989c281
Fix link to stand ie fallback icon
thisisdano Mar 5, 2021
f1d2e82
Fix icon path
thisisdano Mar 11, 2021
1a510dc
Don't include usa-icons in an icon object name path
thisisdano Mar 11, 2021
5d1cdcc
Use new icon set for banner
thisisdano Mar 11, 2021
c86e0c6
Remove calc() in favor of native Sass math
thisisdano Mar 12, 2021
41d00d3
Reposition icon to top in validation
thisisdano Mar 12, 2021
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
1 change: 1 addition & 0 deletions src/img/usa-icons-bg/check_circle--white.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions src/img/usa-icons-bg/error--white.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions src/img/usa-icons-bg/info--white.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions src/img/usa-icons-bg/warning--white.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
100 changes: 62 additions & 38 deletions src/stylesheets/components/_alerts.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,39 @@
$alert-slim-icon-size: units(3);
Copy link
Contributor Author

@thisisdano thisisdano Mar 11, 2021

Choose a reason for hiding this comment

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

We'll be adding this icon with a :before block and the mask technique instead of a background image on the alert itself, so some vars and calculations need to change.

Copy link
Contributor

@mejiaj mejiaj Mar 12, 2021

Choose a reason for hiding this comment

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

We might need a $alert-slim-icon-padding. The icon on slim variation stays the same but the padding varies when the icon size is updated.

For example, change $theme-alert-icon-size to 6.

image

$alert-icon-optical-factor: calc(#{units($theme-alert-icon-size)} / 6);
$alert-icon-optical-padding: calc(
#{units($theme-alert-padding-x)} - #{$alert-icon-optical-factor}
);

@mixin add-alert-icon($name, $color, $bgcolor) {
$this-icon-object: map-merge(
$icon-object,
(
"name": $name,
"color": $color,
"height": $theme-alert-icon-size,
)
);
&:before {
@include add-color-icon($this-icon-object, $bgcolor);
// Position centered in the alert
bottom: 0;
content: "";
display: block;
// padding - optical spacing value
left: $alert-icon-optical-padding;
position: absolute;
height: auto;
top: 0;
Comment on lines +17 to +25
Copy link
Contributor Author

@thisisdano thisisdano Mar 11, 2021

Choose a reason for hiding this comment

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

This changes the icon positioning a bit, so the icon is vertically centered on the alert.

Copy link
Contributor

@mejiaj mejiaj Mar 12, 2021

Choose a reason for hiding this comment

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

IE11 doesn't support calc() on generated content.

Maybe we should try a left value before this one or something like:

// Some left value for ie11
left: 1rem;

// Calc for modern browsers
@supports (left: $alert-icon-optical-padding) {
  left: $alert-icon-optical-padding;
}

image

Copy link
Contributor Author

@thisisdano thisisdano Mar 12, 2021

Choose a reason for hiding this comment

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

It doesn't look like the calc() is working for the content padding-left either

Copy link
Contributor Author

@thisisdano thisisdano Mar 12, 2021

Choose a reason for hiding this comment

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

Just removed the calc()s and used Sass math instead

}
&.usa-alert--slim:before {
background-size: $alert-slim-icon-size;
width: $alert-slim-icon-size;
@supports (mask: url("")) {
mask-size: $alert-slim-icon-size;
}
}
}

// Alert variables ---------- //

$alert-icons: (
Expand All @@ -8,36 +44,32 @@ $alert-icons: (
emergency: "error",
);

$alert-padding-left: units($theme-alert-padding-x) +
units($theme-alert-bar-width);
$icon-object: (
"name": "ICON_NAME",
"svg-height": 40,
"svg-width": 40,
"height": $theme-icon-image-size,
"color": "ink",
);

$alert-padding-left: units($theme-alert-bar-width);

.usa-alert {
@include typeset($theme-alert-font-family);
@include border-box-sizing;
background-color: color("base-lightest");
background-position: $alert-padding-left units($theme-alert-padding-x - 1);
background-repeat: no-repeat;
background-size: units($theme-alert-icon-size);
padding-bottom: units(2);
padding-left: $alert-padding-left;
padding-right: units($theme-alert-padding-x);
padding-top: units($theme-alert-padding-x);
@include set-text-and-bg(
"base-lightest",
$theme-alert-text-color,
$theme-alert-text-reverse-color
);
@include u-padding-y($theme-alert-padding-y);
position: relative;

* + & {
margin-top: units(2);
}

// TODO: why is this not simply a border?
&::before {
background-color: color("base-light");
content: "";
height: 100%;
left: 0;
position: absolute;
top: 0;
width: units($theme-alert-bar-width);
}
border-left: units($theme-alert-bar-width) solid color("base-light");
Copy link
Contributor Author

@thisisdano thisisdano Mar 11, 2021

Choose a reason for hiding this comment

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

Change alert bar to a border for simplicity

Copy link
Contributor

@mejiaj mejiaj Mar 12, 2021

Choose a reason for hiding this comment

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

We should move this up before position: relative.


> .usa-list,
.usa-alert__body > .usa-list {
Expand All @@ -49,14 +81,14 @@ $alert-padding-left: units($theme-alert-padding-x) +
}
}

.usa-alert__icon {
display: table-cell;
padding-right: units($theme-alert-bar-width);
@each $name, $icon in $alert-icons {
.usa-alert--#{$name} {
@include alert-status-styles($name, $icon);
}
}

.usa-alert__body {
display: table-cell;
vertical-align: top;
@include u-padding-x($theme-alert-padding-x);
Copy link
Contributor Author

@thisisdano thisisdano Mar 11, 2021

Choose a reason for hiding this comment

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

Remove the weird table-related styling

}

.usa-alert__heading {
Expand All @@ -74,25 +106,13 @@ $alert-padding-left: units($theme-alert-padding-x) +
}

.usa-alert__text:only-child {
margin-bottom: units($theme-alert-bar-width);
padding-top: units(0.5);
}

@each $name, $icon in $alert-icons {
.usa-alert--#{$name} {
@include alert-status-styles($name, $icon);
}
@include u-padding-y(0);
}

.usa-alert--slim {
@include alert-slim-styles;
}

.usa-alert--no-heading {
background-position: $alert-padding-left
calc(#{units($theme-alert-padding-x)} - #{units(0.5)});
}

.usa-alert--validation {
background-size: units(3);
background-position: $alert-padding-left
Expand All @@ -106,3 +126,7 @@ $alert-padding-left: units($theme-alert-padding-x) +
margin-top: units(2);
}
}

.usa-alert--emergency {
border-left: none;
}
15 changes: 4 additions & 11 deletions src/stylesheets/core/mixins/_alert-slim-styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,8 @@
@mixin alert-slim-styles {
background-position: $alert-padding-left units(1.5);
background-size: units(3);
padding-bottom: units($theme-alert-bar-width);
padding-top: units($theme-alert-bar-width);

@include u-padding-y(1);
.usa-alert__body {
padding-left: units(4);
}

.usa-alert__text:only-child {
margin-bottom: units(0.5);
padding-top: units(0.5);
padding-left: calc(
#{$alert-slim-icon-size} + (2 * #{$alert-icon-optical-padding})
);
}
}
27 changes: 13 additions & 14 deletions src/stylesheets/core/mixins/_alert-status-styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,20 @@
$bgcolor: if($name != "emergency", "#{$name}-lighter", $name);
$banner-text-color-token: get-color-token-from-bg(
$bgcolor,
$context: "Alert text"
$theme-alert-text-reverse-color,
Copy link
Contributor Author

@thisisdano thisisdano Mar 11, 2021

Choose a reason for hiding this comment

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

Use theme colors

$theme-alert-text-color,
$context: "Alert (#{$name})"
);
$icon-path: if(
$banner-text-color-token == "ink",
"usa-icons/#{$icon}",
"alerts/#{$icon}-white"
);
@include add-background-svg($icon-path);

@include add-alert-icon($icon, $banner-text-color-token, $bgcolor);
background-color: color($bgcolor);
border-left-color: color($name);
color: color($banner-text-color-token);

&::before {
background-color: color($name);
}

.usa-alert__body {
padding-left: units($theme-alert-icon-size) + units($theme-alert-padding-x);
padding-left: calc(
#{units($theme-alert-icon-size)} + (2 * #{$alert-icon-optical-padding})
);
}

.usa-link {
Expand All @@ -31,10 +28,12 @@
}

&.usa-alert--no-icon {
background-image: none;
&:before {
display: none;
}

.usa-alert__body {
padding-left: 0;
padding-left: units($theme-alert-padding-x);
}
}
}
19 changes: 10 additions & 9 deletions src/stylesheets/core/mixins/_icon.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,16 @@
map-get($icon-object, "path"),
$theme-image-path
);
$ie11-variant: get-color-token-from-bg($contrast-bg, $color-variant, "black");
$filename-ie11-variant: if($ie11-variant == "black", null, $ie11-variant);
$filename: if(
$filename-ie11-variant,
"#{$filename-base}-#{$filename-ie11-variant}.svg",
"#{$filename-base}.svg"
$ie11-variant: get-color-token-from-bg($contrast-bg, "white", "black");
$filename-ie11: if(
$ie11-variant == "white",
"usa-icons-bg/#{$filename-base}--white.svg",
"usa-icons/#{$filename-base}.svg"
);

$image-props: url("#{$path}/#{$filename}") no-repeat center / #{$width} #{$height};
$mask-props: url("#{$path}/#{$filename-base}.svg") no-repeat center / #{$width}
#{$height};
$image-props: url("#{$path}/#{$filename-ie11}") no-repeat center / #{$width} #{$height};
Copy link
Contributor Author

@thisisdano thisisdano Mar 11, 2021

Choose a reason for hiding this comment

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

Fix this code so it pulls the right --white variant from usa-icons-bg for IE support


// Default background shorthand for browsers that don't support mask or supports.
background: $image-props;
Expand All @@ -135,11 +136,11 @@
transform: rotate($rotate);
}

// Mask supportered styles
// Mask supported styles
@supports (mask: url("")) {
background: none;
background-color: color($color);
mask: $image-props;
mask: $mask-props;
@if $color-hover {
&:hover {
background-color: color($color-hover);
Expand Down
1 change: 1 addition & 0 deletions src/stylesheets/settings/_settings-components.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ $theme-alert-bar-width: 1 !default;
$theme-alert-font-family: "ui" !default;
$theme-alert-icon-size: 5 !default;
$theme-alert-padding-x: 2.5 !default;
$theme-alert-padding-y: 2 !default;
Copy link
Contributor Author

@thisisdano thisisdano Mar 11, 2021

Choose a reason for hiding this comment

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

Add explicit padding-y setting instead of calculating it from padding-x, which isn't expected.

$theme-alert-text-color: default !default;
$theme-alert-text-reverse-color: default !default;
Comment on lines 31 to 32
Copy link
Contributor

@mejiaj mejiaj Mar 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Should we deprecate these?

We could use the defaults instead. I know they're set to default, but I wonder if these are even used.

$theme-text-color
$theme-text-reverse-color

Copy link
Contributor Author

@thisisdano thisisdano Mar 12, 2021

Choose a reason for hiding this comment

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

They're maybe a little edge-y but if you wanted a different color for alerts or reverse alerts it would be helpful. I'd prefer not to deprecate them one release after introducing them, I guess. 😬

$theme-alert-link-color: default !default;
Expand Down
1 change: 1 addition & 0 deletions src/stylesheets/theme/_uswds-theme-components.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ $theme-alert-bar-width: 1;
$theme-alert-font-family: "ui";
$theme-alert-icon-size: 5;
$theme-alert-padding-x: 2.5;
$theme-alert-padding-y: 2;
$theme-alert-text-color: default;
$theme-alert-text-reverse-color: default;
$theme-alert-link-color: default;
Expand Down