-
Notifications
You must be signed in to change notification settings - Fork 52
FEATURE: Allow untranslated posts in inline-translation mode to be manually translated #230
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,29 +31,35 @@ export default class TranslatedPost extends Component { | |
| return this.post.translatedTitle; | ||
| } | ||
|
|
||
| get showTranslation() { | ||
| return !this.siteSettings.experimental_topic_translation; | ||
| } | ||
|
|
||
| <template> | ||
| <div class="post-translation"> | ||
| <ConditionalLoadingSpinner | ||
| class="post-translation" | ||
| @condition={{this.loading}} | ||
| @size="small" | ||
| > | ||
| <hr /> | ||
| {{#if this.translatedTitle}} | ||
| <div class="topic-attribution"> | ||
| {{this.translatedTitle}} | ||
| {{#if this.showTranslation}} | ||
| <hr /> | ||
| {{#if this.translatedTitle}} | ||
| <div class="topic-attribution"> | ||
| {{this.translatedTitle}} | ||
| </div> | ||
| {{/if}} | ||
| <div class="post-attribution"> | ||
| {{i18n | ||
| "translator.translated_from" | ||
| language=this.post.detectedLang | ||
| translator=this.siteSettings.translator | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Amazing, thank you! |
||
| }} | ||
| </div> | ||
| <div class="cooked"> | ||
| {{htmlSafe this.post.translatedText}} | ||
| </div> | ||
| {{/if}} | ||
| <div class="post-attribution"> | ||
| {{i18n | ||
| "translator.translated_from" | ||
| language=this.post.detectedLang | ||
| translator=this.siteSettings.translator | ||
| }} | ||
| </div> | ||
| <div class="cooked"> | ||
| {{htmlSafe this.post.translatedText}} | ||
| </div> | ||
| </ConditionalLoadingSpinner> | ||
| </div> | ||
| </template> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import { render } from "@ember/test-helpers"; | ||
| import { hbs } from "ember-cli-htmlbars"; | ||
| import { module, test } from "qunit"; | ||
| import { setupRenderingTest } from "discourse/tests/helpers/component-test"; | ||
|
|
||
| module("Integration | Component | toggle-translation-button", function (hooks) { | ||
| setupRenderingTest(hooks); | ||
|
|
||
| test("doesn't render when post cannot be translated", async function (assert) { | ||
| this.set("post", { can_translate: false }); | ||
|
|
||
| await render(hbs` | ||
| <PostMenu::ToggleTranslationButton @post={{this.post}} /> | ||
| `); | ||
|
|
||
| assert.dom("button").doesNotExist(); | ||
| }); | ||
|
|
||
| test("renders translation button with correct states", async function (assert) { | ||
| const post = { | ||
| can_translate: true, | ||
| isTranslated: false, | ||
| isTranslating: false, | ||
| }; | ||
|
|
||
| this.set("post", post); | ||
|
|
||
| await render(hbs` | ||
| <PostMenu::ToggleTranslationButton @post={{this.post}} @showLabel={{true}} /> | ||
| `); | ||
|
|
||
| assert.dom("button").exists(); | ||
| assert.dom("button").hasText("View translation"); | ||
| assert.dom("button").doesNotHaveClass("translated"); | ||
|
|
||
| post.isTranslating = true; | ||
| await render(hbs` | ||
| <PostMenu::ToggleTranslationButton @post={{this.post}} @showLabel={{true}} /> | ||
| `); | ||
| assert.dom("button").hasAttribute("disabled"); | ||
| assert.dom("button").hasText("Translating"); | ||
|
|
||
| post.isTranslating = false; | ||
| post.isTranslated = true; | ||
| await render(hbs` | ||
| <PostMenu::ToggleTranslationButton @post={{this.post}} @showLabel={{true}} /> | ||
| `); | ||
| assert.dom("button").hasClass("translated"); | ||
| assert.dom("button").hasText("Hide translation"); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| import { render } from "@ember/test-helpers"; | ||
| import { hbs } from "ember-cli-htmlbars"; | ||
| import { module, test } from "qunit"; | ||
| import { setupRenderingTest } from "discourse/tests/helpers/component-test"; | ||
|
|
||
| module("Integration | Component | translated-post", function (hooks) { | ||
| setupRenderingTest(hooks); | ||
|
|
||
| test("renders translation when post is translated", async function (assert) { | ||
| this.set("outletArgs", { | ||
| post: { | ||
| isTranslated: true, | ||
| isTranslating: false, | ||
| translatedText: "こんにちは", | ||
| translatedTitle: "良い一日", | ||
| detectedLang: "ja", | ||
| }, | ||
| }); | ||
|
|
||
| this.siteSettings.experimental_topic_translation = false; | ||
| this.siteSettings.translator = "Google"; | ||
|
|
||
| await render(hbs` | ||
| <TranslatedPost @outletArgs={{this.outletArgs}} /> | ||
| `); | ||
|
|
||
| assert.dom(".post-translation").exists(); | ||
| assert.dom(".topic-attribution").hasText("良い一日"); | ||
| assert.dom(".post-attribution").hasText("Translated from ja by Google"); | ||
| assert.dom(".cooked").hasText("こんにちは"); | ||
| }); | ||
|
|
||
| test("hides translation when experimental_topic_translation is enabled", async function (assert) { | ||
| this.set("outletArgs", { | ||
| post: { | ||
| isTranslated: true, | ||
| isTranslating: false, | ||
| translatedText: "Bonjour monde", | ||
| }, | ||
| }); | ||
|
|
||
| this.siteSettings.experimental_topic_translation = true; | ||
|
|
||
| await render(hbs` | ||
| <TranslatedPost @outletArgs={{this.outletArgs}} /> | ||
| `); | ||
|
|
||
| assert.dom(".topic-attribution").doesNotExist(); | ||
| assert.dom(".post-attribution").doesNotExist(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| import { setupTest } from "ember-qunit"; | ||
| import { module, test } from "qunit"; | ||
| import pretender, { response } from "discourse/tests/helpers/create-pretender"; | ||
|
|
||
| module("Unit | Service | translator", function (hooks) { | ||
| setupTest(hooks); | ||
|
|
||
| test("translatePost - standard translation", async function (assert) { | ||
| const service = this.owner.lookup("service:translator"); | ||
|
|
||
| pretender.post("/translator/translate", () => { | ||
| return response({ | ||
| detected_lang: "ja", | ||
| translation: "I am a cat", | ||
| title_translation: "Surprise!", | ||
| }); | ||
| }); | ||
|
|
||
| const post = { | ||
| id: 1, | ||
| post_number: 2, | ||
| }; | ||
|
|
||
| await service.translatePost(post); | ||
|
|
||
| assert.strictEqual(post.detectedLang, "ja"); | ||
| assert.strictEqual(post.translatedText, "I am a cat"); | ||
| assert.strictEqual(post.translatedTitle, "Surprise!"); | ||
| }); | ||
|
|
||
| test("translatePost - with experimental translation for first post", async function (assert) { | ||
| const service = this.owner.lookup("service:translator"); | ||
|
|
||
| service.siteSettings.experimental_topic_translation = true; | ||
|
|
||
| let headerUpdateCalled = false; | ||
| let postStreamRefreshCalled = false; | ||
| let titleSet = null; | ||
|
|
||
| service.appEvents.on( | ||
| "header:update-topic", | ||
| () => (headerUpdateCalled = true) | ||
| ); | ||
| service.appEvents.on( | ||
| "post-stream:refresh", | ||
| () => (postStreamRefreshCalled = true) | ||
| ); | ||
| service.documentTitle.setTitle = (title) => (titleSet = title); | ||
|
|
||
| pretender.post("/translator/translate", () => { | ||
| return response({ | ||
| detected_lang: "ja", | ||
| translation: "I am a cat", | ||
| title_translation: "Surprise!", | ||
| }); | ||
| }); | ||
|
|
||
| const topic = { | ||
| set: function (key, value) { | ||
| this[key] = value; | ||
| }, | ||
| }; | ||
| const post = { | ||
| id: 1, | ||
| post_number: 1, | ||
| topic, | ||
| set: function (key, value) { | ||
| this[key] = value; | ||
| }, | ||
| }; | ||
|
|
||
| await service.translatePost(post); | ||
|
|
||
| assert.true(headerUpdateCalled); | ||
| assert.true(postStreamRefreshCalled); | ||
| assert.strictEqual(titleSet, "Surprise!"); | ||
| assert.strictEqual(post.cooked, "I am a cat"); | ||
| assert.false(post.can_translate); | ||
| assert.strictEqual(topic.fancy_title, "Surprise!"); | ||
| }); | ||
|
|
||
| test("clearPostTranslation", function (assert) { | ||
| const service = this.owner.lookup("service:translator"); | ||
|
|
||
| const post = { | ||
| detectedLang: "ja", | ||
| translatedText: "Hello", | ||
| translatedTitle: "Title", | ||
| }; | ||
|
|
||
| service.clearPostTranslation(post); | ||
|
|
||
| assert.strictEqual(post.detectedLang, null); | ||
| assert.strictEqual(post.translatedText, null); | ||
| assert.strictEqual(post.translatedTitle, null); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
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
showButtonmaybe? It's not used elsewhere in this fileThere was a problem hiding this comment.
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!