From 04dc96fa09469745fbd671b69d2a2ac9ac4779fe Mon Sep 17 00:00:00 2001 From: Burak Erol <140210017+erolburak@users.noreply.github.com> Date: Mon, 23 Sep 2024 17:35:59 +0200 Subject: [PATCH] Update translation feature (#107) --- BobbysNews/Factory/ViewModelFactory.swift | 8 +- BobbysNews/Presentation/ContentView.swift | 197 ++++++++++-------- .../Presentation/ContentViewModel.swift | 70 +++++-- BobbysNews/Presentation/DetailView.swift | 12 +- BobbysNews/Presentation/DetailViewModel.swift | 13 +- BobbysNews/Resource/Localizable.xcstrings | 60 +++++- .../TopHeadlinesPersistenceController.swift | 4 +- .../BobbysNewsDomain/Entity/Article.swift | 4 +- .../Entity/ArticleTests.swift | 4 +- .../Extension/ArticleExtensionTests.swift | 4 +- .../Factory/ViewModelFactoryTests.swift | 3 +- .../Presentation/DetailViewModelTests.swift | 7 +- 12 files changed, 256 insertions(+), 130 deletions(-) diff --git a/BobbysNews/Factory/ViewModelFactory.swift b/BobbysNews/Factory/ViewModelFactory.swift index 6c43149..6c4baeb 100644 --- a/BobbysNews/Factory/ViewModelFactory.swift +++ b/BobbysNews/Factory/ViewModelFactory.swift @@ -6,6 +6,7 @@ // import BobbysNewsDomain +import SwiftUI final class ViewModelFactory { // MARK: - Private Properties @@ -28,7 +29,10 @@ final class ViewModelFactory { readTopHeadlinesUseCase: useCaseFactory.readTopHeadlinesUseCase) } - func detailViewModel(article: Article) -> DetailViewModel { - DetailViewModel(article: article) + func detailViewModel(article: Article, + articleImage: Image?) -> DetailViewModel + { + DetailViewModel(article: article, + articleImage: articleImage) } } diff --git a/BobbysNews/Presentation/ContentView.swift b/BobbysNews/Presentation/ContentView.swift index 00f7f5a..67058cc 100644 --- a/BobbysNews/Presentation/ContentView.swift +++ b/BobbysNews/Presentation/ContentView.swift @@ -13,7 +13,6 @@ struct ContentView: View { // MARK: - Private Properties @AppStorage("country") private var country = "" - @Namespace private var animation // MARK: - Properties @@ -25,17 +24,7 @@ struct ContentView: View { NavigationStack { ScrollView { ForEach($viewModel.articles) { $article in - NavigationLink { - DetailView(viewModel: ViewModelFactory.shared.detailViewModel(article: article)) - .navigationTransition(.zoom(sourceID: article.id, - in: animation)) - } label: { - ListItem(article: $article, - translationSessionConfiguration: viewModel.translationSessionConfiguration) - } - .matchedTransitionSource(id: article.id, - in: animation) - .accessibilityIdentifier(article.id == viewModel.articles.first?.id ? "NavigationLink" : "") + ListItem(article: $article) } } .navigationTitle("TopHeadlines") @@ -77,13 +66,14 @@ struct ContentView: View { } } } + .menuActionDismissBehavior(.disabled) } Section { - Toggle("Translation", + Toggle("Translate", systemImage: "translate", - isOn: $viewModel.translationBool) - .accessibilityIdentifier("TranslationToggle") + isOn: $viewModel.translate) + .accessibilityIdentifier("TranslateToggle") } Section { @@ -149,8 +139,8 @@ struct ContentView: View { } } else { switch viewModel.stateTopHeadlines { - case .isLoading: - Text("TopHeadlinesLoading") + case .isLoading, .isTranslating: + Text(viewModel.stateTopHeadlines == .isLoading ? "TopHeadlinesLoading" : "TopHeadlinesTranslating") .fontWeight(.black) case .loaded: EmptyView() @@ -172,6 +162,22 @@ struct ContentView: View { .foregroundStyle(.secondary) .accessibilityIdentifier("RefreshButton") } + case .emptyTranslate: + ContentUnavailableView { + Label("EmptyTranslateTopHeadlines", + systemImage: "translate") + } description: { + Text("EmptyTranslateTopHeadlinesMessage") + } actions: { + Button("Disable") { + viewModel.translate = false + } + .textCase(.uppercase) + .font(.system(.subheadline, + weight: .black)) + .foregroundStyle(.secondary) + .accessibilityIdentifier("DisableButton") + } } } } @@ -200,6 +206,12 @@ struct ContentView: View { .sensoryFeedback(trigger: viewModel.sensoryFeedbackBool) { _, _ in viewModel.sensoryFeedback } + .onChange(of: viewModel.translate) { _, newValue in + viewModel.translate(translate: newValue) + } + .translationTask(viewModel.translationSessionConfiguration) { translateSession in + await viewModel.translate(translateSession: translateSession) + } } } @@ -210,106 +222,109 @@ struct ContentView: View { private struct ListItem: View { // MARK: - Private Properties + @Namespace private var animation + @State private var articleImage: Image? @State private var showTranslationPresentation = false @State private var translationPresentationText = "" // MARK: - Properties @Binding var article: Article - let translationSessionConfiguration: TranslationSession.Configuration? // MARK: - Layouts var body: some View { - HStack { - VStack(alignment: .leading) { - Text(article.source?.name ?? String(localized: "EmptyArticleSource")) - .font(.system(.subheadline, - weight: .black)) - .lineLimit(1) + NavigationLink { + DetailView(viewModel: ViewModelFactory.shared.detailViewModel(article: article, + articleImage: articleImage)) + .navigationTransition(.zoom(sourceID: article.id, + in: animation)) + } label: { + HStack { + VStack(alignment: .leading) { + Text(article.source?.name ?? String(localized: "EmptyArticleSource")) + .font(.system(.subheadline, + weight: .black)) + .lineLimit(1) - Text(article.publishedAt?.toRelative ?? String(localized: "EmptyArticlePublishedAt")) - .font(.system(size: 8, - weight: .semibold)) + Text(article.publishedAt?.toRelative ?? String(localized: "EmptyArticlePublishedAt")) + .font(.system(size: 8, + weight: .semibold)) - Spacer() + Spacer() - Text((translationSessionConfiguration == nil ? article.title : article.titleTranslation) ?? String(localized: "EmptyArticleTitle")) - .font(.system(.subheadline, - weight: .semibold)) - .lineLimit(2) - } - .multilineTextAlignment(.leading) + Text((article.titleTranslated ?? article.title) ?? String(localized: "EmptyArticleTitle")) + .font(.system(.subheadline, + weight: .semibold)) + .lineLimit(2) + } + .multilineTextAlignment(.leading) - Spacer() + Spacer() - Group { - if let urlToImage = article.urlToImage { - AsyncImage(url: urlToImage, - transaction: .init(animation: .easeIn(duration: 0.75))) - { asyncImagePhase in - if let image = asyncImagePhase.image { - image - .resizable() - .scaledToFill() - .frame(width: 80, - height: 80, - alignment: .center) - .clipped() - } else { - ProgressView() + Group { + if let urlToImage = article.urlToImage { + AsyncImage(url: urlToImage, + transaction: Transaction(animation: .easeIn(duration: 0.75))) + { asyncImagePhase in + if let image = asyncImagePhase.image { + image + .resizable() + .scaledToFill() + .frame(width: 80, + height: 80, + alignment: .center) + .clipped() + .onAppear { + articleImage = image + } + } else { + ProgressView() + } } + } else { + Image(systemName: "photo.circle.fill") + .resizable() + .aspectRatio(contentMode: .fit) + .frame(height: 24) + .foregroundStyle(.gray) + .symbolEffect(.bounce, + options: .nonRepeating) } - } else { - Image(systemName: "photo.circle.fill") - .resizable() - .aspectRatio(contentMode: .fit) - .frame(height: 24) - .foregroundStyle(.gray) - .symbolEffect(.bounce, - options: .nonRepeating) } + .frame(width: 80, + height: 80) + .background(.bar) + .clipShape(.rect(cornerRadius: 12)) } - .frame(width: 80, - height: 80) - .background(.bar) - .clipShape(.rect(cornerRadius: 12)) - } - .padding(.horizontal) - .padding(.vertical, 20) - .contentShape(.rect) - .contextMenu { - if let url = article.url { - ShareLink("Share", - item: url) - .accessibilityIdentifier("ShareLink") - } - if let title = article.title { - Button("Translate", - systemImage: "translate") - { - translationPresentationText = title - showTranslationPresentation = true - } - .accessibilityIdentifier("TranslateButton") - } - } - .translationPresentation(isPresented: $showTranslationPresentation, - text: translationPresentationText) - .translationTask(translationSessionConfiguration) { session in - Task { - if let content = article.content { - article.contentTranslation = try await session.translate(content).targetText + .padding(.horizontal) + .padding(.vertical, 20) + .contentShape(.rect) + .contextMenu { + if let url = article.url { + ShareLink("Share", + item: url) + .accessibilityIdentifier("ShareLink") } if let title = article.title { - article.titleTranslation = try await session.translate(title).targetText + Button("Translate", + systemImage: "translate") + { + translationPresentationText = title + showTranslationPresentation = true + } + .accessibilityIdentifier("TranslateButton") } } + .translationPresentation(isPresented: $showTranslationPresentation, + text: translationPresentationText) } + .matchedTransitionSource(id: article.id, + in: animation) + .accessibilityIdentifier("NavigationLink") } } #Preview("ListItem") { - ListItem(article: .constant(PreviewMock.article), - translationSessionConfiguration: nil) + ListItem(article: .constant(PreviewMock.article)) } diff --git a/BobbysNews/Presentation/ContentViewModel.swift b/BobbysNews/Presentation/ContentViewModel.swift index ecd5740..e57a96f 100644 --- a/BobbysNews/Presentation/ContentViewModel.swift +++ b/BobbysNews/Presentation/ContentViewModel.swift @@ -22,9 +22,9 @@ final class ContentViewModel { enum StateTopHeadlines { /// General States - case isLoading, loaded + case isLoading, loaded, isTranslating /// Empty States - case emptyFetch, emptyRead + case emptyFetch, emptyRead, emptyTranslate } struct SettingsTip: Tip { @@ -83,17 +83,7 @@ final class ContentViewModel { var showConfirmationDialog = false var stateSources: StateSources = .isLoading var stateTopHeadlines: StateTopHeadlines = .isLoading - var translationBool = false { - willSet { - if newValue, translationSessionConfiguration == nil { - translationSessionConfiguration = TranslationSession.Configuration() - } else { - translationSessionConfiguration = nil - readTopHeadlines() - } - } - } - + var translate = false var translationSessionConfiguration: TranslationSession.Configuration? // MARK: - Lifecycles @@ -161,6 +151,7 @@ final class ContentViewModel { selectedCountry = "" stateSources = .emptyRead stateTopHeadlines = .emptyRead + translate = false sensoryFeedbackTrigger(feedback: .success) } catch { showAlert(error: .reset) @@ -171,6 +162,59 @@ final class ContentViewModel { SettingsTip.show = true } + func translate(translate: Bool) { + if translate, translationSessionConfiguration == nil { + translationSessionConfiguration = TranslationSession.Configuration() + } else if translate { + translationSessionConfiguration?.invalidate() + } else { + readTopHeadlines() + } + } + + @MainActor + func translate(translateSession: TranslationSession) async { + stateTopHeadlines = .isTranslating + var contentRequests: [TranslationSession.Request]? = [] + var titleRequests: [TranslationSession.Request]? = [] + for (index, article) in articles.enumerated() { + if let content = article.content { + contentRequests?.append(TranslationSession.Request(sourceText: content, + clientIdentifier: "\(index)")) + } + if let title = article.title { + titleRequests?.append(TranslationSession.Request(sourceText: title, + clientIdentifier: "\(index)")) + } + } + do { + if let contentRequests, + !contentRequests.isEmpty + { + for try await response in translateSession.translate(batch: contentRequests) { + guard let index = Int(response.clientIdentifier ?? "") else { + continue + } + articles[index].contentTranslated = response.targetText + } + } + if let titleRequests, + !titleRequests.isEmpty + { + for try await response in translateSession.translate(batch: titleRequests) { + guard let index = Int(response.clientIdentifier ?? "") else { + continue + } + articles[index].titleTranslated = response.targetText + } + } + updateStateTopHeadlines(state: contentRequests?.isEmpty == true && titleRequests?.isEmpty == true ? .emptyTranslate : .loaded) + } catch { + updateStateTopHeadlines(error: error, + state: .emptyTranslate) + } + } + private func configureTipKit() { try? Tips.configure([.displayFrequency(.immediate), .datastoreLocation(.groupContainer(identifier: "com.burakerol.BobbysNews"))]) diff --git a/BobbysNews/Presentation/DetailView.swift b/BobbysNews/Presentation/DetailView.swift index d593c84..5f60e40 100644 --- a/BobbysNews/Presentation/DetailView.swift +++ b/BobbysNews/Presentation/DetailView.swift @@ -43,9 +43,14 @@ struct DetailView: View { } Group { - if let urlToImage = viewModel.article.urlToImage { + if let image = viewModel.articleImage { + image + .resizable() + .scaledToFill() + .containerRelativeFrame(.horizontal) + } else if let urlToImage = viewModel.article.urlToImage { AsyncImage(url: urlToImage, - transaction: .init(animation: .easeIn(duration: 0.75))) + transaction: Transaction(animation: .easeIn(duration: 0.75))) { asyncImagePhase in if let image = asyncImagePhase.image { image @@ -182,7 +187,8 @@ struct DetailView: View { #Preview("DetailView") { NavigationStack { - DetailView(viewModel: ViewModelFactory.shared.detailViewModel(article: PreviewMock.article)) + DetailView(viewModel: ViewModelFactory.shared.detailViewModel(article: PreviewMock.article, + articleImage: nil)) } } diff --git a/BobbysNews/Presentation/DetailViewModel.swift b/BobbysNews/Presentation/DetailViewModel.swift index dbe7d0e..57e481e 100644 --- a/BobbysNews/Presentation/DetailViewModel.swift +++ b/BobbysNews/Presentation/DetailViewModel.swift @@ -6,7 +6,7 @@ // import BobbysNewsDomain -import Foundation +import SwiftUI @Observable final class DetailViewModel { @@ -14,11 +14,13 @@ final class DetailViewModel { var article: Article var articleContent: String { - article.contentTranslation ?? article.content ?? String(localized: "EmptyArticleContent") + article.contentTranslated ?? article.content ?? String(localized: "EmptyArticleContent") } + var articleImage: Image? + var articleTitle: String { - article.titleTranslation ?? article.title ?? String(localized: "EmptyArticleTitle") + article.titleTranslated ?? article.title ?? String(localized: "EmptyArticleTitle") } var navigationTitleOpacity: Double { @@ -51,7 +53,10 @@ final class DetailViewModel { // MARK: - Lifecycles - init(article: Article) { + init(article: Article, + articleImage: Image?) + { self.article = article + self.articleImage = articleImage } } diff --git a/BobbysNews/Resource/Localizable.xcstrings b/BobbysNews/Resource/Localizable.xcstrings index f6f2217..89388ec 100644 --- a/BobbysNews/Resource/Localizable.xcstrings +++ b/BobbysNews/Resource/Localizable.xcstrings @@ -98,6 +98,22 @@ } } }, + "Disable" : { + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Deaktivieren" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Disable" + } + } + } + }, "EmptyArticleAuthor" : { "localizations" : { "de" : { @@ -309,6 +325,38 @@ } } }, + "EmptyTranslateTopHeadlines" : { + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Keine übersetzten Schlagzeilen" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "No translated top headlines" + } + } + } + }, + "EmptyTranslateTopHeadlinesMessage" : { + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Geladene übersetzte Schlagzeilen werden hier angezeigt." + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Available translated top headlines are displayed here." + } + } + } + }, "ErrorDescription" : { "extractionState" : "manual", "localizations" : { @@ -768,34 +816,34 @@ } } }, - "Translate" : { + "TopHeadlinesTranslating" : { "localizations" : { "de" : { "stringUnit" : { "state" : "translated", - "value" : "Übersetzen" + "value" : "Schlagzeilen werden übersetzt …" } }, "en" : { "stringUnit" : { "state" : "translated", - "value" : "Translate" + "value" : "Top headlines translating …" } } } }, - "Translation" : { + "Translate" : { "localizations" : { "de" : { "stringUnit" : { "state" : "translated", - "value" : "Übersetzung" + "value" : "Übersetzen" } }, "en" : { "stringUnit" : { "state" : "translated", - "value" : "Translation" + "value" : "Translate" } } } diff --git a/BobbysNewsData/Sources/BobbysNewsData/DB/PersistenceController/TopHeadlinesPersistenceController.swift b/BobbysNewsData/Sources/BobbysNewsData/DB/PersistenceController/TopHeadlinesPersistenceController.swift index 82cec16..97f8e2d 100644 --- a/BobbysNewsData/Sources/BobbysNewsData/DB/PersistenceController/TopHeadlinesPersistenceController.swift +++ b/BobbysNewsData/Sources/BobbysNewsData/DB/PersistenceController/TopHeadlinesPersistenceController.swift @@ -41,7 +41,9 @@ final class TopHeadlinesPersistenceController: PTopHeadlinesPersistenceControlle try PersistenceController.shared.backgroundContext.performAndWait { let existingArticles = try PersistenceController.shared.backgroundContext.fetch(ArticleDB.fetchRequest()) topHeadlinesAPI.articles?.forEach { articleAPI in - guard articleAPI.title?.isEmpty == false else { + guard articleAPI.title?.localizedCaseInsensitiveContains("[removed]") == false, + articleAPI.content?.localizedCaseInsensitiveContains("[removed]") == false + else { return } let existingArticle = existingArticles.first { $0.title == articleAPI.title } diff --git a/BobbysNewsDomain/Sources/BobbysNewsDomain/Entity/Article.swift b/BobbysNewsDomain/Sources/BobbysNewsDomain/Entity/Article.swift index 3ed6dc4..3a901bd 100644 --- a/BobbysNewsDomain/Sources/BobbysNewsDomain/Entity/Article.swift +++ b/BobbysNewsDomain/Sources/BobbysNewsDomain/Entity/Article.swift @@ -20,8 +20,8 @@ public struct Article: Hashable, Identifiable, Sendable { public let title: String? public let url: URL? public let urlToImage: URL? - public var contentTranslation: String? - public var titleTranslation: String? + public var contentTranslated: String? + public var titleTranslated: String? // MARK: - Lifecycles diff --git a/BobbysNewsDomain/Tests/BobbysNewsDomainTests/Entity/ArticleTests.swift b/BobbysNewsDomain/Tests/BobbysNewsDomainTests/Entity/ArticleTests.swift index 640f828..3388f19 100644 --- a/BobbysNewsDomain/Tests/BobbysNewsDomainTests/Entity/ArticleTests.swift +++ b/BobbysNewsDomain/Tests/BobbysNewsDomainTests/Entity/ArticleTests.swift @@ -21,7 +21,7 @@ struct ArticleTests { // Then #expect(article?.author == "Test" && article?.content == "Test" && - article?.contentTranslation == nil && + article?.contentTranslated == nil && article?.publishedAt == .distantPast && article?.source?.category == "Test" && article?.source?.country == "en-gb" && @@ -32,7 +32,7 @@ struct ArticleTests { article?.source?.url == URL(string: "Test") && article?.story == "Test" && article?.title == "Test" && - article?.titleTranslation == nil && + article?.titleTranslated == nil && article?.url == URL(string: "Test") && article?.urlToImage == URL(string: "Test"), "Article initializing failed!") diff --git a/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/Extension/ArticleExtensionTests.swift b/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/Extension/ArticleExtensionTests.swift index 9189395..98ddd69 100644 --- a/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/Extension/ArticleExtensionTests.swift +++ b/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/Extension/ArticleExtensionTests.swift @@ -22,7 +22,7 @@ struct ArticleExtensionTests { // Then #expect(article.author == "Test" && article.content == "Test" && - article.contentTranslation == nil && + article.contentTranslated == nil && article.publishedAt == .distantPast && article.source?.category == "Test" && article.source?.country == "en-gb" && @@ -33,7 +33,7 @@ struct ArticleExtensionTests { article.source?.url == URL(string: "Test") && article.story == "Test" && article.title == "Test" && - article.titleTranslation == nil && + article.titleTranslated == nil && article.url == URL(string: "Test") && article.urlToImage == URL(string: "Test"), "ArticleExtension Article initializing failed!") diff --git a/BobbysNewsTests/Factory/ViewModelFactoryTests.swift b/BobbysNewsTests/Factory/ViewModelFactoryTests.swift index a4a25a8..7d792a9 100644 --- a/BobbysNewsTests/Factory/ViewModelFactoryTests.swift +++ b/BobbysNewsTests/Factory/ViewModelFactoryTests.swift @@ -30,7 +30,8 @@ struct ViewModelFactoryTests { // Given let detailViewModel: DetailViewModel? // When - detailViewModel = sut.detailViewModel(article: EntityMock.article) + detailViewModel = sut.detailViewModel(article: EntityMock.article, + articleImage: nil) // Then #expect(detailViewModel != nil, "DetailViewModel initializing failed!") diff --git a/BobbysNewsTests/Presentation/DetailViewModelTests.swift b/BobbysNewsTests/Presentation/DetailViewModelTests.swift index e427b63..c0a8eae 100644 --- a/BobbysNewsTests/Presentation/DetailViewModelTests.swift +++ b/BobbysNewsTests/Presentation/DetailViewModelTests.swift @@ -16,12 +16,13 @@ struct DetailViewModelTests { let article = EntityMock.article let detailViewModel: DetailViewModel? // When - detailViewModel = DetailViewModel(article: article) + detailViewModel = DetailViewModel(article: article, + articleImage: nil) // Then #expect(detailViewModel != nil && detailViewModel?.article.author == article.author && detailViewModel?.article.content == article.content && - detailViewModel?.article.contentTranslation == nil && + detailViewModel?.article.contentTranslated == nil && detailViewModel?.article.publishedAt == article.publishedAt && detailViewModel?.article.source?.category == article.source?.category && detailViewModel?.article.source?.country == article.source?.country && @@ -32,7 +33,7 @@ struct DetailViewModelTests { detailViewModel?.article.source?.url == article.source?.url && detailViewModel?.article.story == article.story && detailViewModel?.article.title == article.title && - detailViewModel?.article.titleTranslation == nil && + detailViewModel?.article.titleTranslated == nil && detailViewModel?.article.url == article.url && detailViewModel?.article.urlToImage == article.urlToImage, "DetailViewModel initializing failed!")