From 4f61755b175aa6f860b88b10763742c23f0ea290 Mon Sep 17 00:00:00 2001 From: Doug Date: Thu, 26 Jan 2023 11:59:19 +0000 Subject: [PATCH] Fix avatar loading in SwiftUI. --- .../Common/Avatar/View/AvatarImage.swift | 21 ++++++---- .../Common/Avatar/View/SpaceAvatarImage.swift | 40 +++++++++++-------- .../Avatar/ViewModel/AvatarViewModel.swift | 25 +++++++----- changelog.d/7305.bugfix | 1 + 4 files changed, 52 insertions(+), 35 deletions(-) create mode 100644 changelog.d/7305.bugfix diff --git a/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift b/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift index b143d4d306..3ddd4d472c 100644 --- a/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift +++ b/RiotSwiftUI/Modules/Common/Avatar/View/AvatarImage.swift @@ -26,9 +26,11 @@ struct AvatarImage: View { var displayName: String? var size: AvatarSize + @State private var avatar: AvatarViewState = .empty + var body: some View { Group { - switch viewModel.viewState { + switch avatar { case .empty: ProgressView() case .placeholder(let firstCharacter, let colorIndex): @@ -42,13 +44,16 @@ struct AvatarImage: View { .frame(maxWidth: CGFloat(size.rawValue), maxHeight: CGFloat(size.rawValue)) .clipShape(Circle()) .onAppear { - viewModel.loadAvatar( - mxContentUri: mxContentUri, - matrixItemId: matrixItemId, - displayName: displayName, - colorCount: theme.colors.namesAndAvatars.count, - avatarSize: size - ) + avatar = viewModel.placeholderAvatar(matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count) + viewModel.loadAvatar(mxContentUri: mxContentUri, + matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count, + avatarSize: size ) { newState in + avatar = newState + } } } } diff --git a/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift b/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift index 2662831e15..708c26a2b5 100644 --- a/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift +++ b/RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift @@ -26,9 +26,11 @@ struct SpaceAvatarImage: View { var displayName: String? var size: AvatarSize + @State private var avatar: AvatarViewState = .empty + var body: some View { Group { - switch viewModel.viewState { + switch avatar { case .empty: ProgressView() case .placeholder(let firstCharacter, let colorIndex): @@ -48,23 +50,27 @@ struct SpaceAvatarImage: View { .clipShape(RoundedRectangle(cornerRadius: 8)) } } - .onChange(of: displayName, perform: { value in - viewModel.loadAvatar( - mxContentUri: mxContentUri, - matrixItemId: matrixItemId, - displayName: value, - colorCount: theme.colors.namesAndAvatars.count, - avatarSize: size - ) - }) + .onChange(of: displayName) { value in + guard case .placeholder = avatar else { return } + viewModel.loadAvatar(mxContentUri: mxContentUri, + matrixItemId: matrixItemId, + displayName: value, + colorCount: theme.colors.namesAndAvatars.count, + avatarSize: size) { newState in + avatar = newState + } + } .onAppear { - viewModel.loadAvatar( - mxContentUri: mxContentUri, - matrixItemId: matrixItemId, - displayName: displayName, - colorCount: theme.colors.namesAndAvatars.count, - avatarSize: size - ) + avatar = viewModel.placeholderAvatar(matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count) + viewModel.loadAvatar(mxContentUri: mxContentUri, + matrixItemId: matrixItemId, + displayName: displayName, + colorCount: theme.colors.namesAndAvatars.count, + avatarSize: size) { newState in + avatar = newState + } } } } diff --git a/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift b/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift index 10055738d4..68037f5521 100644 --- a/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift +++ b/RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewModel.swift @@ -22,14 +22,22 @@ import Foundation final class AvatarViewModel: ObservableObject { private let avatarService: AvatarServiceProtocol - @Published private(set) var viewState = AvatarViewState.empty - init(avatarService: AvatarServiceProtocol) { self.avatarService = avatarService } private var cancellables = Set() + func placeholderAvatar(matrixItemId: String, + displayName: String?, + colorCount: Int) -> AvatarViewState { + let placeholderViewModel = PlaceholderAvatarViewModel(displayName: displayName, + matrixItemId: matrixItemId, + colorCount: colorCount) + + return .placeholder(placeholderViewModel.firstCharacterCapitalized, placeholderViewModel.stableColorIndex) + } + /// Load an avatar /// - Parameters: /// - mxContentUri: The matrix content URI of the avatar. @@ -41,14 +49,10 @@ final class AvatarViewModel: ObservableObject { matrixItemId: String, displayName: String?, colorCount: Int, - avatarSize: AvatarSize) { - let placeholderViewModel = PlaceholderAvatarViewModel(displayName: displayName, - matrixItemId: matrixItemId, - colorCount: colorCount) - - viewState = .placeholder(placeholderViewModel.firstCharacterCapitalized, placeholderViewModel.stableColorIndex) - + avatarSize: AvatarSize, + avatarCompletion: @escaping (AvatarViewState) -> Void) { guard let mxContentUri = mxContentUri, mxContentUri.count > 0 else { + avatarCompletion(placeholderAvatar(matrixItemId: matrixItemId, displayName: displayName, colorCount: colorCount)) return } @@ -56,8 +60,9 @@ final class AvatarViewModel: ObservableObject { .sink { completion in guard case let .failure(error) = completion else { return } UILog.error("[AvatarService] Failed to retrieve avatar", context: error) + // No need to call the completion, there's nothing we can do and the error is logged. } receiveValue: { image in - self.viewState = .avatar(image) + avatarCompletion(.avatar(image)) } .store(in: &cancellables) } diff --git a/changelog.d/7305.bugfix b/changelog.d/7305.bugfix new file mode 100644 index 0000000000..5f940f9463 --- /dev/null +++ b/changelog.d/7305.bugfix @@ -0,0 +1 @@ +Space Switcher: Fix a bug where the avatars would all be the same.