Machine translation of posts #4307

charlag · 2024-03-03T22:08:32Z

No description provided.

charlag · 2024-03-03T22:19:33Z

app/build.gradle

@@ -40,6 +40,13 @@ android {
        buildConfigField("String", "CUSTOM_INSTANCE", "\"$CUSTOM_INSTANCE\"")
        buildConfigField("String", "SUPPORT_ACCOUNT_URL", "\"$SUPPORT_ACCOUNT_URL\"")
    }
+    compileOptions {


this fixed my IDE but idk if we should remove it before the release?

we shouldn't even merge it

I think the blessed way now is

kotlin { jvmToolchain(17) }

https://developer.android.com/build/jdks#toolchain

charlag · 2024-03-03T22:39:33Z

@connyduck bitrise failed for no apparent reason

Tak

Looks reasonable. I've installed it locally, but I don't think any of my instances support translation 😐

app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt

charlag · 2024-03-04T10:43:52Z

app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt

+
+            val translateItem = menu.findItem(R.id.status_translate)
+            translateItem.isVisible =
+                onMoreTranslate != null && status.language != Locale.getDefault().language && instanceInfoRepository.getCachedInstanceInfoOrFallback().translationEnabled == true


I fetch it here before showing the dropdown but I could also preload it when the screen is opened

I'd preload it, otherwise there might be an annoying delay when clicking the more button

doesn't seem to be a problem a practice but I can preload!

connyduck · 2024-03-05T11:54:50Z

app/.idea/workspace.xml

+    </task>
+    <servers />
+  </component>
+</project>


I don't think this file should be checked in

nope, was a mistake, slipped in at some point, sorry!

connyduck · 2024-03-05T12:21:30Z

It took me a while to figure out why the menu item was not shown for me, even though m.s supports translation. Turns out instance info is never downloaded again when it is already cached, except when going to About/Compose/EditProfile. That needs to change somehow.

connyduck

Works very well 😍

connyduck · 2024-03-05T12:02:57Z

app/src/main/res/menu/status_more.xml

@@ -15,6 +15,9 @@
    <item
        android:id="@+id/status_copy_link"
        android:title="@string/action_copy_link" />
+    <item
+        android:id="@+id/status_translate"
+        android:title="Translate" />


should be translateable

connyduck · 2024-03-05T12:08:10Z

app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt

+
+            val translateItem = menu.findItem(R.id.status_translate)
+            translateItem.isVisible =
+                onMoreTranslate != null && status.language != Locale.getDefault().language && instanceInfoRepository.getCachedInstanceInfoOrFallback().translationEnabled == true


I'd preload it, otherwise there might be an annoying delay when clicking the more button

connyduck · 2024-03-05T12:29:49Z

app/src/main/res/layout/item_status.xml

+        android:layout_marginEnd="4dp"
+        android:maxLines="4"
+        android:visibility="gone"
+        tools:visibility="visible" />


I'd add a lineSpacingMultiplier="1.1", align the button top and bottom to the text, and add 4dp top margin.
Like this:

I'm a bit confused, currently the first line is aligned with the button text, would you rather center it relative to the text? To me both looks good, I will try your idea with centering!

padding between them, right?

idk how to both center the button to the text but also not make button overlap above?

padding between them, right?

4dp additional padding between username and text, but of course there should also be a bit of padding between text & button.

idk how to both center the button to the text but also not make button overlap above?

A yes when the text is only one line it can overlap 🤔
Some possibilities:

another barrier

force the text to be at least 2 lines

leave it top aligned

I don't care much.

connyduck · 2024-03-05T12:31:14Z

.../main/java/com/keylesspalace/tusky/components/timeline/viewmodel/NetworkTimelineViewModel.kt

+            .onFailure {
+                status.update()
+            }
+            .asResult()


Why not just forward the NetworkResult?

it's kind of leaking an impl detail to me but if you think it's useless I can just pass it through

I have no strong opinion about either, you can leave it if you want

connyduck · 2024-03-05T12:34:21Z

app/src/main/java/com/keylesspalace/tusky/fragment/NotificationsFragment.java

+            .setMessage(R.string.notification_clear_text)
+            .setPositiveButton(android.R.string.ok, (DialogInterface dia, int which) -> clearNotifications())
+            .setNegativeButton(android.R.string.cancel, null)
+            .show();


Can you maybe remove these indendation changes?

for sure, sorry!

connyduck · 2024-03-05T12:35:43Z

app/src/main/res/values/strings.xml

@@ -211,6 +211,9 @@
    <string name="action_copy_link">Copy the link</string>
    <string name="action_open_as">Open as %s</string>
    <string name="action_share_as">Share as …</string>
+    <string name="action_translate">Translate</string>
+    <string name="action_show_original">Show original</string>
+    <string name="action_show_untransalted">Show untranslated</string>


Typo in the name, but also unused?

charlag · 2024-03-05T21:35:38Z

I've fixed most things (except for the button position). I had to rebase to get @ApplicationScope

charlag · 2024-03-05T21:40:01Z

Thank you for catching those issues with instance cache!
@Tak I think you just run into the same bugs tbh

charlag · 2024-03-06T15:12:12Z

app/src/main/java/com/keylesspalace/tusky/components/search/SearchViewModel.kt

 ) : ViewModel() {

+    init {
+        instanceInfoRepository.precache()


I don't like having side-effects in constructor but I don't want to introduce an init() method either

charlag · 2024-03-06T15:14:10Z

I believe I have addressed everything except for the build.gradle thing, I can take it out, no problem with that

connyduck · 2024-03-06T18:30:36Z

We actually wanted to use Java 21 #4235

charlag · 2024-03-06T19:39:37Z

oh I see
I will remove it

Tak · 2024-03-07T12:39:38Z

app/src/main/java/com/keylesspalace/tusky/fragment/SFragment.kt

+        translateItem.isVisible =
+            onMoreTranslate != null && status.language != Locale.getDefault().language && instanceInfoRepository.cachedInstanceInfoOrFallback.translationEnabled == true
+        translateItem.setTitle(if (translation != null) R.string.action_show_original else R.string.action_translate)
+


I'm getting this one when opening the menu for one of my own posts from the detailed view - not sure if we need to nullcheck or otherwise guard the menu item visibility logic

Process: com.keylesspalace.tusky.test, PID: 4953 java.lang.NullPointerException: Attempt to invoke interface method 'android.view.MenuItem android.view.MenuItem.setVisible(boolean)' on a null object reference at com.keylesspalace.tusky.fragment.SFragment$more$1.invokeSuspend(SFragment.kt:221) at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) at kotlinx.coroutines.internal.DispatchedContinuationKt.resumeCancellableWith(DispatchedContinuation.kt:363) at kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable(Cancellable.kt:26) at kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable$default(Cancellable.kt:21) at kotlinx.coroutines.CoroutineStart.invoke(CoroutineStart.kt:88) at kotlinx.coroutines.AbstractCoroutine.start(AbstractCoroutine.kt:123) at kotlinx.coroutines.BuildersKt__Builders_commonKt.launch(Builders.common.kt:52) at kotlinx.coroutines.BuildersKt.launch(Unknown Source:1) at kotlinx.coroutines.BuildersKt__Builders_commonKt.launch$default(Builders.common.kt:43) at kotlinx.coroutines.BuildersKt.launch$default(Unknown Source:1) at com.keylesspalace.tusky.fragment.SFragment.more(SFragment.kt:157) at com.keylesspalace.tusky.components.viewthread.ViewThreadFragment.onMore(ViewThreadFragment.kt:377) at com.keylesspalace.tusky.adapter.StatusBaseViewHolder.lambda$setupButtons$11(StatusBaseViewHolder.java:717) at com.keylesspalace.tusky.adapter.StatusBaseViewHolder.$r8$lambda$2lpNa1dIlwqMDFIdAYaiyDBB3A4(Unknown Source:0) at com.keylesspalace.tusky.adapter.StatusBaseViewHolder$$ExternalSyntheticLambda9.onClick(D8$$SyntheticClass:0) at android.view.View.performClick(View.java:7448) at android.view.View.performClickInternal(View.java:7425) at android.view.View.access$3600(View.java:810) at android.view.View$PerformClick.run(View.java:28305) at android.os.Handler.handleCallback(Handler.java:938) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loop(Looper.java:223) at android.app.ActivityThread.main(ActivityThread.java:7656) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:592) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:947)

ah good catch!

I wish we just made the menus programmatically

- it is not inflated for user's own posts but we didn't check for that - some servers return language codes in uppercase and it wasn't matching Java's lowercase code when it should have

charlag · 2024-03-09T11:21:46Z

fixed the crash, fixed another issue with language code comparison, rebased again

timnolte · 2024-04-04T11:24:29Z

@connyduck & @charlag was there some fix for the instance caching issue? Perhaps I missed it but this will work with instances using either DeepL or LibreTranslate right? I'm not seeing the option to translate posts like I do on Megalodon.

smiba · 2024-05-03T22:11:46Z

Unfortunately not working for me, I get an error "Required value 'mediaAttachments' ([...]) missing" when trying to translate a post.

Running mastodon 4.1.15

connyduck · 2024-05-04T06:52:55Z

@smiba Can you give me more details please? Which status are you translating into which language from which mastodon server?

smiba · 2024-05-04T21:49:04Z

@smiba Can you give me more details please? Which status are you translating into which language from which mastodon server?

I forgot which status, but they all behave the same and respond with an error. I think it was Japanese into English though.

It's from the mastodon server I manage: https://woof.tech -- We're running an older version (4.1.15) of Mastodon with backported changes. Not sure if this issue is with all 4.1 instances but I'd imagine so? I haven't made any changes to it's translation API or translation functionality in general
Registration is open if you wish to try it yourself

If there is anything else I can do please let me know, couldn't quickly figure out if I could dump Tusky's logs from my phone without adb etc.

connyduck · 2024-05-05T05:38:07Z

@smiba Unfortunately it is not possible to create an account. Clicking the confirmation link shows this screen and the resend does nothing:

Why you don't upgrade to 4.2?

smiba · 2024-05-05T10:07:08Z

@smiba Unfortunately it is not possible to create an account. Clicking the confirmation link shows this screen and the resend does nothing:

Why you don't upgrade to 4.2?

Sorry, should be resolved now!

We're running some patches that I've yet to migrate to 4.2, which means we're currently stuck on 4.1. I hope to finally have time soon to migrate everything to 4.2

connyduck · 2024-05-05T10:36:41Z

#4422

smiba · 2024-05-05T10:43:21Z

Thank you 💜

…on some servers (#4422) see #4307 (comment)

charlag force-pushed the translations branch from abbd30a to 2180e82 Compare March 3, 2024 22:19

charlag commented Mar 3, 2024

View reviewed changes

charlag force-pushed the translations branch 2 times, most recently from 40f8cbb to c2fef72 Compare March 3, 2024 22:36

charlag marked this pull request as ready for review March 3, 2024 22:43

charlag requested review from Tak and connyduck March 3, 2024 22:44

Tak approved these changes Mar 4, 2024

View reviewed changes

app/src/main/java/com/keylesspalace/tusky/components/search/fragments/SearchStatusesFragment.kt Outdated Show resolved Hide resolved

charlag commented Mar 4, 2024

View reviewed changes

connyduck reviewed Mar 5, 2024

View reviewed changes

connyduck requested changes Mar 5, 2024

View reviewed changes

charlag force-pushed the translations branch from 8315f53 to cd69215 Compare March 5, 2024 21:34

charlag force-pushed the translations branch from cd69215 to 4e309ce Compare March 5, 2024 21:39

charlag commented Mar 6, 2024

View reviewed changes

charlag force-pushed the translations branch from 0c5cdcf to ef86899 Compare March 6, 2024 23:18

Tak reviewed Mar 7, 2024

View reviewed changes

charlag added 7 commits March 9, 2024 12:16

Implement machine translation for posts

efba197

Prefer cached instanceInfo for more menu

a844d4e

Fix status description, add a11y actions to status search fragment

c0f217b

Extract locale name util

7769643

Move translation status to the top to make it more prominent

0d5537a

Cleanup after review for translation feature

d5dcae5

Precache instanceInfo for UI

58bce32

charlag added 4 commits March 9, 2024 12:16

Fix ComposeActivityTest

53c2df2

Improve translation status & button layout

108a4b6

Fix typo

1444466

Fix issues with translations menu button

16a5dc6

- it is not inflated for user's own posts but we didn't check for that - some servers return language codes in uppercase and it wasn't matching Java's lowercase code when it should have

charlag force-pushed the translations branch from a9e54b9 to 16a5dc6 Compare March 9, 2024 11:18

connyduck approved these changes Mar 9, 2024

View reviewed changes

connyduck merged commit fbb2279 into develop Mar 9, 2024
3 checks passed

connyduck deleted the translations branch March 9, 2024 15:12

This was referenced Mar 9, 2024

Post translation option in post menu. #2823

Closed

[Feature Request] LibreTranslate integration #2188

Closed

[Feature Request] translation feature #134

Closed

can you add translate the blog #4042

Closed

connyduck mentioned this pull request May 5, 2024

add default value to Translation.mediaAttachments to fix translation on some servers #4422

Merged

connyduck added a commit that referenced this pull request May 10, 2024

add default value to Translation.mediaAttachments to fix translation …

82817a0

…on some servers (#4422) see #4307 (comment)

connyduck added a commit that referenced this pull request May 10, 2024

add default value to Translation.mediaAttachments to fix translation …

dc4fd8f

…on some servers (#4422) see #4307 (comment)

Machine translation of posts #4307

Machine translation of posts #4307

Conversation

charlag commented Mar 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charlag commented Mar 3, 2024

Tak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

connyduck commented Mar 5, 2024

connyduck left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charlag commented Mar 5, 2024

charlag commented Mar 5, 2024

Choose a reason for hiding this comment

charlag commented Mar 6, 2024

connyduck commented Mar 6, 2024

charlag commented Mar 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charlag commented Mar 9, 2024

timnolte commented Apr 4, 2024

smiba commented May 3, 2024 • edited Loading

connyduck commented May 4, 2024

smiba commented May 4, 2024

connyduck commented May 5, 2024

smiba commented May 5, 2024

connyduck commented May 5, 2024

smiba commented May 5, 2024

smiba commented May 3, 2024 •

edited

Loading