Skip to content

Commit 398003e

Browse files
castastrophegithub-actions[bot]starryeyez024
authored
fix: [pfe-icon] icon color vs. background-color discrepancy (#1541) (#1545)
* fix: Update flood color on background circle to use broadcast context instead * fix: Update pfe-icon styles to support background color and color * fix: Add additional documentation' * fix: Update internal mixin * fix: Clean up block references * fix: Update changelog * fix: newline 🤦‍♀️ * fix: Create deprecated section * fix: Resolve duplication and missing hook * fix: Adjusting icon values * Update elements/pfe-icon/README.md Co-authored-by: Kendall Totten <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Kendall Totten <[email protected]>
1 parent 267ff8e commit 398003e

File tree

5 files changed

+104
-108
lines changed

5 files changed

+104
-108
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Development files
22
node_modules
3+
./docs/node_modules
34
tmp
45
.tmp
56

‎CHANGELOG-1.x.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
# 1.8.0 (2021)
22

33
- [f1c1176](https://github.com/patternfly/patternfly-elements/commit/f1c1176d9278d6e5b8066b42fc040ea01d98ecb2) feat: pfe-icon now supports setting default icon sets
4-
- [](https://github.com/patternfly/patternfly-elements/commit/) feat: Update fetch mixin to support region input (#1328)
4+
- [267ff8e](https://github.com/patternfly/patternfly-elements/commit/267ff8ee7df7cd0512f16c58fdb169f941bfa4cd) feat: Update fetch mixin to support region input (#1328)
55
- [56eb55e](https://github.com/patternfly/patternfly-elements/commit/56eb55ec8b4b62aee7d36950d158229cbf50ddef) fix: pfe-accordion IE11 regression; background context should always be white with black text
6+
- [](https://github.com/patternfly/patternfly-elements/commit/) fix: [pfe-icon] Update icon color vs. background color custom property support
7+
68

79
# 1.7.0 (2021-05-10)
810

‎elements/pfe-icon/README.md‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ There are no slots, but if you wish to display some text when JS is disabled, yo
2020
| Name | Values | Description |
2121
| --- | --- | --- |
2222
| `icon` | `iconSet-iconName` | For example, `rh-leaf` loads a leaf icon from an icon set named "rh". |
23-
| `pfe-size` | `sm` `md` `lg` `xl` `1x` `2x` `3x` `4x` | The default size is 1em, so icon size matches text size. `2x`, etc, are multiples of font size. `sm`, `md`, etc are fixed pixel-based sizes. |
24-
| `pfe-color` | `base` `lightest` `lighter` `darker` `darkest` `complement` `accent` `accent` `critical` `important` `moderate` `success` `info` `default` | The color variant to use. This draws from your theming layer to color the icon. This will set icon color or background color (if `pfe-circled` is true). |
25-
| `pfe-circled` | boolean attribute | Whether to draw a circular background behind the icon. |
23+
| `size` | `sm` `md` `lg` `xl` `1x` `2x` `3x` `4x` | The default size is 1em, so icon size matches text size. `2x`, etc, are multiples of font size. `sm`, `md`, etc are fixed pixel-based sizes. |
24+
| `color` | `base` `lightest` `lighter` `darker` `darkest` `complement` `accent` `accent` `critical` `important` `moderate` `success` `info` `default` | The color variant to use. This draws from your theming layer to color the icon. This will set icon color or background color (if `circled` is true). |
25+
| `circled` | boolean attribute | Whether to draw a circular background behind the icon. |
2626

2727
## Icon sets
2828

@@ -86,10 +86,23 @@ To updating an existing icon set, you use the same `addIconSet` function. The f
8686

8787
## Variables
8888

89-
There are several powerful variables available to hook into and override default styles.
89+
There are several powerful ways to hook into and override default styles.
9090

91-
- Color: the `color` attribute is available to pull icon color from your theming layer. For more fine-grained control, `--pfe-icon--color` is available to override the color of a specific icon or sets of icons and will be applied to either the SVG lines or the background of the circle (if circled). [Examples][color-examples]
92-
- Background color: the `color` attribute is available to pull background color from your theming layer. For more fine-grained control, `--pfe-icon--BackgroundColor` is available to override the background color of a specific icon or sets of icons. Be sure to set `--pfe-icon--context` to the appropriate context if you are setting the background-color.
91+
- Color: the `color` attribute is available to pull icon color from your theming layer. For more fine-grained control, `--pfe-icon--color` is available to override the color of a specific icon or sets of icons and will be applied to the SVG. [Examples][color-examples]
92+
- Background color: the `color` attribute is available to pull background color from your theming layer. For more fine-grained control, `--pfe-icon--BackgroundColor` is available to override the background color of a specific icon or sets of icons. Be sure to set `--pfe-icon--context` to the appropriate context if you are setting the background-color or use the more fine-grained `--pfe-icon--color` to set a specific color on the SVG lines.
93+
94+
| Theme hook | Description | Default |
95+
| -------------- | ----------- | ------- |
96+
| `--pfe-icon--size` | The height and width of the icon | var(--pfe-theme--icon-size, 1em) |
97+
| `--pfe-icon--spacing` | | var(--pfe-theme--container-spacer, 1rem) |
98+
| `--pfe-icon--Padding` | Padding around the icon | 0 (when circled .5em) |
99+
| `--pfe-icon--BackgroundColor` | Background color for when the icon is circled | transparent |
100+
| `--pfe-icon--context` | Icon context when `--pfe-icon--BackgroundColor` is used | light |
101+
| `--pfe-icon--BorderColor` | Border color when icon is circled | var(--pfe-icon--BackgroundColor, transparent) |
102+
| `--pfe-icon--BorderWidth` | Thickness of the border when circled | var(--pfe-theme--ui--border-width, 1px) |
103+
| `--pfe-icon--color` | Sets the color of the SVG lines | var(--pfe-icon--Color, var(--pfe-broadcasted--text, #3c3f42)) |
104+
| *Deprecated* |
105+
| `--pfe-icon--Color` | Deprecated | var(--pfe-broadcasted--text, #3c3f42) |
93106

94107
## Test
95108

@@ -107,7 +120,7 @@ From the PFElements root directory, run:
107120

108121
## Code style
109122

110-
Card (and all PFElements) use [Prettier][prettier] to auto-format JS and JSON. The style rules get applied when you commit a change. If you choose to, you can [integrate your editor][prettier-ed] with Prettier to have the style rules applied on every save.
123+
All PFElements use [Prettier][prettier] to auto-format JS and JSON. The style rules get applied when you commit a change. If you choose to, you can [integrate your editor][prettier-ed] with Prettier to have the style rules applied on every save.
111124

112125
[prettier]: https://github.com/prettier/prettier/
113126
[prettier-ed]: https://prettier.io/docs/en/editors.html

‎elements/pfe-icon/docs/index.md‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@ tags:
99
- component
1010
---
1111
<style>
12-
main.basic pfe-icon {
13-
font-size: 4rem;
14-
}
15-
1612
main.basic pfe-icon[circled] {
1713
margin-right: 8px;
1814
margin-bottom: 8px;
@@ -28,16 +24,16 @@ tags:
2824

2925
Icon delivers icon elements that can be sized, colored, and circled. Other icon sets can also be registered and added for use.
3026

31-
<pfe-icon icon="rh-leaf"></pfe-icon>
32-
<pfe-icon icon="rh-protected" color="complement"></pfe-icon>
33-
<pfe-icon icon="rh-code" color="accent"></pfe-icon>
34-
<pfe-icon icon="rh-cloud" color="darkest"></pfe-icon>
27+
<pfe-icon icon="rh-leaf" size="4x"></pfe-icon>
28+
<pfe-icon icon="rh-protected" size="4x" color="complement"></pfe-icon>
29+
<pfe-icon icon="rh-code" size="4x" color="accent"></pfe-icon>
30+
<pfe-icon icon="rh-cloud" size="4x" color="darkest"></pfe-icon>
3531

3632
### Circled
37-
<pfe-icon icon="rh-sports-play" circled></pfe-icon>
38-
<pfe-icon icon="rh-fast-jet" circled color="complement"></pfe-icon>
39-
<pfe-icon icon="rh-shopping-cart" circled color="accent"></pfe-icon>
40-
<pfe-icon icon="rh-server-stack" circled color="darkest"></pfe-icon>
33+
<pfe-icon icon="rh-sports-play" size="4x" circled></pfe-icon>
34+
<pfe-icon icon="rh-fast-jet" size="4x" circled color="complement"></pfe-icon>
35+
<pfe-icon icon="rh-shopping-cart" size="4x" circled color="accent"></pfe-icon>
36+
<pfe-icon icon="rh-server-stack" size="4x" circled color="darkest"></pfe-icon>
4137
:::
4238

4339
::: section

‎elements/pfe-icon/src/pfe-icon.scss‎

Lines changed: 72 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,15 @@ $LOCAL-VARIABLES: (
99
spacing: pfe-var(container-spacer),
1010
Padding: 0,
1111
BackgroundColor: transparent,
12-
BorderWidth: 0,
13-
// Legacy variable for changing icon color
12+
BorderWidth: pfe-var(ui--border-width),
13+
// @TODO Deprecated variable for changing icon color
1414
Color: pfe-broadcasted(text),
1515
);
1616

1717
// Nested internal variables (pfe-local calls), maps cannot "self-reference"
18-
$LOCAL-VARIABLES: map-deep-merge($LOCAL-VARIABLES, (
19-
// Current variable for updating icon color
20-
color: pfe-local(Color),
21-
));
22-
23-
// Nested internal variables (pfe-local calls), maps cannot "self-reference"
24-
$LOCAL-VARIABLES: map-deep-merge($LOCAL-VARIABLES, (
25-
BorderColor: pfe-local(color),
26-
));
18+
$LOCAL-VARIABLES: map-deep-merge($LOCAL-VARIABLES, ( // Current variable for updating icon color
19+
color: pfe-local(Color),
20+
BorderColor: pfe-local(BackgroundColor)));
2721

2822
/// ===========================================================================
2923
/// Component Specific SASS Vars
@@ -33,7 +27,18 @@ $LOCAL-VARIABLES: map-deep-merge($LOCAL-VARIABLES, (
3327
background-color: white !important;
3428

3529
& svg filter feFlood {
36-
flood-color: black !important;
30+
flood-color: black !important;
31+
}
32+
}
33+
34+
@mixin pfe-icon--sizing($desired-size) {
35+
// Respect var if user set CSS var, but fallback to sizing property map
36+
max-width: pfe-local(size, $fallback: $desired-size);
37+
max-height: pfe-local(size, $fallback: $desired-size);
38+
39+
svg {
40+
width: pfe-local(size, $fallback: $desired-size);
41+
height: pfe-local(size, $fallback: $desired-size);
3742
}
3843
}
3944

@@ -44,29 +49,30 @@ $LOCAL-VARIABLES: map-deep-merge($LOCAL-VARIABLES, (
4449
display: inline-block;
4550

4651
// Content-box model supports sizing while preserving aspect ratio (despite layout)
47-
box-sizing: content-box!important;
48-
max-width: pfe-local(size);
49-
width: fit-content !important;
50-
max-height: pfe-local(size);
51-
height: fit-content !important;
52-
line-height: 0;
53-
54-
@at-root #{&}([block]) {
55-
display: block;
56-
margin-bottom: pfe-local(spacing);
57-
margin-top: pfe-local(spacing);
58-
59-
&:first-child {
60-
margin-top: 0;
61-
}
62-
}
63-
52+
box-sizing: content-box !important;
53+
width: fit-content !important;
54+
height: fit-content !important;
55+
line-height: 0;
56+
57+
max-width: pfe-local(size);
58+
max-height: pfe-local(size);
59+
6460
svg {
6561
width: pfe-local(size);
6662
height: pfe-local(size);
6763
}
6864
}
6965

66+
:host([block]) {
67+
display: block;
68+
margin-bottom: pfe-local(spacing);
69+
margin-top: pfe-local(spacing);
70+
71+
&:first-child {
72+
margin-top: 0;
73+
}
74+
}
75+
7076
:host(:not(.load-failed)) {
7177
vertical-align: middle;
7278
border-radius: 50%;
@@ -75,13 +81,12 @@ $LOCAL-VARIABLES: map-deep-merge($LOCAL-VARIABLES, (
7581
padding: pfe-local(Padding);
7682

7783
@include browser-query(ie11) {
78-
@include greyscale-fallback();
84+
@include greyscale-fallback();
7985
}
8086

81-
8287
// Edge 12+ CSS
8388
@include browser-query(edge12) {
84-
@include greyscale-fallback();
89+
@include greyscale-fallback();
8590
}
8691

8792
svg image {
@@ -101,7 +106,7 @@ $LOCAL-VARIABLES: map-deep-merge($LOCAL-VARIABLES, (
101106

102107
@for $num from 2 through 4 {
103108
:host([size="#{$num}x"]) {
104-
--pfe-icon--size: #{$num}em;
109+
@include pfe-icon--sizing(#{$num}em);
105110
}
106111
}
107112

@@ -115,87 +120,66 @@ $sizing: (
115120
// @TODO: Sort out sizing
116121
@each $label, $size in $sizing {
117122
:host([size="#{$label}"]) {
118-
// Respect var if user set CSS var, but fallback to sizing property map
119-
$desired-size: pfe-local(size, $fallback: $size);
120-
max-width: $desired-size;
121-
max-height: $desired-size;
122-
123-
svg {
124-
width: $desired-size;
125-
height: $desired-size;
126-
}
123+
@include pfe-icon--sizing($size);
127124
}
128125
}
129126

127+
// @TODO May need to add feedback--darkest colors too
128+
:host([circled]:not([circled="false"])) {
129+
padding: pfe-local(Padding, $fallback: .5em);
130+
background-color: pfe-local(BackgroundColor, $fallback: pfe-var(surface--lightest));
131+
border-color: pfe-local(BorderColor, $fallback: pfe-local(BackgroundColor, $fallback: pfe-var(surface--border)));
132+
}
133+
130134
@each $level in (critical, important, moderate, success, info, default) {
131-
:host([color="#{$level}"]) {
135+
:host([color="#{$level}"]:not([circled])),
136+
:host([color="#{$level}"][circled="false"]) {
132137
filter feFlood {
133-
flood-color: pfe-local(
134-
color,
135-
$fallback: pfe-var(feedback--#{$level})
136-
);
138+
flood-color: pfe-local(color,
139+
$fallback: pfe-local(Color, $fallback: pfe-var(feedback--#{$level})));
137140
}
138-
@if $level != "moderate" {
141+
}
142+
143+
144+
/* Override circled items with color set to named property */
145+
:host([color="#{$level}"][circled]:not([circled="false"])) {
146+
// Respect var if user set CSS var, but fallback to property
147+
$desired-color: pfe-local(BackgroundColor, $fallback: pfe-var(feedback--#{$level}));
148+
background-color: $desired-color;
149+
border-color: pfe-local(BorderColor, $fallback: $desired-color);
150+
151+
@if $level !="moderate" {
139152
--pfe-icon--context: dark;
140153
}
141154
}
142155
}
143156

144157
@each $color in (lightest, base, darker, darkest, complement, accent) {
145-
:host([color="#{$color}"]) {
146-
--pfe-icon--context: #{pfe-var(surface--#{$color}--context)};
147-
158+
:host([color="#{$color}"]:not([circled])),
159+
:host([color="#{$color}"][circled="false"]) {
148160
filter feFlood {
149-
flood-color: pfe-local(
150-
color,
151-
$fallback: pfe-var(surface--#{$color})
152-
);
161+
flood-color: pfe-local(color, $fallback: pfe-local(Color, $fallback: pfe-var(surface--#{$color})));
153162
}
154163
}
155-
}
156164

157-
// May need Feedback Darkest colors too.
158-
159-
:host([circled]:not([circled="false"])) {
160-
padding: pfe-local(Padding, $fallback: .5em);
161-
border-width: pfe-local(BorderWidth, $fallback: pfe-var(ui--border-width));
162-
border-color: pfe-local(color, $fallback: pfe-var(surface--border));
163-
background-color: pfe-local(BackgroundColor, pfe-local(color, $fallback: pfe-var(surface--lightest)));
164165

165-
filter feFlood {
166-
flood-color: pfe-local(color);
167-
}
168-
}
169-
170-
/**
171-
* Override circled items with color set to named property
172-
*/
173-
@each $color in (lightest, base, darker, darkest, complement, accent) {
166+
/* Override circled items with color set to named property */
174167
:host([color=#{$color}][circled]:not([circled="false"])) {
168+
--pfe-icon--context: #{pfe-var(surface--#{$color}--context)};
169+
175170
// Respect var if user set CSS var, but fallback to property
176-
$desired-color: pfe-local(color, $fallback: pfe-var(surface--#{$color}));
177-
background-color: $desired-color;
178-
border-color: $desired-color;
179-
}
180-
}
181-
@each $level in (critical, important, moderate, success, info, default) {
182-
:host([color="#{$level}"][circled]:not([circled="false"])) {
183-
// Respect var if user set CSS var, but fallback to property
184-
$desired-color: pfe-local(backgroundColor, $fallback: pfe-var(feedback--#{$level}));
171+
$desired-color: pfe-local(BackgroundColor, $fallback: pfe-var(surface--#{$color}));
185172
background-color: $desired-color;
186-
border-color: $desired-color;
173+
border-color: pfe-local(BorderColor, $fallback: $desired-color);
187174
}
188175
}
189176

190-
:host(.load-failed) svg image {
191-
display: none;
192-
}
193-
194177
:host(.load-failed.has-fallback) svg,
195-
:host(.load-failed[on-fail="collapse"]) svg {
178+
:host(.load-failed[on-fail="collapse"]) svg,
179+
:host(.load-failed) svg image {
196180
display: none;
197181
}
198182

199183
:host(.load-failed[on-fail="collapse"]) {
200-
--pfe-icon--size: 0;
184+
@include pfe-icon--sizing(0);
201185
}

0 commit comments

Comments
 (0)