From 1ab56fd8d197ddb5880a1bd88f677f6ef052ea16 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Wed, 2 Aug 2023 13:24:57 +0300 Subject: [PATCH] Simplify the screen templates --- .../UITests/UITestsAppCoordinator.swift | 6 +- .../UITests/UITestsScreenIdentifier.swift | 3 +- .../ElementX/TemplateScreenCoordinator.swift | 18 ++-- .../ElementX/TemplateScreenModels.swift | 45 +++------- .../ElementX/TemplateScreenViewModel.swift | 20 ++--- .../ElementX/View/TemplateScreen.swift | 87 +++++-------------- .../Tests/UI/TemplateScreenUITests.swift | 21 +---- .../Unit/TemplateScreenViewModelTests.swift | 20 ++--- 8 files changed, 60 insertions(+), 160 deletions(-) diff --git a/ElementX/Sources/UITests/UITestsAppCoordinator.swift b/ElementX/Sources/UITests/UITestsAppCoordinator.swift index 0d2402228..14eaf0591 100644 --- a/ElementX/Sources/UITests/UITestsAppCoordinator.swift +++ b/ElementX/Sources/UITests/UITestsAppCoordinator.swift @@ -148,10 +148,8 @@ class MockScreen: Identifiable { userIndicatorController: ServiceLocator.shared.userIndicatorController)) navigationStackCoordinator.setRootCoordinator(coordinator) return navigationStackCoordinator - case .simpleRegular: - return TemplateScreenCoordinator(parameters: .init(promptType: .regular)) - case .simpleUpgrade: - return TemplateScreenCoordinator(parameters: .init(promptType: .upgrade)) + case .templateScreen: + return TemplateScreenCoordinator(parameters: .init()) case .home: let navigationStackCoordinator = NavigationStackCoordinator() let session = MockUserSession(clientProxy: MockClientProxy(userID: "@mock:matrix.org"), diff --git a/ElementX/Sources/UITests/UITestsScreenIdentifier.swift b/ElementX/Sources/UITests/UITestsScreenIdentifier.swift index 04eb15222..843f2b674 100644 --- a/ElementX/Sources/UITests/UITestsScreenIdentifier.swift +++ b/ElementX/Sources/UITests/UITestsScreenIdentifier.swift @@ -28,8 +28,7 @@ enum UITestsScreenIdentifier: String { case analyticsPrompt case analyticsSettingsScreen case migration - case simpleRegular - case simpleUpgrade + case templateScreen case home case settings case bugReport diff --git a/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenCoordinator.swift b/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenCoordinator.swift index c2663c615..9ff71f1d9 100644 --- a/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenCoordinator.swift +++ b/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenCoordinator.swift @@ -17,13 +17,10 @@ import Combine import SwiftUI -struct TemplateScreenCoordinatorParameters { - let promptType: TemplateScreenPromptType -} +struct TemplateScreenCoordinatorParameters { } enum TemplateScreenCoordinatorAction { - case accept - case cancel + case done // Consider adding CustomStringConvertible conformance if the actions contain PII } @@ -41,18 +38,17 @@ final class TemplateScreenCoordinator: CoordinatorProtocol { init(parameters: TemplateScreenCoordinatorParameters) { self.parameters = parameters - viewModel = TemplateScreenViewModel(promptType: parameters.promptType) + viewModel = TemplateScreenViewModel() } func start() { viewModel.actions.sink { [weak self] action in + MXLog.info("Coordinator: received view model action: \(action)") + guard let self else { return } switch action { - case .accept: - MXLog.info("User accepted the prompt.") - self.actionsSubject.send(.accept) - case .cancel: - self.actionsSubject.send(.cancel) + case .done: + self.actionsSubject.send(.done) } } .store(in: &cancellables) diff --git a/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenModels.swift b/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenModels.swift index 92a2ab9d8..dcd0d4a1c 100644 --- a/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenModels.swift +++ b/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenModels.swift @@ -16,50 +16,25 @@ import Foundation -enum TemplateScreenPromptType { - case regular - case upgrade -} - -extension TemplateScreenPromptType: Identifiable, CaseIterable { - var id: Self { self } - - var title: String { - switch self { - case .regular: - return "Make this chat public?" - case .upgrade: - return "Privacy warning" - } - } - - var imageSystemName: String { - switch self { - case .regular: - return "app.gift" - case .upgrade: - return "shield" - } - } -} - enum TemplateScreenViewModelAction { - case accept - case cancel + case done // Consider adding CustomStringConvertible conformance if the actions contain PII } struct TemplateScreenViewState: BindableState { - var promptType: TemplateScreenPromptType - var count: Int + var title: String + var placeholder: String + var bindings: TemplateScreenViewStateBindings +} + +struct TemplateScreenViewStateBindings { + var composerText: String } enum TemplateScreenViewAction { - case incrementCount - case decrementCount - case accept - case cancel + case done + case textChanged // Consider adding CustomStringConvertible conformance if the actions contain PII } diff --git a/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenViewModel.swift b/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenViewModel.swift index 4319c7563..f39432759 100644 --- a/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenViewModel.swift +++ b/Tools/Scripts/Templates/SimpleScreenExample/ElementX/TemplateScreenViewModel.swift @@ -26,22 +26,22 @@ class TemplateScreenViewModel: TemplateScreenViewModelType, TemplateScreenViewMo actionsSubject.eraseToAnyPublisher() } - init(promptType: TemplateScreenPromptType, initialCount: Int = 0) { - super.init(initialViewState: TemplateScreenViewState(promptType: promptType, count: 0)) + init() { + super.init(initialViewState: TemplateScreenViewState(title: "Template title", + placeholder: "Enter something here", + bindings: .init(composerText: "Initial composer text"))) } // MARK: - Public override func process(viewAction: TemplateScreenViewAction) { + MXLog.info("View model: received view action: \(viewAction)") + switch viewAction { - case .accept: - actionsSubject.send(.accept) - case .cancel: - actionsSubject.send(.cancel) - case .incrementCount: - state.count += 1 - case .decrementCount: - state.count -= 1 + case .done: + actionsSubject.send(.done) + case .textChanged: + MXLog.info("View model: composer text changed to: \(state.bindings.composerText)") } } } diff --git a/Tools/Scripts/Templates/SimpleScreenExample/ElementX/View/TemplateScreen.swift b/Tools/Scripts/Templates/SimpleScreenExample/ElementX/View/TemplateScreen.swift index 1b0c00359..4589456da 100644 --- a/Tools/Scripts/Templates/SimpleScreenExample/ElementX/View/TemplateScreen.swift +++ b/Tools/Scripts/Templates/SimpleScreenExample/ElementX/View/TemplateScreen.swift @@ -17,75 +17,30 @@ import SwiftUI struct TemplateScreen: View { - @Environment(\.colorScheme) private var colorScheme - - var counterColor: Color { - colorScheme == .light ? .compound.textSecondary : .compound.textInfoPrimary - } - @ObservedObject var context: TemplateScreenViewModel.Context var body: some View { - ScrollView { - mainContent - .padding(.top, 50) - .padding(.horizontal) - .readableFrame() - } - .safeAreaInset(edge: .bottom) { - buttons - .padding(.horizontal) - .padding(.vertical) - .readableFrame() - .background(Color.compound.bgSubtleSecondary) - } - } - - /// The main content of the view to be shown in a scroll view. - var mainContent: some View { - VStack(spacing: 36) { - Text(context.viewState.promptType.title) - .font(.compound.headingMDBold) - .multilineTextAlignment(.center) - .foregroundColor(.compound.textPrimary) - .accessibilityIdentifier("title") - - Image(systemName: context.viewState.promptType.imageSystemName) - .resizable() - .scaledToFit() - .frame(width: 100) - - HStack { - Text("Counter: \(context.viewState.count)") - .font(.compound.bodyLG) - .multilineTextAlignment(.center) - .foregroundColor(counterColor) - - Button("−") { - context.send(viewAction: .decrementCount) + Form { + Section { + TextField(text: $context.composerText) { + Text(context.viewState.placeholder) + .compoundFormTextFieldPlaceholder() } - .buttonStyle(.elementGhost()) + .textFieldStyle(.compoundForm) - Button("+") { - context.send(viewAction: .incrementCount) + Button { + context.send(viewAction: .done) + } label: { + Label("Done", systemImage: "door.left.hand.closed") } - .buttonStyle(.elementGhost()) + .buttonStyle(.compoundFormCentred()) } + .compoundFormSection() } - } - - /// The action buttons shown at the bottom of the view. - var buttons: some View { - VStack { - Button { context.send(viewAction: .accept) } label: { - Text("Accept") - } - .buttonStyle(.elementAction(.xLarge)) - - Button { context.send(viewAction: .cancel) } label: { - Text("Cancel") - .padding(.vertical, 12) - } + .compoundForm() + .navigationTitle(context.viewState.title) + .onChange(of: context.composerText) { _ in + context.send(viewAction: .textChanged) } } } @@ -93,12 +48,10 @@ struct TemplateScreen: View { // MARK: - Previews struct TemplateScreen_Previews: PreviewProvider { - static let regularViewModel = TemplateScreenViewModel(promptType: .regular) - static let upgradeViewModel = TemplateScreenViewModel(promptType: .upgrade) + static let viewModel = TemplateScreenViewModel() static var previews: some View { - TemplateScreen(context: regularViewModel.context) - .previewDisplayName("Regular") - TemplateScreen(context: upgradeViewModel.context) - .previewDisplayName("Upgrade") + NavigationStack { + TemplateScreen(context: viewModel.context) + } } } diff --git a/Tools/Scripts/Templates/SimpleScreenExample/Tests/UI/TemplateScreenUITests.swift b/Tools/Scripts/Templates/SimpleScreenExample/Tests/UI/TemplateScreenUITests.swift index 1e2640874..4080c453c 100644 --- a/Tools/Scripts/Templates/SimpleScreenExample/Tests/UI/TemplateScreenUITests.swift +++ b/Tools/Scripts/Templates/SimpleScreenExample/Tests/UI/TemplateScreenUITests.swift @@ -19,25 +19,12 @@ import XCTest @MainActor class TemplateScreenUITests: XCTestCase { - func testRegularScreen() async throws { - let app = Application.launch(.simpleRegular) + func testScreen() async throws { + let app = Application.launch(.templateScreen) - let title = app.staticTexts["title"] + let title = app.staticTexts["Template title"] XCTAssert(title.exists) - - XCTAssertEqual(title.label, "Make this chat public?") - try await app.assertScreenshot(.simpleRegular) - } - - func testUpgradeScreen() async throws { - let app = Application.launch(.simpleUpgrade) - - let title = app.staticTexts["title"] - XCTAssert(title.exists) - - XCTAssertEqual(title.label, "Privacy warning") - - try await app.assertScreenshot(.simpleUpgrade) + try await app.assertScreenshot(.templateScreen) } } diff --git a/Tools/Scripts/Templates/SimpleScreenExample/Tests/Unit/TemplateScreenViewModelTests.swift b/Tools/Scripts/Templates/SimpleScreenExample/Tests/Unit/TemplateScreenViewModelTests.swift index a1f3c0c29..fdce7269c 100644 --- a/Tools/Scripts/Templates/SimpleScreenExample/Tests/Unit/TemplateScreenViewModelTests.swift +++ b/Tools/Scripts/Templates/SimpleScreenExample/Tests/Unit/TemplateScreenViewModelTests.swift @@ -20,10 +20,6 @@ import XCTest @MainActor class TemplateScreenViewModelTests: XCTestCase { - private enum Constants { - static let counterInitialValue = 0 - } - var viewModel: TemplateScreenViewModelProtocol! var context: TemplateScreenViewModelType.Context { @@ -31,21 +27,17 @@ class TemplateScreenViewModelTests: XCTestCase { } override func setUpWithError() throws { - viewModel = TemplateScreenViewModel(promptType: .regular, initialCount: Constants.counterInitialValue) + viewModel = TemplateScreenViewModel() } func testInitialState() { - XCTAssertEqual(context.viewState.count, Constants.counterInitialValue) + XCTAssertFalse(context.viewState.placeholder.isEmpty) + XCTAssertFalse(context.composerText.isEmpty) } func testCounter() async throws { - context.send(viewAction: .incrementCount) - XCTAssertEqual(context.viewState.count, 1) - - context.send(viewAction: .incrementCount) - XCTAssertEqual(context.viewState.count, 2) - - context.send(viewAction: .decrementCount) - XCTAssertEqual(context.viewState.count, 1) + context.composerText = "123" + context.send(viewAction: .textChanged) + XCTAssertEqual(context.composerText, "123") } }