-
Notifications
You must be signed in to change notification settings - Fork 33
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
Localize Stripe links #222
base: master
Are you sure you want to change the base?
Conversation
Deploying organicmaps with Cloudflare Pages
|
8c5e444
to
5d229ff
Compare
<a href="{{ config.extra.stripe | safe }}" title="Credit/Debit Card"><img src="/images/donates/visa_mc.svg" alt="Credit/Debit Card"></a> | ||
<a href="{{ config.extra.stripe | safe }}" title="Apple Pay" id="applePay"><img src="/images/donates/apple.svg" alt="Apple Pay"></a> | ||
<a href="{{ config.extra.stripe | safe }}" title="Google Pay" id="googlePay"><img src="/images/donates/google.svg" alt="Google Pay"></a> | ||
<a href="https://donate.organicmaps.app/?locale={{ lang }}" title="Credit/Debit Card"><img src="/images/donates/visa_mc.svg" alt="Credit/Debit Card"></a> |
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.
- Do all our langs match stripe langs?
- What is the stripe behavior when a lang is not supported there?
- Is it possible to enable automatic lang detection without passing an explicit parameter, as it was mentioned on the stripe's documentation page?
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.
- Do all our langs match stripe langs?
They seem to be matching, including composed codes like pt-BR.
What is the stripe behavior when a lang is not supported there?
Stripe auto-detects the language.
Is it possible to enable automatic lang detection without passing an explicit parameter, as it was mentioned on the stripe's documentation page?
Automatic language detection works already out of the box in Stripe. This PR just do all the best to continue with the same language as seen on the web-site. What is the problem with that?
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 problem is that this approach requires some code/support, and may introduce unexpected issues with unsupported languages/wrong parameters, while automatic detection by stripe works automatically and doesn't require any changes.
What are the benefits enabled by merging this PR?
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.
Sorry, please provide any constructive arguments why this PR shouldn't be merged based on before/after analysis.
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.
his PR just do all the best to continue with the same language as seen on the web-site. What is the problem with that?
The wrong locale on our website will prohibit Stripe from autodetecting the correct locale that is not supported yet on our website. For example, a Greek user will see en OM website and will see en Stripe instead of el.
P.S. A shortcode can help to avoid copy-paste.
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 wrong locale on our website will prohibit Stripe from autodetecting the correct locale that is not supported yet on our website. For example, a Greek user will see en OM website and will see en Stripe instead of el.
Our web-site is already localized to almost all locales that Stripe supports. Greek localization will be added eventually if there is a demand. Our web-site and Stripe probably have different logic for auto-detecting locales. Plus our web-site provides explicit language choosers for users. From the user's prospective, the donation page is an integral part of the web-site. Preserving the same locale when navigating from one section of web-site to another sounds reasonable to me.
Not to mention that links to AppStores on this web-site also have includes the locale code (i.e.hl={{ lang }}
) since the beginning. Nobody complained, but for flimsy reasons we are not merging this PR.
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.
Any unsupported language will fall back to en on our website. I suggest to use automatic stripe language detection logic for en in this case, to help users see localized stripe pages when their languages are not yet supported on our website.
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.
Updated
5d229ff
to
45ef9e6
Compare
@@ -1,28 +1,33 @@ | |||
{% if lang == 'en' %} | |||
{% set stripe = 'https://donate.organicmaps.app' %} |
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.
set stripe = config.extra.stripe
for easier configurability?- nit: slash at the end.
<div class="donate_buttons"> | ||
{%- if lang == 'ru' %} | ||
<a href="https://donate.organicmaps.app?currency=rub" title="По карте МИР в рублях"><img id="mir" src="/images/donates/mir.svg" alt="По карте МИР в рублях"></a> | ||
<a href="https://t.me/OrganicMapsRu/29441" title="Telegram по карте МИР в рублях"><img id="telegramRu" src="/images/donates/telegram.svg" alt="Telegram по карте МИР в рублях"></a> | ||
{% endif -%} | ||
<a href="{{ config.extra.stripe | safe }}" title="Credit/Debit Card"><img src="/images/donates/visa_mc.svg" alt="Credit/Debit Card"></a> |
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.
Is safe
not needed anymore?
https://support.stripe.com/questions/supported-languages-for-stripe-checkout-and-payment-links