diff --git a/ElementX/Sources/Mocks/RoomSummaryProviderMock.swift b/ElementX/Sources/Mocks/RoomSummaryProviderMock.swift index 2531e9752..9dba4e86d 100644 --- a/ElementX/Sources/Mocks/RoomSummaryProviderMock.swift +++ b/ElementX/Sources/Mocks/RoomSummaryProviderMock.swift @@ -50,15 +50,22 @@ extension RoomSummaryProviderMock { setFilterClosure = { [initialRooms, roomListSubject] filter in switch filter { - case let .include(predicate): + case let .search(query): var rooms = initialRooms - if let filter = predicate.filters.first { - rooms = rooms.filter { filter == .people ? $0.isDirect : !$0.isDirect } + if !query.isEmpty { + rooms = rooms.filter { $0.name?.localizedCaseInsensitiveContains(query) ?? false } } - if let query = predicate.query, !query.isEmpty { - rooms = rooms.filter { $0.name?.localizedCaseInsensitiveContains(query) ?? false } + roomListSubject.send(rooms) + case let .all(filters): + var rooms = initialRooms + + if filters.count > 1 { + // for testing purpose chaining more than one filter will always return an empty state + rooms = [] + } else if let filter = filters.first { + rooms = rooms.filter { filter == .people ? $0.isDirect : !$0.isDirect } } roomListSubject.send(rooms) diff --git a/ElementX/Sources/Other/SwiftUI/Search.swift b/ElementX/Sources/Other/SwiftUI/Search.swift index 8a0a14563..50e3b6acb 100644 --- a/ElementX/Sources/Other/SwiftUI/Search.swift +++ b/ElementX/Sources/Other/SwiftUI/Search.swift @@ -17,6 +17,8 @@ import SwiftUI import SwiftUIIntrospect +// MARK: - Search Controller Extensions + extension View { /// A custom replacement for searchable that allows more precise configuration of the underlying search controller. /// @@ -169,3 +171,21 @@ private struct SearchController: UIViewControllerRepresentable { } } } + +// MARK: - Searchable Extensions + +struct IsSearchingModifier: ViewModifier { + @Environment(\.isSearching) private var isSearchingEnv + @Binding var isSearching: Bool + + func body(content: Content) -> some View { + content + .onChange(of: isSearchingEnv) { isSearching = $0 } + } +} + +extension View { + func isSearching(_ isSearching: Binding) -> some View { + modifier(IsSearchingModifier(isSearching: isSearching)) + } +} diff --git a/ElementX/Sources/Screens/EmojiPickerScreen/View/EmojiPickerScreen.swift b/ElementX/Sources/Screens/EmojiPickerScreen/View/EmojiPickerScreen.swift index 36f2526fb..c71ed16a3 100644 --- a/ElementX/Sources/Screens/EmojiPickerScreen/View/EmojiPickerScreen.swift +++ b/ElementX/Sources/Screens/EmojiPickerScreen/View/EmojiPickerScreen.swift @@ -58,7 +58,7 @@ struct EmojiPickerScreen: View { .navigationTitle(L10n.commonReactions) .navigationBarTitleDisplayMode(.inline) .toolbar { toolbar } - .modifier(IsSearching(isSearching: $isSearching)) + .isSearching($isSearching) .searchable(text: $searchString, placement: .navigationBarDrawer(displayMode: .always)) .compoundSearchField() } @@ -87,17 +87,6 @@ struct EmojiPickerScreen: View { } } -/// A view modifier to extract whether the search field is focussed from a subview. -private struct IsSearching: ViewModifier { - @Environment(\.isSearching) private var isSearchFieldFocused - @Binding var isSearching: Bool - - func body(content: Content) -> some View { - content - .onChange(of: isSearchFieldFocused) { isSearching = $0 } - } -} - // MARK: - Previews struct EmojiPickerScreen_Previews: PreviewProvider, TestablePreview { diff --git a/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift b/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift index d151a443d..2c12abcaf 100644 --- a/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift +++ b/ElementX/Sources/Screens/GlobalSearchScreen/GlobalSearchScreenViewModel.swift @@ -45,7 +45,7 @@ class GlobalSearchScreenViewModel: GlobalSearchScreenViewModelType, GlobalSearch .map(\.bindings.searchQuery) .removeDuplicates() .sink { [weak self] searchQuery in - self?.roomSummaryProvider.setFilter(.include(.init(query: searchQuery))) + self?.roomSummaryProvider.setFilter(.search(query: searchQuery)) } .store(in: &cancellables) @@ -60,7 +60,7 @@ class GlobalSearchScreenViewModel: GlobalSearchScreenViewModelType, GlobalSearch switch viewAction { case .dismiss: actionsSubject.send(.dismiss) - roomSummaryProvider.setFilter(.include(.all)) // This is a shared provider + roomSummaryProvider.setFilter(.all(filters: [])) // This is a shared provider case .select(let roomID): actionsSubject.send(.select(roomID: roomID)) case .reachedTop: diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift index c1204feed..76c127ccb 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift @@ -89,7 +89,7 @@ struct HomeScreenViewState: BindableState { var rooms: [HomeScreenRoom] = [] var roomListMode: HomeScreenRoomListMode = .skeletons - var shouldShowFilters = false + var areFiltersEnabled = false var markAsUnreadEnabled = false var markAsFavouriteEnabled = false @@ -122,6 +122,10 @@ struct HomeScreenViewState: BindableState { var shouldShowEmptyFilterState: Bool { shouldShowFilters && bindings.filtersState.isFiltering && visibleRooms.isEmpty } + + var shouldShowFilters: Bool { + areFiltersEnabled && !bindings.isSearchFieldFocused + } } struct HomeScreenViewStateBindings { diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift index efcbb023f..0fc3e17e2 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift @@ -106,10 +106,10 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol return } if !value { - state.shouldShowFilters = false + state.areFiltersEnabled = false state.bindings.filtersState.clearFilters() } else { - state.shouldShowFilters = true + state.areFiltersEnabled = true } } .store(in: &cancellables) @@ -233,10 +233,9 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol roomSummaryProvider?.setFilter(.excludeAll) } else { if state.bindings.isSearchFieldFocused { - roomSummaryProvider?.setFilter(.include(.init(query: state.bindings.searchQuery, - filters: state.bindings.filtersState.activeFilters.set))) + roomSummaryProvider?.setFilter(.search(query: state.bindings.searchQuery)) } else { - roomSummaryProvider?.setFilter(.include(.init(filters: state.bindings.filtersState.activeFilters.set))) + roomSummaryProvider?.setFilter(.all(filters: state.areFiltersEnabled ? state.bindings.filtersState.activeFilters.set : [])) } } } diff --git a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift index 9bbc1bf8b..83900ada1 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift @@ -37,21 +37,21 @@ struct RoomListFilterView_Previews: PreviewProvider, TestablePreview { private struct FilterToggleStyle: ToggleStyle { private func strokeColor(isOn: Bool) -> Color { - isOn ? .compound.bgSubtleSecondary : .compound.borderInteractiveSecondary + isOn ? .compound.bgActionPrimaryRest : .compound.borderInteractiveSecondary } private func backgroundColor(isOn: Bool) -> Color { - isOn ? .compound.bgSubtleSecondary : .compound.bgCanvasDefault + isOn ? .compound.bgActionPrimaryRest : .compound.bgCanvasDefault } private func foregroundColor(isOn: Bool) -> Color { - isOn ? .compound.textPrimary : .compound.textSecondary + isOn ? .compound.textOnSolidPrimary : .compound.textPrimary } func makeBody(configuration: Configuration) -> some View { let shape = RoundedRectangle(cornerRadius: 20) configuration.label - .font(.compound.bodyLGSemibold) + .font(.compound.bodyLG) .foregroundColor(foregroundColor(isOn: configuration.isOn)) .padding(.horizontal, 16) .padding(.vertical, 8) diff --git a/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift b/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift index 1bf6e5b73..799f82f24 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift @@ -53,7 +53,7 @@ struct HomeScreenContent: View { .layoutPriority(1) } case .rooms: - if context.viewState.shouldShowFilters { + if context.viewState.areFiltersEnabled { // Showing empty views in pinned headers makes the room list spasm when reaching the top LazyVStack(spacing: 0, pinnedViews: [.sectionHeaders]) { Section { @@ -68,6 +68,7 @@ struct HomeScreenContent: View { .readFrame($topSectionFrame) } } + .isSearching($context.isSearchFieldFocused) .searchable(text: $context.searchQuery) .compoundSearchField() .disableAutocorrection(true) @@ -76,6 +77,7 @@ struct HomeScreenContent: View { LazyVStack(spacing: 0) { HomeScreenRoomList(context: context) + .isSearching($context.isSearchFieldFocused) } .searchable(text: $context.searchQuery) .compoundSearchField() diff --git a/ElementX/Sources/Screens/HomeScreen/View/HomeScreenRoomList.swift b/ElementX/Sources/Screens/HomeScreen/View/HomeScreenRoomList.swift index 2d714d327..47ac6e2d1 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/HomeScreenRoomList.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/HomeScreenRoomList.swift @@ -17,17 +17,9 @@ import SwiftUI struct HomeScreenRoomList: View { - @Environment(\.isSearching) var isSearchFieldFocused - @ObservedObject var context: HomeScreenViewModel.Context var body: some View { - filteredContent - .onChange(of: isSearchFieldFocused) { context.isSearchFieldFocused = $0 } - } - - @ViewBuilder - private var filteredContent: some View { // Hide the room list when the search bar is focused but the query is empty // This works hand in hand with the room list service layer filtering and // avoids glitches when focusing the search bar diff --git a/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift b/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift index 32ec813b4..e3c06d72c 100644 --- a/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift +++ b/ElementX/Sources/Screens/MessageForwardingScreen/MessageForwardingScreenViewModel.swift @@ -49,7 +49,7 @@ class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, Me .removeDuplicates() .sink { [weak self] searchQuery in guard let self else { return } - self.roomSummaryProvider?.setFilter(.include(.init(query: searchQuery))) + self.roomSummaryProvider?.setFilter(.search(query: searchQuery)) } .store(in: &cancellables) @@ -60,7 +60,7 @@ class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, Me switch viewAction { case .cancel: actionsSubject.send(.dismiss) - roomSummaryProvider?.setFilter(.include(.all)) + roomSummaryProvider?.setFilter(.all(filters: [])) case .send: guard let roomID = state.selectedRoomID else { fatalError() diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift index c2eda371c..9df8f614f 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProvider.swift @@ -102,7 +102,7 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol { }) // Forces the listener above to be called with the current state - setFilter(.include(.all)) + setFilter(.all(filters: [])) listUpdatesTaskHandle = listUpdatesSubscriptionResult?.entriesStream @@ -153,12 +153,11 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol { switch filter { case .excludeAll: _ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .none) - case let .include(predicate): - var filters = predicate.filters.map(\.rustFilter) - if let query = predicate.query { - filters.append(.normalizedMatchRoomName(pattern: query.lowercased())) - } - // We never want to show left rooms. + case let .search(query): + let filters: [RoomListEntriesDynamicFilterKind] = [.normalizedMatchRoomName(pattern: query), .nonLeft] + _ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .all(filters: filters)) + case let .all(filters): + var filters = filters.map(\.rustFilter) filters.append(.nonLeft) _ = listUpdatesSubscriptionResult?.controller.setFilter(kind: .all(filters: filters)) } diff --git a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift index 1354d0ee1..8d233db7c 100644 --- a/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift +++ b/ElementX/Sources/Services/Room/RoomSummary/RoomSummaryProviderProtocol.swift @@ -97,27 +97,12 @@ enum RoomSummary: CustomStringConvertible, Equatable { } enum RoomSummaryProviderFilter: Equatable { - struct Predicate: Equatable { - let query: String? - let filters: Set - - static var all: Predicate { - Predicate() - } - - /// - Parameters: - /// - query: If provided the filter will do a normalized search, default is nil - /// - filters: Additional filters that can be provided for further filtering the room list, default is empty which means no additional filtering is done - init(query: String? = nil, filters: Set = []) { - self.query = query - self.filters = filters - } - } - /// Filters out everything case excludeAll /// Includes only the items that satisfy the predicate logic - case include(Predicate) + case search(query: String) + /// Includes only what satisfies the filters used + case all(filters: Set) } // sourcery: AutoMockable diff --git a/PreviewTests/__Snapshots__/PreviewTests/test_roomListFilterView.1.png b/PreviewTests/__Snapshots__/PreviewTests/test_roomListFilterView.1.png index f02294ffa..a860dd55a 100644 --- a/PreviewTests/__Snapshots__/PreviewTests/test_roomListFilterView.1.png +++ b/PreviewTests/__Snapshots__/PreviewTests/test_roomListFilterView.1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:28cde6aea3ba1a8dc8bf1d44359735c5cec7618ee51d4a087acd9ea0581c02bb -size 61950 +oid sha256:3ac210fff37bb6b8023fa1527ac1eeb5fdbad9c80357b42f68248f9940db3531 +size 62409 diff --git a/PreviewTests/__Snapshots__/PreviewTests/test_roomListFilterView.2.png b/PreviewTests/__Snapshots__/PreviewTests/test_roomListFilterView.2.png index d527bbb20..443cc1ac2 100644 --- a/PreviewTests/__Snapshots__/PreviewTests/test_roomListFilterView.2.png +++ b/PreviewTests/__Snapshots__/PreviewTests/test_roomListFilterView.2.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:07160b58b8cc326f373622079b8513322aa27c1ef44fe95c60f41ef87d747ea1 -size 60192 +oid sha256:01003a81f1ace9c184c28728f8bee3528e326f0be88ebcbab3006af649208a10 +size 60619 diff --git a/PreviewTests/__Snapshots__/PreviewTests/test_roomListFiltersView.1.png b/PreviewTests/__Snapshots__/PreviewTests/test_roomListFiltersView.1.png index 75344fe58..be2eec393 100644 --- a/PreviewTests/__Snapshots__/PreviewTests/test_roomListFiltersView.1.png +++ b/PreviewTests/__Snapshots__/PreviewTests/test_roomListFiltersView.1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:25da13ca12e10bb1a8634fb62b9073ec02aecbfe02872f81bdd49d619a9b1ca6 -size 69664 +oid sha256:a5972aae36b27aad2dd6c2fdddcdc99a369d566f162042b1916f77d857607134 +size 70253 diff --git a/PreviewTests/__Snapshots__/PreviewTests/test_roomListFiltersView.2.png b/PreviewTests/__Snapshots__/PreviewTests/test_roomListFiltersView.2.png index e5f8d1921..661455d5e 100644 --- a/PreviewTests/__Snapshots__/PreviewTests/test_roomListFiltersView.2.png +++ b/PreviewTests/__Snapshots__/PreviewTests/test_roomListFiltersView.2.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:2af52fe41d2238cd6e16f759d533ad21445569270b583a2e268a7926d68e37e0 -size 73653 +oid sha256:269445bf0caa7e3ca562e542028081fb5bf9c87d42b5509cce7f69dda472f46e +size 73468 diff --git a/UnitTests/Sources/HomeScreenViewModelTests.swift b/UnitTests/Sources/HomeScreenViewModelTests.swift index 84707e850..1dfaf37d0 100644 --- a/UnitTests/Sources/HomeScreenViewModelTests.swift +++ b/UnitTests/Sources/HomeScreenViewModelTests.swift @@ -171,11 +171,23 @@ class HomeScreenViewModelTests: XCTestCase { try await Task.sleep(for: .milliseconds(100)) XCTAssertEqual(roomSummaryProvider.roomListPublisher.value.count, 2) XCTAssertEqual(roomSummaryProvider.roomListPublisher.value.first?.name, "Foundation and Earth") - + } + + func testSearch() async throws { context.isSearchFieldFocused = true context.searchQuery = "lude to Found" try await Task.sleep(for: .milliseconds(100)) XCTAssertEqual(roomSummaryProvider.roomListPublisher.value.first?.name, "Prelude to Foundation") XCTAssertEqual(roomSummaryProvider.roomListPublisher.value.count, 1) + XCTAssertFalse(context.viewState.shouldShowFilters) + } + + func testFiltersEmptyState() async throws { + context.filtersState.activateFilter(.people) + context.filtersState.activateFilter(.favourites) + try await Task.sleep(for: .milliseconds(100)) + XCTAssertTrue(context.viewState.shouldShowEmptyFilterState) + context.isSearchFieldFocused = true + XCTAssertFalse(context.viewState.shouldShowEmptyFilterState) } } diff --git a/changelog.d/pr-2530.wip b/changelog.d/pr-2530.wip new file mode 100644 index 000000000..aadcccf51 --- /dev/null +++ b/changelog.d/pr-2530.wip @@ -0,0 +1 @@ +Searching hides and ignores filtering now. \ No newline at end of file