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 Apple translation popovers for notes for iOS 17.4+ and macOS 14.4+ #2303

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tyiu
Copy link
Collaborator

@tyiu tyiu commented Jun 21, 2024

Changelog-Added: Add Apple translation popovers for notes for iOS 17.4+ and macOS 14.4+

RPReplay_Final1718937575.MP4

Simulator Screenshot - iPhone 15 Pro - 2024-07-16 at 13 07 06 Medium

These changes are gated around iOS and macOS versions. If an older OS is being run, Apple translations won't be offered.

@alltheseas
Copy link
Collaborator

@tyiu is it possible to set up auto translate using iOS offline translations? That is, remove the need to tap "translate note"

@tyiu
Copy link
Collaborator Author

tyiu commented Jun 21, 2024

@tyiu is it possible to set up auto translate using iOS offline translations? That is, remove the need to tap "translate note"

Technically yes, but only in iOS 18.0+

@jb55
Copy link
Collaborator

jb55 commented Jun 24, 2024

very cool, will test this soon

@tyiu
Copy link
Collaborator Author

tyiu commented Jul 16, 2024

There's a bug when damus_state.settings.auto_translate is set to true but Offline Translations is selected. It won't show the Translate button in that case, which is incorrect. I'll work on a fix, and also look into the auto translations with iOS 18.0.

@alltheseas alltheseas added bug Something is not working, or not working as intended translations Automated DeepL translations, etc labels Jul 16, 2024
Changelog-Added: Add Apple translation popovers for notes for iOS 17.4+ and macOS 14.4+
@tyiu tyiu changed the title Add offline note translations for iOS 17.4+ and macOS 14.4+ Add Apple translation popovers for notes for iOS 17.4+ and macOS 14.4+ Jul 16, 2024
@tyiu
Copy link
Collaborator Author

tyiu commented Jul 16, 2024

I mischaracterized the this PR. It's not exclusively offline note translations from Apple. The translation requests can still go to Apple servers, but can be offline as well via downloaded languages if there's network connectivity issues. I've updated the description, commit, and PR title accordingly

Note that the branch name says offline-translations, which is incorrect. But I didn't want to create a new PR just for the sake of a new branch name.

There's a bug when damus_state.settings.auto_translate is set to true but Offline Translations is selected. It won't show the Translate button in that case, which is incorrect. I'll work on a fix, and also look into the auto translations with iOS 18.0

I fixed the bug that I reported in my previous comment.

@tyiu tyiu removed the bug Something is not working, or not working as intended label Jul 16, 2024
@@ -38,7 +38,13 @@ enum TranslationService: String, CaseIterable, Identifiable, StringCodable {
var model: Model {
switch self {
case .none:
return .init(tag: self.rawValue, displayName: NSLocalizedString("none_translation_service", value: "None", comment: "Dropdown option for selecting no translation service."))
let displayName: String
if TranslationService.isAppleTranslationPopoverSupported {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd like to solicit feedback here. Right now, if you have iOS 17.4+, there's no way to turn off translations. It's just using Apple by default, but requires an explicit tap on the Translate Note button.

Alternatively, I could make None and Apple separate options, but it feels like we have an opportunity to be more opinionated here to just say everyone needs translations.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to consider the privacy aspect, given that some of those translate requests might be sent to Apple servers.

In my opinion, we should be opinionated and place the button "Translate note" by default, and then add a confirmation dialog explaining the user the text might be sent to Apple servers, with an option to dismiss that dialog forever if the user chooses to do so.

For the iOS 18 feature where translations can be made automatically, I believe that should be opt-in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iOS 17 translations Automated DeepL translations, etc
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

4 participants