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 HoC markdown files to the translation pipeline #37633

Merged
merged 15 commits into from
Nov 11, 2020

Conversation

hacodeorg
Copy link
Contributor

@hacodeorg hacodeorg commented Nov 3, 2020

FND-1329: Make code.org/hourofcode/* pages translatable.

Before adding HoC markdown files to the translation pipeline, we have to remove all Ruby code and iframe from them. This means:

  1. For md.erb file, remove its Ruby code and change the file extension to just md.
  2. For all md files that use iframe to display Youtube videos, convert them to md.partial and use the display_video_thumbnail partial instead. (Additionally, always center the videos and remove the black bars above and below them.)

Then we can finally add a list of safe markdown files to sync-in.rb.

Screenshots

Before After
Screen Shot 2020-11-05 at 11 00 51 AM Screen Shot 2020-11-05 at 11 01 02 AM
Screen Shot 2020-11-05 at 11 03 20 AM Screen Shot 2020-11-05 at 11 03 25 AM

Links

  • Slack thread on making a markdown file safe vs. translator_friendly.

Testing story

Steps to verify that pegasus/sites.v3/code.org/public/hourofcode/starwars.md is translatable:

  1. Run the localize_markdown_content function in bin/i18n/sync-in.rb. Verify that the starwars.md.partial file is copied to i18n/locales/source/markdown/public/hourofcode/starwars.md.
  2. Simulate the result of sync-down.rb step by copying a translated version of starwars.md to i18n/locales/<locale>/codeorg-markdown/hourofcode/starwars.md. An example locale is vi-VN.
  3. Run bin/i18n/sync-out.rb, specifically the distribute_translations method, and verify that the translated file is copied to pegasus/sites.v3/code.org/i18n/public/hourofcode/starwars.vi-VN.md.partial.
  4. Open http://localhost.code.org:3000/lang/vi/hourofcode/starwars. The page should be translated now and the Starwars intro video still shows up correctly.

@hacodeorg hacodeorg requested a review from a team November 3, 2020 10:07
@hacodeorg hacodeorg marked this pull request as ready for review November 3, 2020 10:07
Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

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

In addition to adding these files to the list of files to be synced, we also need to triage them to make sure that none of the files contain content we don't want to expose to translators; this usually means things like embedded ERB or HTML.

For example, this embedded iframe in starwars.md:

https://github.com/code-dot-org/code-dot-org/blame/staging/pegasus/sites.v3/code.org/public/hourofcode/starwars.md#L10

We'll want to go through each of these files and pull this content out into views, then include them using the markdown partials syntax (see https://code.org/editing/concepts)

Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

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

This is looking real good!

Have you checked with someone on marketing to confirm that the minor changes included here are acceptable?

---

# Make a Flappy Game: An Hour of Code Tutorial

<center><iframe style="max-width:100%" width="600" height="337" src="https://www.youtube.com/embed/VQ4lo6Huylc" frameborder="0" allowfullscreen></iframe></center>
<div style="width:600px; max-width:100%; margin-left:auto; margin-right:auto;">
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason not to put this HTML in the view as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought a video partial view can be used in other contexts, so I separate it from its container div.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can see that, but I think in this context it makes sense to keep them together.

The specific motivation here is to keep complicated stuff like html out of the hands of translators, to minimize confusion (they tend to translate things like HTML tag names). In the case where we do want to reuse this in another context, it wouldn't be too much work to separate the template out at that point rather than doing it now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do that to make the markdown files cleaner.

For my own education, does CrowdIn automatically display (and hide) simple HTML elements from translator? For example, this file is full of HTML tags https://github.com/code-dot-org/code-dot-org/blob/staging/pegasus/sites.v3/code.org/public/athome.md.partial. But in CrowdIn, the file is displayed quite nicely without exposing those tags to translator https://crowdin.com/translate/codeorg-markdown/177852/en-vi?filter=basic&value=0.

Copy link
Contributor

Choose a reason for hiding this comment

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

Crowdin sometimes automatically hides HTML elements in that view, but doesn't always. If you look through the individual strings in that file, you will find several that turn the embedded tags into <0> and </0>, but it doesn't do so consistently even on the same string.

Note also that this display smoothing is a property of the Crowdin editing UI, and so in addition to being inconsistent it doesn't apply to our translators who work outside of Crowdin, like Translate By Humans.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it! Thanks Elijah. I've pushed a new commit moving the HTML tags to ERB files; awaiting for the test run. Anything else you think I should change?

@Hamms Hamms mentioned this pull request Nov 10, 2020
8 tasks
Copy link
Contributor

@Hamms Hamms left a comment

Choose a reason for hiding this comment

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

Nothing else; this looks great! Thanks for accommodating all my feedback!

@hacodeorg hacodeorg merged commit e9606ba into staging Nov 11, 2020
@hacodeorg hacodeorg deleted the ha/translate-hoc-pages branch November 11, 2020 01:18
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.

None yet

3 participants