[DSS-157] Avatar - Custom Icon and Color

[anechol]

Description

Adds props to allow for background and foreground (icon stroke/fill) color for badges. Attempting to provide a backwards compatible solution to not disturb what's already been used in kp.

Screenshots

Screen.Recording.2022-10-14.at.10.21.02.AM.mov

Testing in sage-lib

Rails

• View Avatar in Rails and React
• Check that new props work as expected

Testing in kajabi-products

(LOW) Adds props to allow for background and foreground (icon stroke/fill) color for badges.

Related

DSS-157

[philschanely]

Looks great!

Not blocking, but what do you think about restructuring so that rather than badge, badgeColor, and badgeIcon we just have badge but it can accept just true (for default badge) or a hash/object with optional icon and color properties for encapsulating the custom settings?

[pixelflips]

I can't quite work out if it's a Storybook issue or not, but when I set the badge to true and adjust the badge color and icon, it changes the icon color and not the background color.

Happy to pair if it's not obvious.

[teenwolfblitzer]

👍 LGTM, with one note for the badge appearance:

Requirements for the issue are unclear on this, but the Figma mocks are displaying various social media logo badges with varying background color fills.

The issue that @pixelflips is seeing is because the icon examples in Rails are using -filled variants. If the intent will be to allow the badge_color to affect the background, then we'll have to ask Design to add those versions of the icons as well. Otherwise, it's working as intended.

[anechol]

@pixelflips @monkeypox8 Thanks for the feedback here. I just spoke to Quinton about this issue and yes, it looks like this is going to have to be brought up to design. Filled icons are working as intended, but those with only strokes are going to look awkward. Going to bring this up in the team sync for ideas.

[anechol]

Per feedback from the team and design, I'll be implementing a solution for both a foreground (icon stroke/fill) and background color.

[pixelflips]

Code looks great, but I am seeing an issue with Storybook. When setting Badge to true and then trying to add a string for the badgeBackgroundColor the screen goes blank and an error shows in the console.

Screen.Recording.2022-10-12.at.9.02.01.AM.mov

[anechol]

Code looks great, but I am seeing an issue with Storybook. When setting Badge to true and then trying to add a string for the badgeBackgroundColor the screen goes blank and an error shows in the console.

Screen.Recording.2022-10-12.at.9.02.01.AM.mov

Yeah @pixelflips this is a weird issue, and I think it's Storybook related, but can't confirm entirely. If you set the background color first and then set badge to true, then it works as inspected.

[pixelflips]

While continuing to review and changing up the controls, also noticed that the icons seem to break out in some cases. Will drop a screenshot so you can see the controls.

[pixelflips]

Yeah @pixelflips this is a weird issue, and I think it's Storybook related, but can't confirm entirely. If you set the background color first and then set badge to true, then it works as inspected.

@anechol it is weird for sure, maybe @ju-Skinner or @philschanely have thoughts on the error I mentioned above. 🤞🏼

[teenwolfblitzer]

Yeah @pixelflips this is a weird issue, and I think it's Storybook related, but can't confirm entirely. If you set the background color first and then set badge to true, then it works as inspected.

Haven't had a chance to dig into it, but best I can tell is that the style conditional rendering the CSS var in the component is causing the issue.

[anechol]

@monkeypox8 Thanks for the pointer. I adjusted the conditional so this should work fine in storybook now.

[anechol]

While continuing to review and changing up the controls, also noticed that the icons seem to break out in some cases. Will drop a screenshot so you can see the controls.

Ahh thanks for pointing this out @pixelflips . I might have to look into this further as I'm not sure what's a good solution to handle every use case for every icon in the library 🤔

[teenwolfblitzer]

👍 LGTM! Left a note on one cleanup item to consider.

[teenwolfblitzer]

(non-blocking) You can also drop the enclosing parentheses () here now that the conditional has been removed.

[teenwolfblitzer]

I might have to look into this further as I'm not sure what's a good solution to handle every use case for every icon in the library 🤔

Agreed; this is going to be pretty difficult to solve without a change to our icon system sizing, so I'm inclined to suggest we leave it for another day if the group's on board. Right now the use case for custom badges is limited. But if it does become an issue, we could always apply a 90% scale transform or something similar, though it'd have to be across the board.

[ju-Skinner]

great job. Thanks for adding specs.

[cameronsimony]

👍 👍