-
Notifications
You must be signed in to change notification settings - Fork 450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add payrexx requested icons #1150
Add payrexx requested icons #1150
Conversation
Hi @sven-otziger, |
Bank transfer is our internally developed payment method to accept payment by bank transfer. Customers receive an invoice for their purchase. Our system then checks the transactions of our bank account and adjusts the status in our system accordingly. |
Cc: @adeniyiao—what are your thoughts here? |
I will suggest using another name instead of generic bank transfer, so if there is a custom name for the bank transfer payment method, please use it. Also, please change the name without any special characters, please see guidelines |
Hi @hellicarusprime @adeniyiao |
Hi @sven-otziger Can you please update the name of the SVG to be the same as the name entered in db/payment_icons.yml? The SVG still contains underscores. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI checks failed due to {:message=>"The 'centi' SVG file does not have the appropriate <title> value"}.
Please fix the SVG format and make sure it follows the following format.
Also, please rebase when committing your next change.
Included the role in the SVG
478d845
to
bf7af29
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI checks failed with the following errors `{:message=>"The 'payrexxbanktransfer' SVG file does not have the appropriate <title> value"}.' Please ensure that the SVG format meets the requirements and resolve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI checks failed again with the following errors: PaymentIconTest#test_Every_payment_SVG_meets_accessibility_requirements [test/unit/payment_icon_test.rb:81]: {:message=>"The 'payrexxbanktransfer' SVG file does not have the appropriate 'id' value on the <title> tag"}.
You can find the SVG format requirements here.
Awesome. Thanks for your changes. Approved. Next deploy is on June 5. |
@Lovedanihonjin Many thanks |
Why are you adding these icons?
I'm adding/updating these icons because we as a payment service provider are offering these payment methods and would like to offer them in Shopify as well.
Help us identify yourself
Link to the brand guidelines:
Checklist to add new icons
db/payment_icons.yml
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
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)
We don't have a direct contact at Shopify. We are just acting as a Shopify partner.
What's the expected date of this change to deploy on Shopify?
We would like the icons released at your earliest convenience.