From 4c84878f88860fde93611962bc91a68b057e02f3 Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Mon, 23 Oct 2023 17:52:10 +0200 Subject: [PATCH] Fix Suggestion Pattern when text is empty (#1943) * fix + debounce improvement * comment * improved existing function to be more generic --- .../Sources/Other/Extensions/Publisher.swift | 19 +++++++++++----- .../CompletionSuggestionService.swift | 10 +++++++-- .../InviteUsersScreenViewModel.swift | 2 +- .../StartChatScreenViewModel.swift | 2 +- .../CompletionSuggestionServiceTests.swift | 22 +++++++++++++++++++ 5 files changed, 45 insertions(+), 10 deletions(-) diff --git a/ElementX/Sources/Other/Extensions/Publisher.swift b/ElementX/Sources/Other/Extensions/Publisher.swift index ba97bfc2e9..eb89c959a7 100644 --- a/ElementX/Sources/Other/Extensions/Publisher.swift +++ b/ElementX/Sources/Other/Extensions/Publisher.swift @@ -25,16 +25,23 @@ extension Publisher where Self.Failure == Never { } } -extension Publisher where Output == String, Failure == Never { - /// Debounce text queries and remove duplicates. - /// Clearing the text publishes the update immediately. - func debounceAndRemoveDuplicates() -> AnyPublisher { +extension Publisher where Output: Equatable, Failure == Never { + func debounceAndRemoveDuplicates(on scheduler: S, delay: @escaping (Output) -> S.SchedulerTimeType.Stride) -> AnyPublisher { map { query in - let milliseconds = query.isEmpty ? 0 : 250 - return Just(query).delay(for: .milliseconds(milliseconds), scheduler: DispatchQueue.main) + Just(query).delay(for: delay(query), scheduler: scheduler) } .switchToLatest() .removeDuplicates() .eraseToAnyPublisher() } } + +extension Publisher where Output == String, Failure == Never { + /// Debounce text queries and remove duplicates. + /// Clearing the text publishes the update immediately. + func debounceTextQueriesAndRemoveDuplicates() -> AnyPublisher { + debounceAndRemoveDuplicates(on: DispatchQueue.main) { query in + query.isEmpty ? .milliseconds(0) : .milliseconds(250) + } + } +} diff --git a/ElementX/Sources/Screens/ComposerToolbar/CompletionSuggestionService.swift b/ElementX/Sources/Screens/ComposerToolbar/CompletionSuggestionService.swift index 146ac978c1..604ae5f7a2 100644 --- a/ElementX/Sources/Screens/ComposerToolbar/CompletionSuggestionService.swift +++ b/ElementX/Sources/Screens/ComposerToolbar/CompletionSuggestionService.swift @@ -55,8 +55,10 @@ final class CompletionSuggestionService: CompletionSuggestionServiceProtocol { return membersSuggestion } } - .debounce(for: 0.5, scheduler: DispatchQueue.main) - .eraseToAnyPublisher() + // We only debounce if the suggestion is nil + .debounceAndRemoveDuplicates(on: DispatchQueue.main) { [weak self] _ in + self?.suggestionTriggerSubject.value != nil ? .milliseconds(500) : .milliseconds(0) + } Task { switch await roomProxy.canUserTriggerRoomNotification(userID: roomProxy.ownUserID) { @@ -73,6 +75,10 @@ final class CompletionSuggestionService: CompletionSuggestionServiceProtocol { } private static func isIncluded(searchText: String, userID: String, displayName: String?) -> Bool { + // If the search text is empty give back all the results + guard !searchText.isEmpty else { + return true + } let containedInUserID = userID.localizedStandardContains(searchText.lowercased()) let containedInDisplayName: Bool diff --git a/ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenViewModel.swift b/ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenViewModel.swift index 97cdbc3a95..f386f3f932 100644 --- a/ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenViewModel.swift +++ b/ElementX/Sources/Screens/InviteUsersScreen/InviteUsersScreenViewModel.swift @@ -95,7 +95,7 @@ class InviteUsersScreenViewModel: InviteUsersScreenViewModelType, InviteUsersScr private func setupSubscriptions(selectedUsers: CurrentValuePublisher<[UserProfileProxy], Never>) { context.$viewState .map(\.bindings.searchQuery) - .debounceAndRemoveDuplicates() + .debounceTextQueriesAndRemoveDuplicates() .sink { [weak self] _ in self?.fetchUsers() } diff --git a/ElementX/Sources/Screens/StartChatScreen/StartChatScreenViewModel.swift b/ElementX/Sources/Screens/StartChatScreen/StartChatScreenViewModel.swift index 9806639bbd..06ece16e6d 100644 --- a/ElementX/Sources/Screens/StartChatScreen/StartChatScreenViewModel.swift +++ b/ElementX/Sources/Screens/StartChatScreen/StartChatScreenViewModel.swift @@ -92,7 +92,7 @@ class StartChatScreenViewModel: StartChatScreenViewModelType, StartChatScreenVie private func setupBindings() { context.$viewState .map(\.bindings.searchQuery) - .debounceAndRemoveDuplicates() + .debounceTextQueriesAndRemoveDuplicates() .sink { [weak self] _ in self?.fetchUsers() } diff --git a/UnitTests/Sources/CompletionSuggestionServiceTests.swift b/UnitTests/Sources/CompletionSuggestionServiceTests.swift index 44656951b7..47920ee172 100644 --- a/UnitTests/Sources/CompletionSuggestionServiceTests.swift +++ b/UnitTests/Sources/CompletionSuggestionServiceTests.swift @@ -87,4 +87,26 @@ final class CompletionSuggestionServiceTests: XCTestCase { service.setSuggestionTrigger(.init(type: .user, text: "every")) try await deferred.fulfill() } + + func testUserSuggestionsWithEmptyText() async throws { + let alice: RoomMemberProxyMock = .mockAlice + let bob: RoomMemberProxyMock = .mockBob + let members: [RoomMemberProxyMock] = [alice, bob, .mockMe] + let roomProxyMock = RoomProxyMock(with: .init(displayName: "test", members: members, canUserTriggerRoomNotification: true)) + let service = CompletionSuggestionService(roomProxy: roomProxyMock, areSuggestionsEnabled: true) + + var deferred = deferFulfillment(service.suggestionsPublisher) { suggestions in + suggestions == [] + } + + 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))] + } + service.setSuggestionTrigger(.init(type: .user, text: "")) + try await deferred.fulfill() + } }