search

eBay / skin

Pure CSS framework designed & developed by eBay for a branded, e-commerce marketplace.
https://ebay.github.io/skin/
MIT License
178 stars 67 forks source link

230302 "-colored" payment card icon strokes not displaying properly in dark mode #2000

Closed ratinsl closed 3 months ago

ratinsl commented 1 year ago

While working with Arthur through icon updates/fixes for the new library, we noticed that the stroke color of the the "-colored" card icons were not updating from black to white in dark mode because of some issues with "stroke" attributes. This makes icons look 1 px short on the top and bottom when placed next to other card icons that have a colored such as blue for AMEX.

We need to do some further digging and experimentation at a later date for how we can adjust the script or build so the stroke/border shape can update between light and dark mode so the heights/radius of the cards look correct.

image
ratinsl commented 5 months ago

Hey @ArtBlue, do we want to circle this discussion back up? Is there anything that can be done with the script to account for the stroke in dark mode?

ArtBlue commented 5 months ago

@ratinsl , this was a while ago, so I hope I'm capturing the context accurately. When the colored icons were originally proposed, we brought up the fact that dark mode would be an issue, and if I recall correctly, based on our initial research (and we came to an agreement on this at the time) was that the colored icons would not change at all in dark mode. They can potentially have a number of unique colors in multiple paths with multiple fill and stroke, which makes them impossible to make dynamic for the purpose of changing them based on dark mode.

I'm really not sure where we go from here. Technically, I'm not sure what can be done since -colored icons cannot be dynamic.

ratinsl commented 5 months ago

Yes, all the above is correct! At the time we were discussing if there was anything in the script that could help but doesn't sound like it... The only thing that could potentially work and question is if it is worth the effort... is exporting all of the card assets without a border and applying the border on the skin side. Is there a world where we can have -colored icons that don't change color between light/dark mode, but an added border on your side can change between light/dark mode?

However, that would only get us so far because there are some cards that don't have borders on purpose because the card color already has enough affordance... in the case of AMEX above. Sounds like if this where the route we wanted to go, we'd have to give every card a border?

ArtBlue commented 5 months ago

@ratinsl , there could be a way to make some portions of svg icons dynamic (such as bounding stroke) and address this issue. The most obvious would be along the lines of what you are suggesting. It will require some manual effort both on your part (at minimum you'd need to regenerate and maintain some of the icons with a bounding stroke) and ours, which we'd want to minimize to avoid errors and having to document/remember steps for people who may take on this responsibility other than us. Let's discuss offline.

agliga commented 3 months ago

@ArtBlue Any updates on this?

ArtBlue commented 3 months ago

@ratinsl , the last few comments we had regarding this offline was about fixing the Venmo icon, correct? I believe the simplest fix we landed on then, which I implemented for the Venmo icon (https://github.com/eBay/skin/pull/2269/files#diff-271cc2abc9566a2ee5ddd8eda503299b38cadfe7af15766118f585d1d5277448R8171) and subsequently for other similar icons was to use a stroke="1", correct? If so, we can close this issue, right?

ratinsl commented 3 months ago

@ArtBlue yes, that is correct!

ArtBlue commented 3 months ago

Great! Since this was just the issue to come up with a solution and that was already done, I'm closing this issue. When you're ready to provide new icons that include stroke-width="1", we can work to get those changes implemented. Thanks!