From 24835a566efd37d588242ea2a3eefca51dc67881 Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Fri, 27 Oct 2023 09:29:20 +0200 Subject: [PATCH] Removed Pill Avatar + @room is not displayed among suggestions for DM (#1968) * removed pill avatar * completion suggestion service shows @room as first and only if not a DM * code improvement --- .../Sources/Other/Pills/PillContext.swift | 27 ++++++++++--------- ElementX/Sources/Other/Pills/PillView.swift | 25 ++++++++--------- .../CompletionSuggestionService.swift | 10 ++++--- .../CompletionSuggestionServiceTests.swift | 6 ++--- UnitTests/Sources/PillContextTests.swift | 13 ++------- .../test_messageText.Custom-Attachment.png | 4 +-- .../PreviewTests/test_pillView.All-Users.png | 4 +-- .../test_pillView.Loaded-Long-Own.png | 4 +-- .../test_pillView.Loaded-Long.png | 4 +-- .../test_pillView.Loading-Own.png | 4 +-- .../PreviewTests/test_pillView.Loading.png | 4 +-- 11 files changed, 50 insertions(+), 55 deletions(-) diff --git a/ElementX/Sources/Other/Pills/PillContext.swift b/ElementX/Sources/Other/Pills/PillContext.swift index c66fad64d..49a1a618f 100644 --- a/ElementX/Sources/Other/Pills/PillContext.swift +++ b/ElementX/Sources/Other/Pills/PillContext.swift @@ -20,11 +20,8 @@ import Foundation @MainActor final class PillContext: ObservableObject { struct PillViewState: Equatable { - let contentID: String let isOwnMention: Bool - let name: String? let displayText: String - let avatarURL: URL? } @Published private(set) var viewState: PillViewState @@ -36,23 +33,29 @@ final class PillContext: ObservableObject { case let .user(id): let isOwnMention = id == roomContext.viewState.ownUserID if let profile = roomContext.viewState.members[id] { - let name = profile.displayName ?? id - viewState = PillViewState(contentID: id, isOwnMention: isOwnMention, name: name, displayText: name, avatarURL: profile.avatarURL) + var name = id + if let displayName = profile.displayName { + name = "@\(displayName)" + } + viewState = PillViewState(isOwnMention: isOwnMention, displayText: name) } else { - viewState = PillViewState(contentID: id, isOwnMention: isOwnMention, name: nil, displayText: id, avatarURL: nil) + viewState = PillViewState(isOwnMention: isOwnMention, displayText: id) cancellable = roomContext.$viewState.sink { [weak self] viewState in guard let self else { return } if let profile = viewState.members[id] { - let name = profile.displayName ?? id - self.viewState = PillViewState(contentID: id, isOwnMention: isOwnMention, name: name, displayText: name, avatarURL: profile.avatarURL) + var name = id + if let displayName = profile.displayName { + name = "@\(displayName)" + } + self.viewState = PillViewState(isOwnMention: isOwnMention, displayText: name) cancellable = nil } } } case .allUsers: - viewState = PillViewState(contentID: roomContext.viewState.roomID, isOwnMention: true, name: roomContext.viewState.roomTitle, displayText: PillConstants.atRoom, avatarURL: roomContext.viewState.roomAvatarURL) + viewState = PillViewState(isOwnMention: true, displayText: PillConstants.atRoom) } } } @@ -71,16 +74,16 @@ extension PillContext { case .loadUser(let isOwn): pillType = .user(userID: testID) let viewModel = PillContext(roomContext: RoomScreenViewModel.mock.context, data: PillTextAttachmentData(type: pillType, font: .preferredFont(forTextStyle: .body))) - viewModel.viewState = PillViewState(contentID: testID, isOwnMention: isOwn, name: nil, displayText: testID, avatarURL: nil) + viewModel.viewState = PillViewState(isOwnMention: isOwn, displayText: testID) Task { try? await Task.sleep(for: .seconds(2)) - viewModel.viewState = PillViewState(contentID: "@test:test.com", isOwnMention: isOwn, name: nil, displayText: "Test Long Display Text", avatarURL: URL.documentsDirectory) + viewModel.viewState = PillViewState(isOwnMention: isOwn, displayText: "@Test Long Display Text") } return viewModel case .loadedUser(let isOwn): pillType = .user(userID: "@test:test.com") let viewModel = PillContext(roomContext: RoomScreenViewModel.mock.context, data: PillTextAttachmentData(type: pillType, font: .preferredFont(forTextStyle: .body))) - viewModel.viewState = PillViewState(contentID: "@test:test.com", isOwnMention: isOwn, name: nil, displayText: "Very Very Long Test Display Text", avatarURL: URL.documentsDirectory) + viewModel.viewState = PillViewState(isOwnMention: isOwn, displayText: "@Very Very Long Test Display Text") return viewModel case .allUsers: pillType = .allUsers diff --git a/ElementX/Sources/Other/Pills/PillView.swift b/ElementX/Sources/Other/Pills/PillView.swift index d280d7219..9ba7bc003 100644 --- a/ElementX/Sources/Other/Pills/PillView.swift +++ b/ElementX/Sources/Other/Pills/PillView.swift @@ -31,20 +31,17 @@ struct PillView: View { } var body: some View { - HStack(spacing: 2) { - LoadableAvatarImage(url: context.viewState.avatarURL, name: context.viewState.name, contentID: context.viewState.contentID, avatarSize: .custom(16), imageProvider: imageProvider) - Text(context.viewState.displayText) - .font(.compound.bodyLGSemibold) - .foregroundColor(textColor) - .lineLimit(1) - } - .padding(.leading, 4) - .padding(.trailing, 6) - .padding(.vertical, 1) - .background { Capsule().foregroundColor(backgroundColor) } - .onChange(of: context.viewState.displayText) { _ in - didChangeText() - } + Text(context.viewState.displayText) + .font(.compound.bodyLGSemibold) + .foregroundColor(textColor) + .lineLimit(1) + .padding(.leading, 4) + .padding(.trailing, 6) + .padding(.vertical, 1) + .background { Capsule().foregroundColor(backgroundColor) } + .onChange(of: context.viewState.displayText) { _ in + didChangeText() + } } } diff --git a/ElementX/Sources/Screens/ComposerToolbar/CompletionSuggestionService.swift b/ElementX/Sources/Screens/ComposerToolbar/CompletionSuggestionService.swift index 604ae5f7a..8c67de8a5 100644 --- a/ElementX/Sources/Screens/ComposerToolbar/CompletionSuggestionService.swift +++ b/ElementX/Sources/Screens/ComposerToolbar/CompletionSuggestionService.swift @@ -18,6 +18,7 @@ import Combine import Foundation final class CompletionSuggestionService: CompletionSuggestionServiceProtocol { + private let roomProxy: RoomProxyProtocol private var canMentionAllUsers = false let areSuggestionsEnabled: Bool private(set) var suggestionsPublisher: AnyPublisher<[SuggestionItem], Never> = Empty().eraseToAnyPublisher() @@ -25,6 +26,7 @@ final class CompletionSuggestionService: CompletionSuggestionServiceProtocol { private let suggestionTriggerSubject = CurrentValueSubject(nil) init(roomProxy: RoomProxyProtocol, areSuggestionsEnabled: Bool) { + self.roomProxy = roomProxy self.areSuggestionsEnabled = areSuggestionsEnabled guard areSuggestionsEnabled else { return @@ -32,7 +34,8 @@ final class CompletionSuggestionService: CompletionSuggestionServiceProtocol { suggestionsPublisher = suggestionTriggerSubject .combineLatest(roomProxy.members) .map { [weak self] suggestionPattern, members -> [SuggestionItem] in - guard let suggestionPattern else { + guard let self, + let suggestionPattern else { return [] } @@ -47,10 +50,11 @@ final class CompletionSuggestionService: CompletionSuggestionServiceProtocol { } return SuggestionItem.user(item: .init(id: member.userID, displayName: member.displayName, avatarURL: member.avatarURL)) } - if self?.canMentionAllUsers == true, + if self.canMentionAllUsers, + !self.roomProxy.isEncryptedOneToOneRoom, Self.isIncluded(searchText: suggestionPattern.text, userID: PillConstants.atRoom, displayName: PillConstants.everyone) { membersSuggestion - .append(SuggestionItem.allUsers(item: .allUsersMention(roomAvatar: roomProxy.avatarURL))) + .insert(SuggestionItem.allUsers(item: .allUsersMention(roomAvatar: self.roomProxy.avatarURL)), at: 0) } return membersSuggestion } diff --git a/UnitTests/Sources/CompletionSuggestionServiceTests.swift b/UnitTests/Sources/CompletionSuggestionServiceTests.swift index 47920ee17..41bc58760 100644 --- a/UnitTests/Sources/CompletionSuggestionServiceTests.swift +++ b/UnitTests/Sources/CompletionSuggestionServiceTests.swift @@ -102,9 +102,9 @@ final class CompletionSuggestionServiceTests: XCTestCase { try await deferred.fulfill() deferred = deferFulfillment(service.suggestionsPublisher) { suggestions in - suggestions == [.user(item: .init(id: alice.userID, displayName: alice.displayName, avatarURL: alice.avatarURL)), - .user(item: .init(id: bob.userID, displayName: bob.displayName, avatarURL: bob.avatarURL)), - .allUsers(item: .allUsersMention(roomAvatar: nil))] + suggestions == [.allUsers(item: .allUsersMention(roomAvatar: nil)), + .user(item: .init(id: alice.userID, displayName: alice.displayName, avatarURL: alice.avatarURL)), + .user(item: .init(id: bob.userID, displayName: bob.displayName, avatarURL: bob.avatarURL))] } service.setSuggestionTrigger(.init(type: .user, text: "")) try await deferred.fulfill() diff --git a/UnitTests/Sources/PillContextTests.swift b/UnitTests/Sources/PillContextTests.swift index 08ec099e1..a9618c709 100644 --- a/UnitTests/Sources/PillContextTests.swift +++ b/UnitTests/Sources/PillContextTests.swift @@ -37,9 +37,6 @@ class PillContextTests: XCTestCase { let context = PillContext(roomContext: mock.context, data: PillTextAttachmentData(type: .user(userID: id), font: .preferredFont(forTextStyle: .body))) XCTAssertFalse(context.viewState.isOwnMention) - XCTAssertNil(context.viewState.avatarURL) - XCTAssertNil(context.viewState.name) - XCTAssertEqual(context.viewState.contentID, id) XCTAssertEqual(context.viewState.displayText, id) let name = "Mr. Test" @@ -48,10 +45,7 @@ class PillContextTests: XCTestCase { await Task.yield() XCTAssertFalse(context.viewState.isOwnMention) - XCTAssertEqual(context.viewState.avatarURL, avatarURL) - XCTAssertEqual(context.viewState.name, name) - XCTAssertEqual(context.viewState.contentID, id) - XCTAssertEqual(context.viewState.displayText, name) + XCTAssertEqual(context.viewState.displayText, "@\(name)") } func testOwnUser() async throws { @@ -90,9 +84,6 @@ class PillContextTests: XCTestCase { let context = PillContext(roomContext: mock.context, data: PillTextAttachmentData(type: .allUsers, font: .preferredFont(forTextStyle: .body))) XCTAssertTrue(context.viewState.isOwnMention) - XCTAssertEqual(context.viewState.avatarURL, avatarURL) - XCTAssertEqual(context.viewState.name, displayName) - XCTAssertEqual(context.viewState.contentID, id) - XCTAssertEqual(context.viewState.displayText, "@room") + XCTAssertEqual(context.viewState.displayText, PillConstants.atRoom) } } diff --git a/UnitTests/__Snapshots__/PreviewTests/test_messageText.Custom-Attachment.png b/UnitTests/__Snapshots__/PreviewTests/test_messageText.Custom-Attachment.png index 64777479d..8376e22de 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_messageText.Custom-Attachment.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_messageText.Custom-Attachment.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:1a15ca8e95ef692687c5b1eede5d0bd6f7fff14b544100928dd409cd6a48d66d -size 62925 +oid sha256:b832b8fc74a6a05083c6d493bb10ea1e50698523511918fefb4f09749276b9a0 +size 62612 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_pillView.All-Users.png b/UnitTests/__Snapshots__/PreviewTests/test_pillView.All-Users.png index fe8944ad2..f258fcb58 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_pillView.All-Users.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_pillView.All-Users.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:e28c9283ff63ad76753b6ad671d996034c07e453e44345cbac176dcf4b433c73 -size 61442 +oid sha256:cb1c8657a870af18a771b7a6b1be6c732ff2e9dfb4b69a558f453ce16d8711e7 +size 60309 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loaded-Long-Own.png b/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loaded-Long-Own.png index 418e7a8a9..001af364d 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loaded-Long-Own.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loaded-Long-Own.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:b78e02b2c47f049fa5e9c5802d76572696d497cd9a09cdb4a05edcb60b1da2df -size 66669 +oid sha256:838977457413677c95b62440aba60f39767b5c57c2dc8bc04db17604836961bf +size 66094 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loaded-Long.png b/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loaded-Long.png index fea437b13..2caef5543 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loaded-Long.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loaded-Long.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:bd550435f2618862e8338704605a98f0471aa49a23425ccfbfc1487ad339279c -size 66822 +oid sha256:b0a781a2db50c0e6b675fe4253421517bdb7c39584b0d810356e976a0dbf7e0e +size 66299 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loading-Own.png b/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loading-Own.png index a7b52a65f..9e7cbba60 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loading-Own.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loading-Own.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:9fd608f61a0055a49ad6b93a2db812c9a6fb60444a12699bffd78b05d61f3ee5 -size 63623 +oid sha256:dc295464d789704bef51b0613c6d62a6f4b2273d2468a8c985db64065317b42e +size 62830 diff --git a/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loading.png b/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loading.png index a47eba4bc..c93d8829d 100644 --- a/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loading.png +++ b/UnitTests/__Snapshots__/PreviewTests/test_pillView.Loading.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:ffe228e557bf1c029f5b07c6e187db1413cedae314747241c7c03cbcb291b240 -size 63702 +oid sha256:dd328f4947f5550ac584574b69eec3c2de4f06d90c9fefabe19a36b039ef0ec0 +size 62922