Implement Apple Translate #2065
Conversation
The user can now choose between his instance's server, DeepL (with API key) and Apple's Translation framework. A translation is cleared if the translation type is changed. The strings aren't yet written, but the translations settings view's inconsistent background is now fixed.
The "always_use_deepl"-setting is now deleted, but its content is transferred to the equivalent value in "preferred_translation_type".
The user is now shown a prompt if they've switched away from .useDeepl, but there's still an API key stored. The API key is not deleted if the user doesn't instruct the app to do so, so this change makes it more transparent, since a user might not expect the key to be stored and might not want this to be the case.
The labels for the buttons and options are now localized. "DeepL API Key" is written consistently (with uppercase Key)
The strings "DeepL" and "Apple Translate" are now also saved in localizable.strings and addressed through keys. They were taken directly previously, which was inconsistent.
The selected value for preferredTranslationType wasn't stored, the synchronization between UserPreferences and Storage is now in place.
The Apple Translate option is hidden if the user hasn't updated their phone to at least iOS 17.4. If the Apple Translate option is selected but the user has downgraded to before iOS 17.4, the standard instance option is selected.
Apple Translate was previously only shown if the standard translate button was visible, that is now fixed. It's now attached to the StatusRowView, which is always present.
The reset of a translation when the translation type is changed is now animated, which is important for iPad users if they've translated a post in the sidebar.
The Mac Catalyst Version doesn't allow the import of the api, so compiler flags now check if the import isn't allowed and then remove all references to Apple Translate.
| @@ -26160,6 +26160,45 @@ | |||
| } | |||
| } | |||
| }, | |||
| "enum.translation-type.apple" : { | |||
There was a problem hiding this comment.
FYI, when adding a new translation, I'm OK with using English localization as the key. It makes the process much simpler as we don't have to add the default english localization to every languages.
There was a problem hiding this comment.
I can revert the one commit that changes that for Apple Translate and DeepL
|
I wonder if it's sime to remove the pro key I provide in DeepLClient. It's a nice fallback when instance translate is not available, but it's expensive. |
This reverts commit 86c5099. # Conflicts: # Packages/Env/Sources/Env/TranslationType.swift
|
And I can also remove the default DeepL client. |
Still thinking about it, I need to review the case where the app use this instead of the instance service. I think it's when the instance service is missing and when the user doesn't have a free key. But there is a lot of usage. |
The DeepL fallback for the instance translation service is removed, error messages are shown if a translation fails.
| @@ -219,6 +233,23 @@ public struct StatusRowView: View { | |||
| StatusDataControllerProvider.shared.dataController(for: viewModel.finalStatus, | |||
| client: viewModel.client) | |||
| ) | |||
| .alert("alert.translation-error.deepl", isPresented: $viewModel.deeplTranslationError) { | |||
There was a problem hiding this comment.
What I was saying about localization is that for the new stuff you want to put in, you can put the English inline directly, it'll become the key and also the default localization for every languages not providing a translation.
| public var description: LocalizedStringKey { | ||
| switch self { | ||
| case .useServerIfPossible: | ||
| "enum.translation-type.use-server-if-possible" |
There was a problem hiding this comment.
Same for this, if we don't have it in localization dic, use the English string directly.
| @@ -12,20 +12,8 @@ public struct DeepLClient: Sendable { | |||
| "https://api\(deeplUserAPIFree && (deeplUserAPIKey != nil) ? "-free" : "").deepl.com/v2/translate" | |||
| } | |||
|
|
|||
| private var APIKey: String { | |||
There was a problem hiding this comment.
I think it's fine to remove the app since it now defaults to the Apple translation service on iOS 17.4. We will see how the users react.
|
I'm gonna change all the string keys to clear text later |
|
Once it's done, all good I'll merge. Thanks! |
|
Just a thought: In StatusRowViewModel, if the instance or DeepL translation fails (because we now removed the app's own PRO key), what if we fallback to Apple translate (without even showing an error) for users on iOS 17.4, where the feature is available? |
The DeepL fallback is reinstated if the user has put in their own API Key
|
Sounds good! I'd still keep DeepL as the first fallback for the instance if the user has a key, though |
Apple Translate is now a fallback for both other translation types, the instance service is a fallback for DeepL.
|
I think I've transformed all the keys I've added to clear text strings, and I've also removed your API key, and made DeepL and the Instance fallbacks for each other and Apple Translate for both. |
|
Thanks! |
|
🚀 |
Instead of just their instance and DeepL, the user has now the option between their instance, DeepL and Apple Translate. The user's previous choice (DeepL on / off) is reflected in the first value of the new setting and the old always_use_deepl App-Storage Key is deleted.
The option is only available on iOS 17.4 or later and not on Mac or Vision Pro. If the user is on an unsupported platform but the option to use Apple Translate is set, it's reset to use the instance.
This also includes a couple of fixes that aren't necessary for Apple Translate, but related:
Tested on iPhone, iPad, Mac & visionOS Simulator