Skip to content

Conversation

@nattsw
Copy link
Contributor

@nattsw nattsw commented Feb 27, 2025

Currently the 🌐 only appears in dual-text mode (not inline translation mode) when the experimental_topic_translation feature is disabled. This means we are unable to translate old posts when in experimental_topic_translation.

This PR allows 🌐 to show up when experimental_topic_translation is enabled, and does not show the dual-text translation below (the old method) but replaces the content inline instead.

In addition in this PR, when experimental_topic_translation is enabled, it takes the response from /translate and updates the post cooked and topic title for inline replacement. (in other scenarios, translation updates are done by messagebus)

(also backfills many untested js files)

Videos to indicate how function works with this PR:

Parallel

parallel.mp4

Inline

inline.mp4

(This isn't really a FIX since it was never a real feature...)


@service modal;
@service translator;
@service siteSettings;
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to use this in showButton maybe? It's not used elsewhere in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I had originally done some implementation in this file but removed it. Thanks!

{{i18n
"translator.translated_from"
language=this.post.detectedLang
translator=this.siteSettings.translator
Copy link
Contributor

Choose a reason for hiding this comment

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

Certainly a succinct setting name! :D I thought this was a mistake, I know you didn't add this and it's too late now anyway, but translator_provider or something would be better, it doesn't look like an actual setting ATM.

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 will likely rename the setting together with the "inline" thing we talked about.

Copy link
Contributor

Choose a reason for hiding this comment

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

Amazing, thank you!

@nattsw nattsw merged commit 6e8b183 into main Feb 27, 2025
6 checks passed
@nattsw nattsw deleted the allow-translating-inline branch February 27, 2025 05:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants