-
Notifications
You must be signed in to change notification settings - Fork 84
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
feat: use XBlockI18NService js translations | FC-0012 #441
Conversation
Thanks for the pull request, @OmarIthawi! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate. |
16bf96a
to
202cff1
Compare
2c8da86
to
b9afb1e
Compare
b9afb1e
to
7ef92fc
Compare
@shadinaif fyi. |
@itsjeyd all the tests have passed, so removing the label. |
Thanks @e0d! |
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.
def validate_field_data(self, validation, data): | ||
# Validate custom parameters is a list. | ||
if not isinstance(data.custom_parameters, list): | ||
_ = self.runtime.service(self, "i18n").ugettext | ||
_ = self.runtime.service(self, 'i18n').ugettext |
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.
no need
_ = self.runtime.service(self, 'i18n').ugettext | |
_ = self.runtime.service(self, "i18n").ugettext |
@@ -257,6 +258,7 @@ class LtiConsumerXBlock(StudioEditableXBlockMixin, XBlock): | |||
""" | |||
|
|||
block_settings_key = 'lti_consumer' | |||
i18n_js_namespace = 'LtiI18N' |
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 make file already uses a name, please change this one
i18n_js_namespace = 'LtiI18N' | |
i18n_js_namespace = 'XBlockLtiConsumerI18N' |
@OmarIthawi Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future. |
Thanks @shadinaif. I've closed it. You're right about the minor notes, Ahmed was being onboarded and I just opened the PR for him. Please re-create the PR with your notes resolved. |
thank you @OmarIthawi , replaced by this PR #452 |
Implement OEP-58 JavaScript translations
atlas pull
for the edx-platform and its plugins FC-0012 edx-platform#33166atlas pull
for XBlock translations | FC-0012 edx-platform#33698Testing
Reference
This pull request is part of the FC-0012 project which implements the Translation Infrastructure update OEP-58.