Closed KarlisJ closed 7 months ago
@KarlisJ I've re-compiled icons for the payment methods that I can see on the WCPay settings, and have included Klarna as I know we're about to release support for it. They're all 64x40px with no corner radius or border. You can download them here!
For the css I put this together quickly, but feel free to rewrite if there's a cleaner way to add the radius and borders
.payment-method__icon, .express-checkout__icon {
border-radius: 3px;
width: 64px;
height: 40px;
position: relative;
}
.payment-method__icon:after, .express-checkout__icon:after {
border-radius: 3px;
width: 64px;
height: 40px;
content: "";
display: block;
box-shadow: inset 0 0 0 1px rgba(0,0,0,0.25);
position: absolute;
top: 0; left: 0;
}
.payment-method__icon img, .express-checkout__icon img {
width: 64px;
height: 40px;
border-radius: 3px;
}
For the credit card grid of icons (Visa, Mastercard, Amex, CB) I've written a quick css grid to display it at the same overall width/height of the single icons
.payment-method__grid {
display: grid;
width: 64px;
height: 40px;
gap: 4px 5px;
grid-template-columns: repeat(auto-fill, 29px);
}
.payment-method__grid > div, .payment-method__grid > div:after, .payment-method__grid > div > img {
width: 30px;
height: 19px;
}
and the html just wraps the 4 icons in the .payment-method__grid
div
<div class="payment-method__grid">
<div class="payment-method__icon"><img src="Visa.svg"></div>
<div class="express-checkout__icon"><img src="Mastercard.svg"></div>
<div class="express-checkout__icon"><img src="American Express.svg"></div>
<div class="express-checkout__icon"><img src="Cartes Bancaires.svg"></div>
</div>
Here's a preview of it in action
Let me know if there's any payment methods that I've missed and I can get them added as well.
The path forward: via CSS, see p1696881086500429/1696355944.423479-slack-C04VCT34PCP
Thank you @bradyv!
I'll take your CSS and create a properly re-usable icon component so that we don't have to remember to apply the correct style whenever we need icons on a new page. So far we haven't been consistent with and I'd like to make it future-proof.
@bradyv I've checked the provided assets against what we have (and use) in our codebase, and some assets are missing:
Could you re-compile these to match the new pattern, please? It's not a blocker right now, as I'll be working on CSS rules, and we can plug in the remaining assets at the last moment before merging PR.
@bradyv I have concern with Credit Card icon (the grid of 4 individual icons).
In some views, we display the icons smaller than their original size. You can test that by setting your WCpay Dev plugin to act as woopayments client is disconnected.
The logos here are set with height 24px. Given that we set exact dimension for the grid items, I think we can't reliably resize entire grid.
Now that I'm writing this I realise that this also affect all icons. We should probably make the border thinner when displaying the icons smaller?
@KarlisJ here's a fresh zip that includes the missing icons mentioned above. I renamed them all to match the current versions so that you don't have to worry about that again.
For the smaller icons, because they're all svg, we should be able to just scale them to display at 40x25px which will match the aspect ratio of the regular size. I think the border at 1px is fine for both sizes.
@KarlisJ Per our chat here's another version of the cc.svg
icon as a single asset. I don't think it's as nice as the grid version but this should work if we need to compromise so that we can draw the borders properly.
Download payments.zip
@bradyv I'm reluctant to continue this approach. As smart as your suggested CSS is, I think it doesn't fit well without our existing codebase.
As you already review the PR, I've created a component that would force the CSS globally. But it also means, we are forcing the width & height for the icons globally as well.
However, in many cases we are displaying them in smaller size
As great as the CSS-based solution is, it takes a lot of effort to implement at the current state of the codebase. Mainly, the border relies on width and height properties, but in many cases, we need the icons smaller than original size which introduces need for wider refactors.
I've implemented https://github.com/Automattic/woocommerce-payments/pull/7522/commits/980317a7598ebfe7386d1a6d9143b9a6009edf24, but it doesn't scale well as we leave ourselves locked in certain size options.
Given that there's not much capacity on team Pulsar side, I suggest we simplify the approach. Can we create the border inside the SVGs? Here's my reasoning:
@KarlisJ I understand your concern. It's been a while since I wrote any front-end markup and I realized overnight that the code I provided is far more complicated than it needs to be, for that I apologize! We can get the same results by applying an outline
and outline-offset
directly to the image.
img {
outline: 1px solid rgba(0,0,0,0.25);
outline-offset: -1px;
border-radius: 3px;
}
With this we can set the image width as we need it + get rid of all the extra css added to the payment-method__icon
, and remove the payment-method__icon_x
entirely.
Given that there's not much capacity on team Pulsar side, I suggest we simplify the approach. Can we create the border inside the SVGs?
Our preference is to not bake the borders into the svg assets because it gives us greater control if we ever want to make any changes to the border, and it allows us to style the assets as needed for different applications.
@bradyv This works fantastic! Thank you!
Can we try to use this outline for the grid for CC? I'm not sure how well we can work with dimensions using the grid and spacings.
If that's too complicated, I'll use the combined asset for CC that you provided, but I can create a follow up issue to combine individual assets into grid.
@KarlisJ Absolutely, that css should work much better with the grid, as we don't have to draw pseudo elements and hard code any math / aspect ratios.
Here's the scss with some slight modifications
.payment-method__icon {
img {
width: 64px;
outline: 1px solid rgba(0,0,0,0.25);
outline-offset: -1px;
border-radius: 3px;
display: block;
}
}
.payment-method__grid {
display: grid;
width: 64px;
height: 40px;
gap: 2px;
grid-template-columns: repeat(auto-fill, 30px);
img {
width: 30px;
}
}
and the html for the grid
<div class="payment-method__grid">
<div class="payment-method__icon">
<img src="">
</div>
<div class="payment-method__icon">
<img src="">
</div>
<div class="payment-method__icon">
<img src="">
</div>
<div class="payment-method__icon">
<img src="">
</div>
</div>
Re-opening based on pbUcTB-4el-p2#minor-icon-styling-issue
@Automattic/gamma: requesting re-triage for this issue.
Describe the bug
For PM icons that have filled background, the border applied with CSS doesn't perfectly align with the fill.
This is no an issue with icons with transparent backgrounds (which is most of cases)
Screenshots
To Reproduce
payments->settings
Expected behavior
There shouldn't be a gap between the background and border.
It can be achieved in several ways:
box-shadow
notborder
)