-
Notifications
You must be signed in to change notification settings - Fork 737
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
Respect AAQ configuration in widgets #6297
Conversation
bab96ea
to
71984ae
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.
Just a question about your choice of concatenation for Jinja strings related to localization. Otherwise good with me.
{% elif aaq_context.has_public_forum %} | ||
<p>{{ _('Still need help? Continue to ask your question on our forums.') }}</p> | ||
{% endif %} | ||
<p>{{ _('Still need help? Continue to ' + ('contact our support team' if has_support else 'ask your question on our forums') + '.') }}</p> |
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.
Will this allow the 'contact our support team' and 'ask your question on our forums' text to be translated easily? I'm not sure this concatenation method is better than using format
. Open to thoughts.
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.
I think there's a valid point for the string extraction process since this is evaluated dynamically. That said, I am not sure how format
can help in this case though. I am going to separate the strings in multiple lines.
{% else %} | ||
{% set link_name = "aaq-widget.aaq-step-3" %} | ||
{% set next_step = url('questions.aaq_step3', product_slug=aaq_context.product_slug) %} | ||
<p>{{ _('Still need help? Sign in to ' + ('contact our support team' if has_support else 'ask your question on our forums') + '.') }}</p> |
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.
Same as above.
71984ae
to
39ebb01
Compare
Updated per comment. Merging this to proceed with testing |
No description provided.