From e1decf19c0fc8e4a06d363fd5e7f2418c82a54ff Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Tue, 13 Feb 2024 02:09:55 +0100 Subject: [PATCH] Fix filters being jumpy when scrolling them (#2459) --- .../Screens/HomeScreen/HomeScreenModels.swift | 3 +- .../HomeScreen/HomeScreenViewModel.swift | 10 +++--- .../View/Filters/RoomListFilterModels.swift | 10 +++--- .../View/Filters/RoomListFilterView.swift | 13 +++---- .../View/Filters/RoomListFiltersView.swift | 36 ++++++++++++------- .../HomeScreen/View/HomeScreenContent.swift | 2 +- .../Sources/HomeScreenViewModelTests.swift | 2 +- 7 files changed, 40 insertions(+), 36 deletions(-) diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift index 9b0a4b54e..8f311cb96 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift @@ -121,8 +121,6 @@ struct HomeScreenViewState: BindableState { return rooms } - var filtersState = RoomListFiltersState() - var bindings = HomeScreenViewStateBindings() var placeholderRooms: [HomeScreenRoom] { @@ -138,6 +136,7 @@ struct HomeScreenViewState: BindableState { } struct HomeScreenViewStateBindings { + var filtersState = RoomListFiltersState() var searchQuery = "" var isSearchFieldFocused = false diff --git a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift index 7142cd352..b67ea398c 100644 --- a/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift +++ b/ElementX/Sources/Screens/HomeScreen/HomeScreenViewModel.swift @@ -89,7 +89,7 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol } if !value { state.shouldShowFilters = false - state.filtersState.clearFilters() + state.bindings.filtersState.clearFilters() } else { state.shouldShowFilters = true } @@ -106,9 +106,9 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol let isSearchFieldFocused = context.$viewState.map(\.bindings.isSearchFieldFocused) let searchQuery = context.$viewState.map(\.bindings.searchQuery) - let enabledFilters = context.viewState.filtersState.$activeFilters + let activeFilters = context.$viewState.map(\.bindings.filtersState.activeFilters) isSearchFieldFocused - .combineLatest(searchQuery, enabledFilters) + .combineLatest(searchQuery, activeFilters) .removeDuplicates { $0 == $1 } .sink { [weak self] isSearchFieldFocused, _, _ in guard let self else { return } @@ -209,9 +209,9 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol } else { if state.bindings.isSearchFieldFocused { roomSummaryProvider?.setFilter(.include(.init(query: state.bindings.searchQuery, - filters: state.filtersState.activeFilters))) + filters: state.bindings.filtersState.activeFilters))) } else { - roomSummaryProvider?.setFilter(.include(.init(filters: state.filtersState.activeFilters))) + roomSummaryProvider?.setFilter(.include(.init(filters: state.bindings.filtersState.activeFilters))) } } } diff --git a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterModels.swift b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterModels.swift index cd52a7e22..ee7f5920c 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterModels.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterModels.swift @@ -71,8 +71,8 @@ enum RoomListFilter: Int, CaseIterable, Identifiable { } } -final class RoomListFiltersState: ObservableObject { - @Published private(set) var activeFilters: Set +struct RoomListFiltersState { + private(set) var activeFilters: Set init(activeFilters: Set = []) { self.activeFilters = activeFilters @@ -97,7 +97,7 @@ final class RoomListFiltersState: ObservableObject { !activeFilters.isEmpty } - func activateFilter(_ filter: RoomListFilter) { + mutating func activateFilter(_ filter: RoomListFilter) { if let incompatibleFilter = filter.incompatibleFilter, activeFilters.contains(incompatibleFilter) { fatalError("[RoomListFiltersState] adding mutually exclusive filters is not allowed") @@ -105,11 +105,11 @@ final class RoomListFiltersState: ObservableObject { activeFilters.insert(filter) } - func deactivateFilter(_ filter: RoomListFilter) { + mutating func deactivateFilter(_ filter: RoomListFilter) { activeFilters.remove(filter) } - func clearFilters() { + mutating func clearFilters() { activeFilters.removeAll() } diff --git a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift index 41de9681a..83900ada1 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFilterView.swift @@ -18,15 +18,10 @@ import SwiftUI struct RoomListFilterView: View { let filter: RoomListFilter - @StateObject var state: RoomListFiltersState + @Binding var isActive: Bool var body: some View { - let binding = Binding(get: { - state.isFilterActive(filter) - }, set: { isEnabled, _ in - isEnabled ? state.activateFilter(filter) : state.deactivateFilter(filter) - }) - Toggle(isOn: binding) { + Toggle(isOn: $isActive) { Text(filter.localizedName) } .toggleStyle(FilterToggleStyle()) @@ -35,8 +30,8 @@ struct RoomListFilterView: View { struct RoomListFilterView_Previews: PreviewProvider, TestablePreview { static var previews: some View { - RoomListFilterView(filter: .people, state: .init()) - RoomListFilterView(filter: .people, state: .init(activeFilters: [.people])) + RoomListFilterView(filter: .people, isActive: .constant(false)) + RoomListFilterView(filter: .people, isActive: .constant(true)) } } diff --git a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFiltersView.swift b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFiltersView.swift index f46f13030..ee60e6a05 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFiltersView.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/Filters/RoomListFiltersView.swift @@ -17,28 +17,30 @@ import SwiftUI struct RoomListFiltersView: View { - @StateObject var state: RoomListFiltersState + @Binding var state: RoomListFiltersState + @Namespace private var namespace var body: some View { ScrollView(.horizontal) { - LazyHStack(spacing: 8) { + HStack(spacing: 8) { if state.isFiltering { clearButton - } else { - // This solves a weird issue withe the LazyHStack - // where it is resized when the button appears and disappears - clearButton - .hidden() - .frame(width: 0) } + ForEach(state.sortedActiveFilters) { filter in - RoomListFilterView(filter: filter, state: state) + RoomListFilterView(filter: filter, + isActive: getBinding(for: filter)) + .matchedGeometryEffect(id: filter.id, in: namespace) + // This will make the animation always render the enabled ones on top + .zIndex(1) } ForEach(state.availableFilters) { filter in - RoomListFilterView(filter: filter, state: state) + RoomListFilterView(filter: filter, + isActive: getBinding(for: filter)) + .matchedGeometryEffect(id: filter.id, in: namespace) } } - .padding(.leading, !state.isFiltering ? 8 : 16) + .padding(.leading, 16) .padding(.vertical, 12) } .scrollIndicators(.hidden) @@ -55,13 +57,21 @@ struct RoomListFiltersView: View { .foregroundColor(.compound.bgActionPrimaryRest) }) } + + private func getBinding(for filter: RoomListFilter) -> Binding { + Binding(get: { + state.isFilterActive(filter) + }, set: { isEnabled, _ in + isEnabled ? state.activateFilter(filter) : state.deactivateFilter(filter) + }) + } } // MARK: - Previews struct RoomListFiltersView_Previews: PreviewProvider, TestablePreview { static var previews: some View { - RoomListFiltersView(state: .init()) - RoomListFiltersView(state: .init(activeFilters: [.rooms, .favourites])) + RoomListFiltersView(state: .constant(.init())) + RoomListFiltersView(state: .constant(.init(activeFilters: [.rooms, .favourites]))) } } diff --git a/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift b/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift index f7b156236..f02689c97 100644 --- a/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift +++ b/ElementX/Sources/Screens/HomeScreen/View/HomeScreenContent.swift @@ -134,7 +134,7 @@ struct HomeScreenContent: View { } private var filters: some View { - RoomListFiltersView(state: context.viewState.filtersState) + RoomListFiltersView(state: $context.filtersState) } @ViewBuilder diff --git a/UnitTests/Sources/HomeScreenViewModelTests.swift b/UnitTests/Sources/HomeScreenViewModelTests.swift index fa180c3ff..b729bab8c 100644 --- a/UnitTests/Sources/HomeScreenViewModelTests.swift +++ b/UnitTests/Sources/HomeScreenViewModelTests.swift @@ -162,7 +162,7 @@ class HomeScreenViewModelTests: XCTestCase { } func testFilters() async throws { - context.viewState.filtersState.activateFilter(.people) + context.filtersState.activateFilter(.people) try await Task.sleep(for: .milliseconds(100)) XCTAssertEqual(roomSummaryProvider.currentFilter, RoomSummaryProviderFilter.include(.init(filters: [.people]))) context.isSearchFieldFocused = true