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

Settings Screen New Design #196

Merged
merged 9 commits into from
Sep 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"screenshot_detected_title" = "You took a screenshot";
"screenshot_detected_message" = "Would you like to submit a bug report?";

"settings_appearance" = "Appearance";
"settings_timeline_style" = "Timeline Style";
"room_timeline_style_plain_long_description" = "Plain Timeline";
"room_timeline_style_bubbled_long_description" = "Bubbled Timeline";
Expand Down
36 changes: 19 additions & 17 deletions ElementX/Sources/AppCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,7 @@ class AppCoordinator: Coordinator {
case (.remoteSigningOut(let isSoft), .completedSigningOut, .signedOut):
self.presentSplashScreen(isSoftLogout: isSoft)
self.hideLoadingIndicator()

case (.homeScreen, .showSettingsScreen, .settingsScreen):
self.presentSettingsScreen()
case (.settingsScreen, .dismissedSettingsScreen, .homeScreen):
self.tearDownDismissedSettingsScreen()


case (.homeScreen, .showSessionVerificationScreen, .sessionVerificationScreen):
self.presentSessionVerification()
case (.sessionVerificationScreen, .dismissedSessionVerificationScreen, .homeScreen):
Expand Down Expand Up @@ -309,7 +304,7 @@ class AppCoordinator: Coordinator {
case .presentRoom(let roomIdentifier):
self.stateMachine.processEvent(.showRoomScreen(roomId: roomIdentifier))
case .presentSettings:
self.stateMachine.processEvent(.showSettingsScreen)
self.presentSettingsScreen()
case .presentBugReport:
self.presentBugReportScreen()
case .verifySession:
Expand Down Expand Up @@ -394,32 +389,39 @@ class AppCoordinator: Coordinator {
// MARK: Settings

private func presentSettingsScreen() {
let parameters = SettingsCoordinatorParameters(navigationRouter: navigationRouter,
let navController = ElementNavigationController()
let newNavigationRouter = NavigationRouter(navigationController: navController)

let parameters = SettingsCoordinatorParameters(navigationRouter: newNavigationRouter,
userSession: userSession,
bugReportService: bugReportService)
let coordinator = SettingsCoordinator(parameters: parameters)
coordinator.callback = { [weak self] action in
guard let self = self else { return }
switch action {
case .dismiss:
self.dismissSettingsScreen()
case .logout:
self.dismissSettingsScreen()
self.stateMachine.processEvent(.signOut)
}
}

add(childCoordinator: coordinator)
coordinator.start()
navigationRouter.push(coordinator) { [weak self] in
guard let self = self else { return }

self.stateMachine.processEvent(.dismissedSettingsScreen)
}
navController.viewControllers = [coordinator.toPresentable()]
navigationRouter.present(navController, animated: true)
}

private func tearDownDismissedSettingsScreen() {
guard let coordinator = childCoordinators.last as? SettingsCoordinator else {
fatalError("Invalid coordinator hierarchy: \(childCoordinators)")

@objc
private func dismissSettingsScreen() {
MXLog.debug("dismissSettingsScreen")

guard let coordinator = childCoordinators.first(where: { $0 is SettingsCoordinator }) else {
return
}

navigationRouter.dismissModule()
remove(childCoordinator: coordinator)
}

Expand Down
13 changes: 1 addition & 12 deletions ElementX/Sources/AppCoordinatorStateMachine.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ class AppCoordinatorStateMachine {

/// Showing the session verification flows
case sessionVerificationScreen

/// Showing the settings screen
case settingsScreen


/// Processing a sign out request
case signingOut

Expand Down Expand Up @@ -77,11 +74,6 @@ class AppCoordinatorStateMachine {
case showSessionVerificationScreen
/// Session verification has finished
case dismissedSessionVerificationScreen

/// Request settings screen presentation
case showSettingsScreen
/// The settings screen has been dismissed
case dismissedSettingsScreen
Comment on lines -80 to -84
Copy link
Member

Choose a reason for hiding this comment

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

How come you modified the state machine here? Presumably to inject the user session?

I think we need to separate out into 2 of these now, one top level for the whole app and another for navigation with a single user session navigation.

}

private let stateMachine: StateMachine<State, Event>
Expand All @@ -97,9 +89,6 @@ class AppCoordinatorStateMachine {

machine.addRoutes(event: .signOut, transitions: [.any => .signingOut])
machine.addRoutes(event: .completedSigningOut, transitions: [.signingOut => .signedOut])

machine.addRoutes(event: .showSettingsScreen, transitions: [.homeScreen => .settingsScreen])
machine.addRoutes(event: .dismissedSettingsScreen, transitions: [.settingsScreen => .homeScreen])

machine.addRoutes(event: .showSessionVerificationScreen, transitions: [.homeScreen => .sessionVerificationScreen])
machine.addRoutes(event: .dismissedSessionVerificationScreen, transitions: [.sessionVerificationScreen => .homeScreen])
Expand Down
2 changes: 2 additions & 0 deletions ElementX/Sources/Generated/Strings+Untranslated.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ extension ElementL10n {
public static let sessionVerificationScreenEmojisMessage = ElementL10n.tr("Untranslated", "session_verification_screen_emojis_message")
/// Lets check if these
public static let sessionVerificationScreenEmojisTitle = ElementL10n.tr("Untranslated", "session_verification_screen_emojis_title")
/// Appearance
public static let settingsAppearance = ElementL10n.tr("Untranslated", "settings_appearance")
/// Timeline Style
public static let settingsTimelineStyle = ElementL10n.tr("Untranslated", "settings_timeline_style")
/// Untranslated
Expand Down
12 changes: 12 additions & 0 deletions ElementX/Sources/Other/Extensions/String.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,16 @@ extension String {
let detector = try? NSRegularExpression(pattern: MatrixEntityRegex.userId.rawValue, options: .caseInsensitive)
return detector?.numberOfMatches(in: self, range: range) ?? 0 == 1
}

/// Calculates a numeric hash same as Element Web
/// See original function here https://github.com/matrix-org/matrix-react-sdk/blob/321dd49db4fbe360fc2ff109ac117305c955b061/src/utils/FormattingUtils.js#L47
var hashCode: Int32 {
var hash: Int32 = 0

for character in self {
let shiftedHash = hash << 5
hash = shiftedHash.subtractingReportingOverflow(hash).partialValue + Int32(character.unicodeScalars[character.unicodeScalars.startIndex].value)
}
return abs(hash)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct HomeScreenRoomCell: View {
.frame(width: avatarSize, height: avatarSize)
.clipShape(Circle())
} else {
PlaceholderAvatarImage(text: room.name)
PlaceholderAvatarImage(text: room.name, contentId: room.id)
.clipShape(Circle())
.frame(width: avatarSize, height: avatarSize)
}
Expand Down
1 change: 1 addition & 0 deletions ElementX/Sources/Screens/RoomScreen/RoomScreenModels.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ enum RoomScreenViewAction {
}

struct RoomScreenViewState: BindableState {
var roomId: String
var roomTitle = ""
var roomAvatar: UIImage?
var roomEncryptionBadge: UIImage?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
self.timelineController = timelineController
self.timelineViewFactory = timelineViewFactory

super.init(initialViewState: RoomScreenViewState(roomTitle: roomName ?? "Unknown room 💥",
super.init(initialViewState: RoomScreenViewState(roomId: timelineController.roomId,
roomTitle: roomName ?? "Unknown room 💥",
roomAvatar: roomAvatar,
roomEncryptionBadge: roomEncryptionBadge,
bindings: .init(composerText: "", composerFocused: false)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ struct RoomHeaderView: View {
.scaledToFill()
.accessibilityIdentifier("roomAvatarImage")
} else {
PlaceholderAvatarImage(text: context.viewState.roomTitle)
PlaceholderAvatarImage(text: context.viewState.roomTitle,
contentId: context.viewState.roomId)
.accessibilityIdentifier("roomAvatarPlaceholderImage")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,32 @@ import SwiftUI

struct PlaceholderAvatarImage: View {
private let textForImage: String
private let contentId: String?

var body: some View {
ZStack {
Color.element.accent
bgColor
Text(textForImage)
.padding(4)
.foregroundColor(.element.background)
// Make the text resizable (i.e. Make it large and then allow it to scale down)
.font(.system(size: 200).weight(.semibold))
.minimumScaleFactor(0.001)
.font(.title2.bold())
ismailgulek marked this conversation as resolved.
Show resolved Hide resolved
}
.aspectRatio(1, contentMode: .fill)
}

init(text: String) {
init(text: String, contentId: String? = nil) {
textForImage = text.first?.uppercased() ?? ""
self.contentId = contentId
}

private var bgColor: Color {
guard let contentId = contentId else {
return .element.accent
}

let colors = Color.element.contentAndAvatars
let colorIndex = Int(contentId.hashCode % Int32(colors.count))
return Color.element.contentAndAvatars[colorIndex % colors.count]
ismailgulek marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -45,7 +55,7 @@ struct PlaceholderAvatarImage_Previews: PreviewProvider {

@ViewBuilder
static var body: some View {
PlaceholderAvatarImage(text: "X")
PlaceholderAvatarImage(text: "X", contentId: "@userid:matrix.org")
.clipShape(Circle())
.frame(width: 150, height: 100)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ struct TimelineSenderAvatarView: View {
.scaledToFill()
.overlay(Circle().stroke(Color.element.accent))
} else {
PlaceholderAvatarImage(text: timelineItem.senderDisplayName ?? timelineItem.senderId)
PlaceholderAvatarImage(text: timelineItem.senderDisplayName ?? timelineItem.senderId,
contentId: timelineItem.senderId)
}
}
.clipShape(Circle())
Expand Down
24 changes: 21 additions & 3 deletions ElementX/Sources/Screens/Settings/SettingsCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ struct SettingsCoordinatorParameters {
}

enum SettingsCoordinatorAction {
case dismiss
case logout
}

Expand All @@ -38,6 +39,8 @@ final class SettingsCoordinator: Coordinator, Presentable {
private var indicatorPresenter: UserIndicatorTypePresenterProtocol
private var loadingIndicator: UserIndicator?
private var statusIndicator: UserIndicator?

private var navigationRouter: NavigationRouterType { parameters.navigationRouter }

// MARK: Public

Expand All @@ -50,7 +53,7 @@ final class SettingsCoordinator: Coordinator, Presentable {
init(parameters: SettingsCoordinatorParameters) {
self.parameters = parameters

let viewModel = SettingsViewModel()
let viewModel = SettingsViewModel(withUserSession: parameters.userSession)
let view = SettingsScreen(context: viewModel.context)
settingsViewModel = viewModel
settingsHostingController = UIHostingController(rootView: view)
Expand All @@ -61,14 +64,16 @@ final class SettingsCoordinator: Coordinator, Presentable {
guard let self = self else { return }
MXLog.debug("SettingsViewModel did complete with result: \(result).")
switch result {
case .close:
self.callback?(.dismiss)
case .toggleAnalytics:
self.toggleAnalytics()
case .reportBug:
self.presentBugReportScreen()
case .crash:
self.parameters.bugReportService.crash()
case .logout:
self.callback?(.logout)
self.confirmSignOut()
}
}
}
Expand Down Expand Up @@ -106,12 +111,25 @@ final class SettingsCoordinator: Coordinator, Presentable {

add(childCoordinator: coordinator)
coordinator.start()
parameters.navigationRouter.push(coordinator, animated: true) { [weak self] in
navigationRouter.push(coordinator, animated: true) { [weak self] in
guard let self = self else { return }

self.remove(childCoordinator: coordinator)
}
}

private func confirmSignOut() {
ismailgulek marked this conversation as resolved.
Show resolved Hide resolved
let alert = UIAlertController(title: ElementL10n.actionSignOut,
message: ElementL10n.actionSignOutConfirmationSimple,
preferredStyle: .alert)

alert.addAction(UIAlertAction(title: ElementL10n.actionCancel, style: .cancel))
alert.addAction(UIAlertAction(title: ElementL10n.actionSignOut, style: .destructive) { [weak self] _ in
self?.callback?(.logout)
})

navigationRouter.present(alert, animated: true)
}

/// Show an activity indicator whilst loading.
/// - Parameters:
Expand Down
6 changes: 6 additions & 0 deletions ElementX/Sources/Screens/Settings/SettingsModels.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@
//

import Foundation
import UIKit

// MARK: - Coordinator

// MARK: View model

enum SettingsViewModelAction {
case close
case toggleAnalytics
case reportBug
case crash
Expand All @@ -31,13 +33,17 @@ enum SettingsViewModelAction {

struct SettingsViewState: BindableState {
var bindings: SettingsViewStateBindings
var userID: String
var userAvatar: UIImage?
var userDisplayName: String?
}

struct SettingsViewStateBindings {
var enableAnalytics = ElementSettings.shared.enableAnalytics
}

enum SettingsViewAction {
case close
case toggleAnalytics
case reportBug
case crash
Expand Down
21 changes: 19 additions & 2 deletions ElementX/Sources/Screens/Settings/SettingsViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,38 @@ class SettingsViewModel: SettingsViewModelType, SettingsViewModelProtocol {

// MARK: Private

private let userSession: UserSessionProtocol

// MARK: Public

var callback: ((SettingsViewModelAction) -> Void)?

// MARK: - Setup

init() {
init(withUserSession userSession: UserSessionProtocol) {
self.userSession = userSession
let bindings = SettingsViewStateBindings()
super.init(initialViewState: .init(bindings: bindings))
super.init(initialViewState: .init(bindings: bindings, userID: userSession.userID))

Task {
if case let .success(userAvatarURLString) = await userSession.clientProxy.loadUserAvatarURLString() {
if case let .success(avatar) = await userSession.mediaProvider.loadImageFromURLString(userAvatarURLString, size: MediaProviderDefaultAvatarSize) {
state.userAvatar = avatar
}
}

if case let .success(userDisplayName) = await self.userSession.clientProxy.loadUserDisplayName() {
state.userDisplayName = userDisplayName
}
ismailgulek marked this conversation as resolved.
Show resolved Hide resolved
}
}

// MARK: - Public

override func process(viewAction: SettingsViewAction) async {
switch viewAction {
case .close:
callback?(.close)
case .toggleAnalytics:
callback?(.toggleAnalytics)
case .reportBug:
Expand Down
Loading