From c17acc3c13a3e16b7aa051c2cd3e837de7bb7648 Mon Sep 17 00:00:00 2001 From: Burak Erol <140210017+erolburak@users.noreply.github.com> Date: Tue, 26 Mar 2024 16:36:34 +0100 Subject: [PATCH] improve ui and refactor cancellable to cancellables (#75) --- BobbysNews/Presentation/ContentView.swift | 68 +++++++------------ .../Presentation/ContentViewModel.swift | 8 +-- BobbysNews/Presentation/DetailView.swift | 24 +++---- BobbysNews/Resource/Localizable.xcstrings | 16 +++++ .../SourcesPersistenceControllerTests.swift | 8 +-- .../TopHeadlinesDataControllerTests.swift | 8 +-- .../Repository/SourcesRepositoryTests.swift | 8 +-- .../TopHeadlinesRepositoryTests.swift | 8 +-- .../Sources/ReadSourcesUseCaseTests.swift | 8 +-- .../ReadTopHeadlinesUseCaseTests.swift | 8 +-- 10 files changed, 77 insertions(+), 87 deletions(-) diff --git a/BobbysNews/Presentation/ContentView.swift b/BobbysNews/Presentation/ContentView.swift index fb356e2..ba4b448 100644 --- a/BobbysNews/Presentation/ContentView.swift +++ b/BobbysNews/Presentation/ContentView.swift @@ -68,13 +68,11 @@ struct ContentView: View { } case .emptyFetch, .emptyRead: Section(viewModel.stateSources == .emptyFetch ? "EmptyFetchSources" : "EmptyReadSources") { - Button { + Button("CountriesLoad", + systemImage: "arrow.down.to.line.circle.fill") { Task { await viewModel.fetchSources(sensoryFeedback: true) } - } label: { - Label("CountriesLoad", - systemImage: "arrow.down.to.line.circle.fill") } } } @@ -96,11 +94,10 @@ struct ContentView: View { } Section { - Button(role: .destructive) { + Button("Reset", + systemImage: "trash.circle.fill", + role: .destructive) { viewModel.showConfirmationDialog = true - } label: { - Label("Reset", - systemImage: "trash.circle.fill") } .accessibilityIdentifier("ResetButton") } @@ -122,46 +119,29 @@ struct ContentView: View { } .overlay(alignment: .center) { if viewModel.selectedCountry.isEmpty { - ContentUnavailableView { - Label("EmptySelectedCountry", - systemImage: "flag.circle.fill") - } description: { - Text("EmptySelectedCountryMessage") - } + ContentUnavailableView("EmptySelectedCountry", + systemImage: "flag.circle.fill", + description: Text("EmptySelectedCountryMessage")) } else { - VStack { - switch viewModel.stateTopHeadlines { - case .isLoading: - Text("TopHeadlinesLoading") - .fontWeight(.black) - case .loaded: - EmptyView() - case .emptyFetch: - ContentUnavailableView { - Label("EmptyFetchTopHeadlines", - systemImage: "newspaper.circle.fill") - } description: { - Text("EmptyFetchTopHeadlinesMessage") - } - case .emptyRead: - ContentUnavailableView { - Label("EmptyReadTopHeadlines", - systemImage: "newspaper.circle.fill") - } description: { - Text("EmptyReadTopHeadlinesMessage") - } - } - - if viewModel.stateTopHeadlines == .emptyFetch || - viewModel.stateTopHeadlines == .emptyRead { - Button { + switch viewModel.stateTopHeadlines { + case .isLoading: + Text("TopHeadlinesLoading") + .fontWeight(.black) + case .loaded: + EmptyView() + case .emptyFetch, .emptyRead: + ContentUnavailableView { + Label(viewModel.stateTopHeadlines == .emptyFetch ? "EmptyFetchTopHeadlines" : "EmptyReadTopHeadlines", + systemImage: "newspaper.circle.fill") + } description: { + Text(viewModel.stateTopHeadlines == .emptyFetch ? "EmptyFetchTopHeadlinesMessage" : "EmptyReadTopHeadlinesMessage") + } actions: { + Button("Refresh") { Task { await viewModel.fetchTopHeadlines(state: .isLoading) } - } label: { - Text("Refresh") - .textCase(.uppercase) } + .textCase(.uppercase) .font(.system(.subheadline, weight: .black)) .foregroundStyle(.secondary) @@ -239,7 +219,7 @@ struct ContentView: View { .frame(width: 80, height: 80) .background(.bar) - .clipShape(RoundedRectangle(cornerRadius: 12)) + .clipShape(.rect(cornerRadius: 12)) } .padding(.horizontal) .padding(.vertical, 20) diff --git a/BobbysNews/Presentation/ContentViewModel.swift b/BobbysNews/Presentation/ContentViewModel.swift index b674af8..12d4732 100644 --- a/BobbysNews/Presentation/ContentViewModel.swift +++ b/BobbysNews/Presentation/ContentViewModel.swift @@ -42,7 +42,7 @@ final class ContentViewModel: Sendable { // MARK: - Private Properties - private var cancellable = Set() + private var cancellables = Set() // MARK: - Properties @@ -94,7 +94,7 @@ final class ContentViewModel: Sendable { } func onDisappear() { - cancellable.removeAll() + cancellables.removeAll() } func fetchSources(sensoryFeedback: Bool? = nil) async { @@ -163,7 +163,7 @@ final class ContentViewModel: Sendable { self?.updateStateSources(completion: .finished, state: countries.isEmpty ? self?.stateSources == .emptyFetch ? .emptyFetch : .emptyRead : .loaded) }) - .store(in: &cancellable) + .store(in: &cancellables) } private func readTopHeadlines() { @@ -181,7 +181,7 @@ final class ContentViewModel: Sendable { self?.updateStateTopHeadlines(completion: .finished, state: articles.isEmpty ? self?.stateTopHeadlines == .emptyFetch ? .emptyFetch : .emptyRead : .loaded) }) - .store(in: &cancellable) + .store(in: &cancellables) } private func sensoryFeedbackTrigger(feedback: SensoryFeedback) { diff --git a/BobbysNews/Presentation/DetailView.swift b/BobbysNews/Presentation/DetailView.swift index 1e67014..bec6ea9 100644 --- a/BobbysNews/Presentation/DetailView.swift +++ b/BobbysNews/Presentation/DetailView.swift @@ -59,8 +59,8 @@ struct DetailView: View { } .frame(height: 280) .background(.bar) - .clipShape(UnevenRoundedRectangle(cornerRadii: RectangleCornerRadii(topLeading: 40, - topTrailing: 40))) + .clipShape(.rect(topLeadingRadius: 40, + topTrailingRadius: 40)) .overlay { LinearGradient(gradient: Gradient(colors: [.clear, Color(uiColor: .systemBackground)]), @@ -93,12 +93,10 @@ struct DetailView: View { .font(.system(.caption2, weight: .semibold)) - Button { + Button("Read") { viewModel.showWebView = true - } label: { - Text("Read") - .textCase(.uppercase) } + .textCase(.uppercase) .font(.system(.subheadline, weight: .black)) .accessibilityIdentifier("ReadButton") @@ -157,12 +155,9 @@ struct DetailView: View { showError: $viewModel.webViewShowError, url: url) } else { - ContentUnavailableView { - Label("ErrorWebView", - systemImage: "newspaper.circle.fill") - } description: { - Text("ErrorWebViewMessage") - } + ContentUnavailableView("ErrorWebView", + systemImage: "newspaper.circle.fill", + description: Text("ErrorWebViewMessage")) } } .navigationTitle("Headline") @@ -170,10 +165,9 @@ struct DetailView: View { .ignoresSafeArea(edges: .bottom) .toolbar { ToolbarItem(placement: .cancellationAction) { - Button { + Button("Close", + systemImage: "xmark.circle.fill") { viewModel.showWebView = false - } label: { - Image(systemName: "xmark.circle.fill") } .accessibilityIdentifier("CloseButton") } diff --git a/BobbysNews/Resource/Localizable.xcstrings b/BobbysNews/Resource/Localizable.xcstrings index 6dba654..bec502c 100644 --- a/BobbysNews/Resource/Localizable.xcstrings +++ b/BobbysNews/Resource/Localizable.xcstrings @@ -33,6 +33,22 @@ } } }, + "Close" : { + "localizations" : { + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Schließen" + } + }, + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Close" + } + } + } + }, "CountriesLoad" : { "extractionState" : "manual", "localizations" : { diff --git a/BobbysNewsData/Tests/BobbysNewsDataTests/DB/PersistenceController/SourcesPersistenceControllerTests.swift b/BobbysNewsData/Tests/BobbysNewsDataTests/DB/PersistenceController/SourcesPersistenceControllerTests.swift index 6f3abea..e0fa1a7 100644 --- a/BobbysNewsData/Tests/BobbysNewsDataTests/DB/PersistenceController/SourcesPersistenceControllerTests.swift +++ b/BobbysNewsData/Tests/BobbysNewsDataTests/DB/PersistenceController/SourcesPersistenceControllerTests.swift @@ -13,18 +13,18 @@ class SourcesPersistenceControllerTests: XCTestCase { // MARK: - Private Properties - private var cancellable: Set! + private var cancellables: Set! private var sut: SourcesPersistenceControllerMock! // MARK: - Actions override func setUpWithError() throws { - cancellable = Set() + cancellables = Set() sut = SourcesPersistenceControllerMock() } override func tearDownWithError() throws { - cancellable.removeAll() + cancellables.removeAll() sut = nil } @@ -53,7 +53,7 @@ class SourcesPersistenceControllerTests: XCTestCase { sources = newSources expectation.fulfill() }) - .store(in: &cancellable) + .store(in: &cancellables) // Then await fulfillment(of: [expectation], timeout: 1) XCTAssertNotNil(sources) diff --git a/BobbysNewsData/Tests/BobbysNewsDataTests/DB/PersistenceController/TopHeadlinesDataControllerTests.swift b/BobbysNewsData/Tests/BobbysNewsDataTests/DB/PersistenceController/TopHeadlinesDataControllerTests.swift index fe86cf4..9c82e52 100644 --- a/BobbysNewsData/Tests/BobbysNewsDataTests/DB/PersistenceController/TopHeadlinesDataControllerTests.swift +++ b/BobbysNewsData/Tests/BobbysNewsDataTests/DB/PersistenceController/TopHeadlinesDataControllerTests.swift @@ -13,18 +13,18 @@ class TopHeadlinesPersistenceControllerTests: XCTestCase { // MARK: - Private Properties - private var cancellable: Set! + private var cancellables: Set! private var sut: TopHeadlinesPersistenceControllerMock! // MARK: - Actions override func setUpWithError() throws { - cancellable = Set() + cancellables = Set() sut = TopHeadlinesPersistenceControllerMock() } override func tearDownWithError() throws { - cancellable.removeAll() + cancellables.removeAll() sut = nil } @@ -53,7 +53,7 @@ class TopHeadlinesPersistenceControllerTests: XCTestCase { topHeadlines = newTopHeadlines expectation.fulfill() }) - .store(in: &cancellable) + .store(in: &cancellables) // Then await fulfillment(of: [expectation], timeout: 1) XCTAssertNotNil(topHeadlines) diff --git a/BobbysNewsData/Tests/BobbysNewsDataTests/Repository/SourcesRepositoryTests.swift b/BobbysNewsData/Tests/BobbysNewsDataTests/Repository/SourcesRepositoryTests.swift index ef22ee1..39242cb 100644 --- a/BobbysNewsData/Tests/BobbysNewsDataTests/Repository/SourcesRepositoryTests.swift +++ b/BobbysNewsData/Tests/BobbysNewsDataTests/Repository/SourcesRepositoryTests.swift @@ -13,18 +13,18 @@ class SourcesRepositoryTests: XCTestCase { // MARK: - Private Properties - private var cancellable: Set! + private var cancellables: Set! private var sut: SourcesRepositoryMock! // MARK: - Actions override func setUpWithError() throws { - cancellable = Set() + cancellables = Set() sut = SourcesRepositoryMock() } override func tearDownWithError() throws { - cancellable.removeAll() + cancellables.removeAll() sut = nil } @@ -58,7 +58,7 @@ class SourcesRepositoryTests: XCTestCase { sources = newSources expectation.fulfill() }) - .store(in: &cancellable) + .store(in: &cancellables) // Then await fulfillment(of: [expectation], timeout: 1) XCTAssertNotNil(sources) diff --git a/BobbysNewsData/Tests/BobbysNewsDataTests/Repository/TopHeadlinesRepositoryTests.swift b/BobbysNewsData/Tests/BobbysNewsDataTests/Repository/TopHeadlinesRepositoryTests.swift index a46de0d..146d078 100644 --- a/BobbysNewsData/Tests/BobbysNewsDataTests/Repository/TopHeadlinesRepositoryTests.swift +++ b/BobbysNewsData/Tests/BobbysNewsDataTests/Repository/TopHeadlinesRepositoryTests.swift @@ -13,18 +13,18 @@ class TopHeadlinesRepositoryTests: XCTestCase { // MARK: - Private Properties - private var cancellable: Set! + private var cancellables: Set! private var sut: TopHeadlinesRepositoryMock! // MARK: - Actions override func setUpWithError() throws { - cancellable = Set() + cancellables = Set() sut = TopHeadlinesRepositoryMock() } override func tearDownWithError() throws { - cancellable.removeAll() + cancellables.removeAll() sut = nil } @@ -60,7 +60,7 @@ class TopHeadlinesRepositoryTests: XCTestCase { topHeadlines = newTopHeadlines expectation.fulfill() }) - .store(in: &cancellable) + .store(in: &cancellables) // Then await fulfillment(of: [expectation], timeout: 1) XCTAssertNotNil(topHeadlines) diff --git a/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/Sources/ReadSourcesUseCaseTests.swift b/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/Sources/ReadSourcesUseCaseTests.swift index 9e0e9a7..263f8c6 100644 --- a/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/Sources/ReadSourcesUseCaseTests.swift +++ b/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/Sources/ReadSourcesUseCaseTests.swift @@ -14,18 +14,18 @@ class ReadSourcesUseCaseTests: XCTestCase { // MARK: - Private Properties - private var cancellable: Set! + private var cancellables: Set! private var sut: ReadSourcesUseCase! // MARK: - Actions override func setUpWithError() throws { - cancellable = Set() + cancellables = Set() sut = ReadSourcesUseCase(sourcesRepository: SourcesRepositoryMock()) } override func tearDownWithError() throws { - cancellable.removeAll() + cancellables.removeAll() sut = nil } @@ -41,7 +41,7 @@ class ReadSourcesUseCaseTests: XCTestCase { sources = $0 expectation.fulfill() }) - .store(in: &cancellable) + .store(in: &cancellables) // Then await fulfillment(of: [expectation], timeout: 1) XCTAssertNotNil(sources) diff --git a/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/TopHeadlines/ReadTopHeadlinesUseCaseTests.swift b/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/TopHeadlines/ReadTopHeadlinesUseCaseTests.swift index 3c73826..ab6d17e 100644 --- a/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/TopHeadlines/ReadTopHeadlinesUseCaseTests.swift +++ b/BobbysNewsDomain/Tests/BobbysNewsDomainTests/UseCase/TopHeadlines/ReadTopHeadlinesUseCaseTests.swift @@ -14,18 +14,18 @@ class ReaApipHeadlinesUseCaseTests: XCTestCase { // MARK: - Private Properties - private var cancellable: Set! + private var cancellables: Set! private var sut: ReadTopHeadlinesUseCase! // MARK: - Actions override func setUpWithError() throws { - cancellable = Set() + cancellables = Set() sut = ReadTopHeadlinesUseCase(topHeadlinesRepository: TopHeadlinesRepositoryMock()) } override func tearDownWithError() throws { - cancellable.removeAll() + cancellables.removeAll() sut = nil } @@ -41,7 +41,7 @@ class ReaApipHeadlinesUseCaseTests: XCTestCase { topHeadlines = newTopHeadlines expectation.fulfill() }) - .store(in: &cancellable) + .store(in: &cancellables) // Then await fulfillment(of: [expectation], timeout: 1) XCTAssertNotNil(topHeadlines)