activemerchant / payment_icons

An easy to use library that allows you to manage and access payment icons
MIT License
142 stars 409 forks source link

Adding overstock citi card icons #1171

Closed anudeesh closed 2 months ago

anudeesh commented 3 months ago

Why are you adding this icons?

I'm adding/updating this icon(s) because shopify needs to show the overstock citi master card and citi credit card icons for payment.

Help us identify yourself

Link to the brand guidelines: https://github.com/activemerchant/payment_icons/blob/master/CONTRIBUTING.md?plain=1#L37

Checklist to add new icons

If this pull request is not adding new icons, you can remove this checklist.

Attach a screenshot of the icon along side the example Visa icon

Screenshot 2024-05-08 at 9 58 38 AM Screenshot 2024-05-08 at 9 59 13 AM

Tips how to create a screenshot

We have found free online SVG editor https://www.freecodeformat.com/svg-editor.php very useful to create one. Here is a sample code for you to verify that you icon appears properly along side the placeholder.

<!-- Change background color if needed to showcase your icon better -->
<style> body { background: black; } </style>

<!-- DO NOT DELETE EXAMPLE -->
<svg viewBox="0 0 38 24" xmlns="http://www.w3.org/2000/svg" role="img" width="38" height="24" aria-labelledby="pi-visa"><title id="pi-visa">Visa</title><path opacity=".07" d="M35 0H3C1.3 0 0 1.3 0 3v18c0 1.7 1.4 3 3 3h32c1.7 0 3-1.3 3-3V3c0-1.7-1.4-3-3-3z"/><path fill="#fff" d="M35 1c1.1 0 2 .9 2 2v18c0 1.1-.9 2-2 2H3c-1.1 0-2-.9-2-2V3c0-1.1.9-2 2-2h32"/><path d="M28.3 10.1H28c-.4 1-.7 1.5-1 3h1.9c-.3-1.5-.3-2.2-.6-3zm2.9 5.9h-1.7c-.1 0-.1 0-.2-.1l-.2-.9-.1-.2h-2.4c-.1 0-.2 0-.2.2l-.3.9c0 .1-.1.1-.1.1h-2.1l.2-.5L27 8.7c0-.5.3-.7.8-.7h1.5c.1 0 .2 0 .2.2l1.4 6.5c.1.4.2.7.2 1.1.1.1.1.1.1.2zm-13.4-.3l.4-1.8c.1 0 .2.1.2.1.7.3 1.4.5 2.1.4.2 0 .5-.1.7-.2.5-.2.5-.7.1-1.1-.2-.2-.5-.3-.8-.5-.4-.2-.8-.4-1.1-.7-1.2-1-.8-2.4-.1-3.1.6-.4.9-.8 1.7-.8 1.2 0 2.5 0 3.1.2h.1c-.1.6-.2 1.1-.4 1.7-.5-.2-1-.4-1.5-.4-.3 0-.6 0-.9.1-.2 0-.3.1-.4.2-.2.2-.2.5 0 .7l.5.4c.4.2.8.4 1.1.6.5.3 1 .8 1.1 1.4.2.9-.1 1.7-.9 2.3-.5.4-.7.6-1.4.6-1.4 0-2.5.1-3.4-.2-.1.2-.1.2-.2.1zm-3.5.3c.1-.7.1-.7.2-1 .5-2.2 1-4.5 1.4-6.7.1-.2.1-.3.3-.3H18c-.2 1.2-.4 2.1-.7 3.2-.3 1.5-.6 3-1 4.5 0 .2-.1.2-.3.2M5 8.2c0-.1.2-.2.3-.2h3.4c.5 0 .9.3 1 .8l.9 4.4c0 .1 0 .1.1.2 0-.1.1-.1.1-.1l2.1-5.1c-.1-.1 0-.2.1-.2h2.1c0 .1 0 .1-.1.2l-3.1 7.3c-.1.2-.1.3-.2.4-.1.1-.3 0-.5 0H9.7c-.1 0-.2 0-.2-.2L7.9 9.5c-.2-.2-.5-.5-.9-.6-.6-.3-1.7-.5-1.9-.5L5 8.2z" fill="#142688"/></svg>

<!-- TODO: insert your icon here -->
<YOUR SVG CODE>

<br>
<!-- TODO: insert your icon here -->
<YOUR SVG CODE>
</br

If the icons are intended for use by Shopify, please provide the following info:

Who are you working with at Shopify? (avoid adding personal details, provide github handle(preferred) or first name and last name)

What's the expected date of this change to deploy on Shopify?

adeniyiao commented 3 months ago

Hi @anudeesh

Thanks for submitting the PR. Some issues to address

{:message=>"The 'overstock_citi_cobrand' SVG file should have a <title> tag as first child of the root <svg> tag"}.
Expected: "title"
  Actual: "g"

Also two issues to fix as per https://github.com/activemerchant/payment_icons/blob/master/CONTRIBUTING.md#name (it should be lowercase alpha characters only)

image

anudeesh commented 3 months ago

Updated the PR as per the comments. Thanks

anudeesh commented 2 months ago

Please make sure to review the contribution guidelines for both your SVG images. They are both missing the border radius and outside border

Updated the SVGs as requested. Attached the new screenshots of the images above

anudeesh commented 2 months ago

Updated the PR as per comments. Attached the new screenshots as well. Thanks

anudeesh commented 2 months ago

Thanks! These look a lot better. Would you mind expanding/outlining the border stroke in the SVG files?

Sorry, I didn't get that. Do you want me to increase the border width more? Currently they are 1px as mentioned in the icon guidelines.

Lovedanihonjin commented 2 months ago

Thanks! These look a lot better. Would you mind expanding/outlining the border stroke in the SVG files?

Sorry, I didn't get that. Do you want me to increase the border width more? Currently they are 1px as mentioned in the icon guidelines.

The border width is perfect. You just need to convert your border stroke into a vector object. Here's a how-to in Figma, and I attached a reference below.

Screenshot 2024-05-01 at 8 43 20 PM
anudeesh commented 2 months ago

Thanks! These look a lot better. Would you mind expanding/outlining the border stroke in the SVG files?

Sorry, I didn't get that. Do you want me to increase the border width more? Currently they are 1px as mentioned in the icon guidelines.

The border width is perfect. You just need to convert your border stroke into a vector object. Here's a how-to in Figma, and I attached a reference below. Screenshot 2024-05-01 at 8 43 20 PM

Updated the SVGs as requested

anudeesh commented 2 months ago

Unfortunately the SVG's are not using the correct inner and outer radiuses. I have attached an example below. The yellow lines indicate what the border radius needs to be. Here is an SVG you can reference: https://github.com/activemerchant/payment_icons/blob/master/app/assets/images/payment_icons/visa.svg

visa

Screenshot 2024-05-07 at 5 25 04 PM

Corrected the border as requested