[MCKIN-2723] Use i18n runtime service to provide translations for HTML templates #154

pomegranited · 2018-03-12T07:44:28Z

Once the dependent PRs are merged, this change is all that's required to allow this XBlock to use its own translations for text found in HTML templates.

JIRA tickets: MCKIN-7023, OSPR-2289

Discussions: WL-230, YONK-879

Dependencies:

[MCKIN-7023] Allows translated strings in XBlock Django templates openedx-unsupported/xblock-utils#48 - merged
[MCKIN-7023] Ensures that default template tags are loaded for Django 1.11 openedx-unsupported/xblock-utils#49 - merged

Screenshots:

Before
these changes, the only text in DnDv2 HTML templates that would appear translated were the text that happened to be translated in edx-platform.

After this change, DnDv2 template translations like this one appear in Studio and LMS:

Sandbox URL: (841a68c)

Merge deadline: None

Testing instructions:

In Studio, create a course and add a Drag and Drop problem (e.g. sandbox Drag and Drop)
Visit the Studio URL with /update_lang appended, and submit eo as your selected Language Code.
Edit the Drag and Drop problem, and note that both blocktrans and trans template tag strings appear translated.

Author notes and concerns:

Translations in the Javascript templates are currently not working at all, and will be addressed by a future change. This issue can be seen in the LMS (student view), and the Studio view, when adding/removing zones.
The process for extracting translations for this XBlock is still quite manual, and so this change does not attempt to correct the approach, or to provide any missing translations.

Reviewers

@mtyaka
edx-solutions reviewer[s] TBD

bradenmacdonald · 2018-03-14T18:08:02Z

drag_and_drop_v2/drag_and_drop_v2.py

            'js_templates': js_templates,
            'id_suffix': id_suffix,
            'fields': self.fields,
            'self': self,
            'data': urllib.quote(json.dumps(self.data)),
-        }
+        })

        fragment = Fragment()
        fragment.add_content(loader.render_template('/templates/html/drag_and_drop_edit.html', context))


Can we change from render_template to render_django_template while we're at it? The former is deprecated since it's ambiguous in an XBlock context.

Fixed with a79afd7.

mtyaka · 2018-03-16T08:39:30Z

@pomegranited Currently adding a new DnDv2 block to a course is broken, which is why tests on https://github.com/edx/edx-platform/pull/17670 are failing.

This does not seem to be related to this PR, but rather a recent change we made to this xblock. Apparently the line that was removed is still needed and the comment mentioning Dogwood was wrong. Can you please add it back in?

Other than that I tested this and it seems to be working great, but some translations are missing (see screenshots below). It seems that most of the missing translations are from js_templates.html, which for some reasons uses {{ i18n ... }} instead of {{ trans ... }}. Maybe we only need to replace that.

I also noticed that the mo file in the eo translations folder was updated 4 days ago, but the po file has last been updated one year ago. Some missing translations could be because of outdated translation files.

pomegranited · 2018-03-16T10:10:41Z

@mtyaka

Currently adding a new DnDv2 block to a course is broken, which is why tests on edx/edx-platform#17670 are failing.

Oo, thank you for figuring out why those tests were failing! Re-added that line with 9bf377e.

It seems that most of the missing translations are from js_templates.html, which for some reasons uses {{ i18n ... }} instead of {% trans ... %}. Maybe we only need to replace that.

The js_templates.html are JS Handlebars templates, and they were supposed to use this i18n service, which says it was hooked up to the client runtime gettext method. This wasn't working for me at all though, and I don't know if it ever was. Check out our demo Studio to see.

So I've submitted deea454, which converts the js_templates.html file to a proper Django template, so we can use this new i18n service. It's not pretty, but it works 😄 Have updated the PR description to include a screenshot,

I also noticed that the mo file in the eo translations folder was updated 4 days ago, but the po file has last been updated one year ago. Some missing translations could be because of outdated translation files.

I updated that with this PR to add the compiled .mo file for the eo test language, since that file was missing from this repo. I haven't touched the actual translation strings in the .po though, so there are likely to still be some missing.

pomegranited · 2018-03-16T11:08:58Z

FYI: I've re-added the "WIP" to the title so we don't forget to merge and tag openedx-unsupported/xblock-utils#48 first.

bradenmacdonald · 2018-03-18T02:29:16Z

drag_and_drop_v2/public/js/drag_and_drop.js

@@ -716,6 +716,7 @@ function DragAndDropBlock(runtime, element, configuration) {
    var renderView = DragAndDropTemplates(configuration);

    var $element = $(element);
+    element = $element[0];  // Works around this Studio bug: https://github.com/edx/edx-platform/pull/11433


I think that restoring this line is an alternative to #153 . We should pick one approach.

CC @ciuin

Not fussed here -- @mtyaka which method would you prefer?

I slightly prefer the restored approach, but am fine with either.

I still think the root cause is a bug in edx-platform. It sometimes initializes the xblock with a jquery object and sometimes with a plain DOM element.

Ok, since @ciuin's #153 has already been approved and merged, let's go with that.

Rebased on 49cf04b, and reverted 9bf377e.

I still think the root cause is a bug in edx-platform. It sometimes initializes the xblock with a jquery object and sometimes with a plain DOM element.

It is, but unfortunately when we've tried to fix it at the platform level, new things broke.

mtyaka · 2018-03-19T08:17:03Z

Thanks @pomegranited!

So I've submitted deea454, which converts the js_templates.html file to a proper Django template, so we can use this new i18n service. It's not pretty, but it works 😄

Sorry about that - I did not realize the strings in js_templates.html were supposed to be translated by JS and not by django. Since it's JS translations which do not work in xblocks at all as far as I can tell, and we'll probably have to fix that anyway, it might make sense to revert the changes until we fix JS translations properly.

mtyaka · 2018-03-19T08:19:00Z

👍

I tested this: I verified that this patch, together with the dependencies correctly translates the studio edit interface. The student view is not translated, because the student view is not using django templates. The student view interface builds the UI entirely via JS, and JS translations are currently not working.
I read through the code
~~I checked for accessibility issues~~ n/a
~~Includes documentation~~ n/a

pomegranited · 2018-03-20T02:28:58Z

Thanks @mtyaka ! I reverted the js_template.html translation change, but kept the commit in a separate branch for future reference: c9dbe04. Also updated the PR description to explain about the JS translation issue.

to take advantage of template translations from openedx-unsupported/xblock-utils#48 Bumps version to 2.1.6

pomegranited · 2018-03-23T01:19:47Z

Squashed 49cf04b...9bf31fe into 4aeb0f7 to make it cleaner to use this change on the client's fork.

pomegranited · 2018-05-10T22:44:08Z

@sanfordstudent All our dependent PRs have merged, so this PR is ready for review. (Yay!)
CC @mduboseedx

pomegranited · 2018-05-14T23:59:47Z

Thank you @sanfordstudent ! Merging now.

openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Mar 12, 2018

pomegranited force-pushed the MCKIN-7023-template-i18n branch from 84fd843 to 0bec8ef Compare March 12, 2018 09:55

pomegranited changed the title ~~WIP: [MCKIN-2723] Use i18n runtime service to provide translations for HTML templates~~ [MCKIN-2723] Use i18n runtime service to provide translations for HTML templates Mar 12, 2018

openedx deleted a comment from openedx-webhooks Mar 12, 2018

nedbat removed needs triage open-source-contribution PR author is not from Axim or 2U labels Mar 12, 2018

bradenmacdonald reviewed Mar 14, 2018

View reviewed changes

pomegranited force-pushed the MCKIN-7023-template-i18n branch 3 times, most recently from a79afd7 to c9dbe04 Compare March 15, 2018 08:51

pomegranited changed the title ~~[MCKIN-2723] Use i18n runtime service to provide translations for HTML templates~~ WIP: [MCKIN-2723] Use i18n runtime service to provide translations for HTML templates Mar 16, 2018

bradenmacdonald reviewed Mar 18, 2018

View reviewed changes

bradenmacdonald mentioned this pull request Mar 18, 2018

[MCKIN-6965] Newly Added Drag and Drop blocks not loading in Studio #153

Merged

3 tasks

mtyaka approved these changes Mar 19, 2018

View reviewed changes

pomegranited force-pushed the MCKIN-7023-template-i18n branch from deea454 to 9bf31fe Compare March 20, 2018 02:18

pomegranited mentioned this pull request Mar 23, 2018

[MCKIN-7023] Improve XBlock i18n by allowing template translations edx/edx-solutions-edx-platform#1032

Merged

1 task

Use render_django_template, and pass in i18n_service

4aeb0f7

to take advantage of template translations from openedx-unsupported/xblock-utils#48 Bumps version to 2.1.6

pomegranited force-pushed the MCKIN-7023-template-i18n branch from 9bf31fe to 4aeb0f7 Compare March 23, 2018 01:19

pomegranited mentioned this pull request Apr 3, 2018

[MCKIN-2723] DnDv3: Use i18n runtime service to provide translations for HTML templates open-craft/xblock-drag-and-drop-v2#25

Merged

1 task

openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Apr 19, 2018

openedx deleted a comment from openedx-webhooks Apr 20, 2018

mduboseedx removed needs triage open-source-contribution PR author is not from Axim or 2U labels Apr 20, 2018

pomegranited changed the title ~~WIP: [MCKIN-2723] Use i18n runtime service to provide translations for HTML templates~~ [MCKIN-2723] Use i18n runtime service to provide translations for HTML templates May 8, 2018

pomegranited changed the title ~~[MCKIN-2723] Use i18n runtime service to provide translations for HTML templates~~ WIP [MCKIN-2723] Use i18n runtime service to provide translations for HTML templates May 9, 2018

This was referenced May 9, 2018

MCKIN-7023 xblock i18n bump versions edx/edx-solutions-edx-platform#1073

Merged

[MCKIN-7023] Ensures that default template tags are loaded for Django 1.11 openedx-unsupported/xblock-utils#49

Merged

Using xblock-utils@v1.1.1

2f68651

pomegranited force-pushed the MCKIN-7023-template-i18n branch from c8f57e7 to 2f68651 Compare May 10, 2018 22:33

pomegranited changed the title ~~WIP [MCKIN-2723] Use i18n runtime service to provide translations for HTML templates~~ [MCKIN-2723] Use i18n runtime service to provide translations for HTML templates May 10, 2018

sanfordstudent approved these changes May 11, 2018

View reviewed changes

pomegranited merged commit 987b7dc into openedx:master May 15, 2018

pomegranited deleted the MCKIN-7023-template-i18n branch May 15, 2018 00:01

pomegranited mentioned this pull request May 15, 2018

[MCKIN-7389-js-i18n] Adding i18n support in JS files #156

Merged

2 tasks

pomegranited mentioned this pull request Jan 24, 2019

Add i18n support to the XBlock open-craft/xblock-html#6

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MCKIN-2723] Use i18n runtime service to provide translations for HTML templates #154

[MCKIN-2723] Use i18n runtime service to provide translations for HTML templates #154

pomegranited commented Mar 12, 2018 •

edited

Loading

bradenmacdonald Mar 14, 2018

pomegranited Mar 15, 2018 •

edited

Loading

mtyaka commented Mar 16, 2018 •

edited

Loading

pomegranited commented Mar 16, 2018 •

edited

Loading

pomegranited commented Mar 16, 2018

bradenmacdonald Mar 18, 2018

pomegranited Mar 19, 2018

mtyaka Mar 19, 2018

pomegranited Mar 20, 2018 •

edited

Loading

bradenmacdonald Mar 20, 2018

mtyaka commented Mar 19, 2018

mtyaka commented Mar 19, 2018

pomegranited commented Mar 20, 2018

pomegranited commented Mar 23, 2018 •

edited

Loading

pomegranited commented May 10, 2018

pomegranited commented May 14, 2018

[MCKIN-2723] Use i18n runtime service to provide translations for HTML templates #154

[MCKIN-2723] Use i18n runtime service to provide translations for HTML templates #154

Conversation

pomegranited commented Mar 12, 2018 • edited Loading

bradenmacdonald Mar 14, 2018

Choose a reason for hiding this comment

pomegranited Mar 15, 2018 • edited Loading

Choose a reason for hiding this comment

mtyaka commented Mar 16, 2018 • edited Loading

pomegranited commented Mar 16, 2018 • edited Loading

pomegranited commented Mar 16, 2018

bradenmacdonald Mar 18, 2018

Choose a reason for hiding this comment

pomegranited Mar 19, 2018

Choose a reason for hiding this comment

mtyaka Mar 19, 2018

Choose a reason for hiding this comment

pomegranited Mar 20, 2018 • edited Loading

Choose a reason for hiding this comment

bradenmacdonald Mar 20, 2018

Choose a reason for hiding this comment

mtyaka commented Mar 19, 2018

mtyaka commented Mar 19, 2018

pomegranited commented Mar 20, 2018

pomegranited commented Mar 23, 2018 • edited Loading

pomegranited commented May 10, 2018

pomegranited commented May 14, 2018

pomegranited commented Mar 12, 2018 •

edited

Loading

pomegranited Mar 15, 2018 •

edited

Loading

mtyaka commented Mar 16, 2018 •

edited

Loading

pomegranited commented Mar 16, 2018 •

edited

Loading

pomegranited Mar 20, 2018 •

edited

Loading

pomegranited commented Mar 23, 2018 •

edited

Loading