Remove JS gettext() in templates #2057
Conversation
|
If I recall correctly, the reason for this change is so that we can more easily extract translation strings from our templates. Is that correct? The better solution to this problem would be to move all Javascript into separate .js files, so that Babel or GNU gettext or whatever knows how to extract the contents of I'm not saying that this PR is a bad idea -- this is a good first step for getting our i18n system working properly. This comment exists mostly so that (a) someone can contradict me if my assumption is wrong, and (b) this comment works as documentation if my assumption is correct. |
|
Test failure, looking into it. |
|
@singingwolfboy you are correct: as the files were written, the strings were not being extracted. The larger improvement would be to move this code into separate js files. This is a smaller change that avoids having to decide to move the js code. |
lms/templates/help_modal.html
Outdated
There was a problem hiding this comment.
The asterisks here shouldn't be part of the translation strings. (I know you didn't put them there..) Also, the <span class='tip'> should be in the showFeedback function, no? How do you feel about cleaning that up?
|
Hi I addressed review comments, fixed the broken tests, and verified all the changes manually on local dev. Any other comments before I squash & merge? |
lms/templates/help_modal.html
Outdated
There was a problem hiding this comment.
This is left-over? The new argument isn't used.
There was a problem hiding this comment.
oop yep i thought I deleted that.
|
We're still talking about how to properly quote text like this being turned into Javascript strings, but this PR is ready to merge. 👍 |
Remove JS gettext() in templates
* Fix Bug. Mod activate account process. openedx#2047 * Fix review.
Move to server-side substitutions. @nedbat