Skip to content
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 i18n support to the XBlock #6

Closed
wants to merge 4 commits into from
Closed

Add i18n support to the XBlock #6

wants to merge 4 commits into from

Conversation

iamjazzar
Copy link
Contributor

@iamjazzar iamjazzar commented Nov 30, 2018

TASK: SE-285

Test Instructions:

  • Go through the commits one by one.
  • Check new added management commands they work as expected and generates the desired output.
  • Link the XBlock locally to some Transifex repo (Code be any repo)
  • Push the translations.
  • Translate and Pull the translations from Transifex.
  • Install the XBlock on your local Studio deployment.
  • Make sure the dummy translations and your pulled translations are reflected on the XBlock.
  • Make sure that no strings left untranslated in the repo.

Copy link

@SSPJ SSPJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything I've reviewed so far looks nice, @AhmedAljazzar !

I haven't been able to check the i18n translation itself because I am having trouble installing this into my docker devstack. I've been following the instructions I found on the mailing list and in the tutorial. If you happen to know any tricks, that'd be helpful.

Also I could use a bit more explanation around Transifex repo. How do I get one of those? Do I need an account on their site?

I may have some more comments when I come back to this Monday, but I wanted to give you something to get started.

http://edx.readthedocs.org/projects/xblock-tutorial/en/latest/edx_platform/edx_lms.html
The template uses django-statici18n to provide translations to static javascript
using `gettext`.
The included Makefile contains targets for extracting, compiling, validating etc.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have the steps listed in order here, as you did with the Testing with Docker section

Makefile Outdated
help: ### Display this help message
@echo "Please use \`make <target>' where <target> is one of"
@perl -nle'print $& if m{^[a-zA-Z_-]+:.*?## .*$$}' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m %-25s\033[0m %s\n", $$1, $$2}'
detect_changed_source_translations:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this one intentionally without a help text?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. This module's look and behavior was fetched as is from cookiecutter-xblock, the module that edX is using as a template to create your new XBlocks.

extract_translations: symlink_translations ## extract strings to be translated, outputting .po files
cd $(PACKAGE_NAME) && i18n_tool extract
mv $(EXTRACTED_DJANGO) $(EXTRACTED_TEXT)
if [ -f "$(EXTRACTED_DJANGOJS)" ]; then cat $(EXTRACTED_DJANGOJS) >> $(EXTRACTED_TEXT); rm $(EXTRACTED_DJANGOJS); fi
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quick comment explaining why this line exists would be nice, something like "Append JS translations to the .po file".

@SSPJ
Copy link

SSPJ commented Dec 3, 2018

When importing to studio and clicking edit, it says Error: Invalid block tag on line 17: 'trans'. Did you forget to register or load this tag? This is because {% load i18n %} has to go in every HTML file.

If I add the translation tag, then I get Error: [Errno 2] No such file or directory: '/edx/src/xblock-html/html_xblock/static/js/html_xblock.js' . I'm not sure if this is because the file is actually missing or if I have forgotten to run a command to generate it. Could you update your instructions to be more detailed?

@iamjazzar
Copy link
Contributor Author

When importing to studio and clicking edit, it says Error: Invalid block tag on line 17: 'trans'. Did you forget to register or load this tag? This is because {% load i18n %} has to go in every HTML file.

Good catch. I pushed the fix.

If I add the translation tag, then I get Error: [Errno 2] No such file or directory: '/edx/src/xblock-html/html_xblock/static/js/html_xblock.js' . I'm not sure if this is because the file is actually missing or if I have forgotten to run a command to generate it. Could you update your instructions to be more detailed?

My bad, should be working now.

@otherpirate
Copy link

otherpirate commented Dec 18, 2018

Hi,

I'm trying to validate the locates but I'm not sure how can I test it.

What I did until now:

  1. Execute xblock-html make run
  2. Set the devstack up make dev.up.all
  3. Change language to pt-br at http://localhost:18010/update_lang
  4. Access course details page http://localhost:18010/course/course-v1:edX+E2E-101+course
  5. There is three wrong text's:
  • Start Date should be Data de Inicio
  • Pacing Type should be Estimulação
  • Checklists should be Verificações

I have found these issues with pt-br and eo, but I'm not sure if a did the setup in the right way.

@@ -222,3 +229,28 @@ def get_editable_fields(self):
fields.append(field_info)

return fields

@staticmethod
def _get_statici18n_js_url():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be added to xblock-utils if possible -- it's copied from xblock-drag-and-drop.

Also need to:

  • add @XBlock.needs('i18n') to the HTML5XBlock
  • use loader.render_django_template, and pass self.runtime.i18n_service into that function, so that this XBlock's own translations are used when translating strings in the templates.

cf openedx/xblock-drag-and-drop-v2#154

@iamjazzar iamjazzar closed this Jul 22, 2019
@Agrendalath Agrendalath deleted the jazzar/i18n branch August 8, 2023 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants