Skip to content

Add a Low Priority room filter behind a feature flag. #4394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ElementX.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,7 @@
6A54F52443EC52AC5CD772C0 /* JoinRoomScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = 869A8A4632E511351BFE2EC4 /* JoinRoomScreen.swift */; };
6A64546ABE648ED9E6DBB459 /* RemoteSettingsHook.swift in Sources */ = {isa = PBXBuildFile; fileRef = D5D186A6DB8FAC5C9D0E4D61 /* RemoteSettingsHook.swift */; };
6A916606A8B92FE8A990A219 /* XCTestCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 85A1941B874A3BE9CDDF43EF /* XCTestCase.swift */; };
6AB306367E56A6F6DFA0E2FF /* RoomSummaryProviderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F46E441BA50705E6CEC89FE0 /* RoomSummaryProviderTests.swift */; };
6AD722DD92E465E56D2885AB /* BugReportScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = BA919F521E9F0EE3638AFC85 /* BugReportScreen.swift */; };
6AEB650311F694A5702255C9 /* UserProfileScreenCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = D5B4932E4EFBC8FAC10972CD /* UserProfileScreenCoordinator.swift */; };
6B31508C6334C617360C2EAB /* RoomMemberDetailsViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EC589E641AE46EFB2962534D /* RoomMemberDetailsViewModelTests.swift */; };
Expand Down Expand Up @@ -2679,6 +2680,7 @@
F409D44C2E6BE50462E82F8A /* en-US */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "en-US"; path = "en-US.lproj/Localizable.strings"; sourceTree = "<group>"; };
F4469F6AE311BDC439B3A5EC /* UserSessionMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserSessionMock.swift; sourceTree = "<group>"; };
F4548A9BDE5CB3AB864BCA9F /* EffectsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EffectsView.swift; sourceTree = "<group>"; };
F46E441BA50705E6CEC89FE0 /* RoomSummaryProviderTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomSummaryProviderTests.swift; sourceTree = "<group>"; };
F4CEB4590CCF70F0E3C0B171 /* GeneratedAccessibilityTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GeneratedAccessibilityTests.swift; sourceTree = "<group>"; };
F506C6ADB1E1DA6638078E11 /* UITests.xctest */ = {isa = PBXFileReference; includeInIndex = 0; lastKnownFileType = wrapper.cfbundle; path = UITests.xctest; sourceTree = BUILT_PRODUCTS_DIR; };
F51D674A5B5F1FE1B878E20F /* nb */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = nb; path = nb.lproj/Localizable.strings; sourceTree = "<group>"; };
Expand Down Expand Up @@ -4498,6 +4500,7 @@
48FEFF746DB341CDB18D7AAA /* RoomRolesAndPermissionsScreenViewModelTests.swift */,
93CF7B19FFCF8EFBE0A8696A /* RoomScreenViewModelTests.swift */,
AEEAFB646E583655652C3D04 /* RoomStateEventStringBuilderTests.swift */,
F46E441BA50705E6CEC89FE0 /* RoomSummaryProviderTests.swift */,
046C0D3F53B0B5EF0A1F5BEA /* RoomSummaryTests.swift */,
2E88534A39781D76487D59DF /* SecureBackupKeyBackupScreenViewModelTests.swift */,
848F69921527D31CAACB93AF /* SecureBackupLogoutConfirmationScreenViewModelTests.swift */,
Expand Down Expand Up @@ -7168,6 +7171,7 @@
84C631E734FD2555B39B681C /* RoomRolesAndPermissionsScreenViewModelTests.swift in Sources */,
46562110EE202E580A5FFD9C /* RoomScreenViewModelTests.swift in Sources */,
CC0D088F505F33A20DC5590F /* RoomStateEventStringBuilderTests.swift in Sources */,
6AB306367E56A6F6DFA0E2FF /* RoomSummaryProviderTests.swift in Sources */,
15913A5B07118C1268A840E4 /* RoomSummaryTests.swift in Sources */,
7691233E3572A9173FD96CB3 /* SecureBackupKeyBackupScreenViewModelTests.swift in Sources */,
EB87DF90CF6F8D5D12404C6E /* SecureBackupLogoutConfirmationScreenViewModelTests.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,7 @@
"screen_roomlist_filter_favourites_empty_state_title" = "You don’t have favourite chats yet";
"screen_roomlist_filter_invites_empty_state_title" = "You don't have any pending invites.";
"screen_roomlist_filter_low_priority" = "Low Priority";
"screen_roomlist_filter_low_priority_empty_state_title" = "You don’t have any low priority chats yet";
"screen_roomlist_filter_mixed_empty_state_subtitle" = "You can deselect filters in order to see your other chats";
"screen_roomlist_filter_mixed_empty_state_title" = "You don’t have chats for this selection";
"screen_roomlist_filter_people_empty_state_title" = "You don’t have any DMs yet";
Expand Down
4 changes: 4 additions & 0 deletions ElementX/Sources/Application/Settings/AppSettings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ final class AppSettings {
// Feature flags
case publicSearchEnabled
case fuzzyRoomListSearchEnabled
case lowPriorityFilterEnabled
case enableOnlySignedDeviceIsolationMode
case enableKeyShareOnInvite
case knockingEnabled
Expand Down Expand Up @@ -345,6 +346,9 @@ final class AppSettings {
@UserPreference(key: UserDefaultsKeys.fuzzyRoomListSearchEnabled, defaultValue: false, storageType: .userDefaults(store))
var fuzzyRoomListSearchEnabled

@UserPreference(key: UserDefaultsKeys.lowPriorityFilterEnabled, defaultValue: false, storageType: .userDefaults(store))
var lowPriorityFilterEnabled

@UserPreference(key: UserDefaultsKeys.knockingEnabled, defaultValue: false, storageType: .userDefaults(store))
var knockingEnabled

Expand Down
2 changes: 2 additions & 0 deletions ElementX/Sources/Generated/Strings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2581,6 +2581,8 @@ internal enum L10n {
internal static var screenRoomlistFilterInvitesEmptyStateTitle: String { return L10n.tr("Localizable", "screen_roomlist_filter_invites_empty_state_title") }
/// Low Priority
internal static var screenRoomlistFilterLowPriority: String { return L10n.tr("Localizable", "screen_roomlist_filter_low_priority") }
/// You don’t have any low priority chats yet
internal static var screenRoomlistFilterLowPriorityEmptyStateTitle: String { return L10n.tr("Localizable", "screen_roomlist_filter_low_priority_empty_state_title") }
/// You can deselect filters in order to see your other chats
internal static var screenRoomlistFilterMixedEmptyStateSubtitle: String { return L10n.tr("Localizable", "screen_roomlist_filter_mixed_empty_state_subtitle") }
/// You don’t have chats for this selection
Expand Down
4 changes: 2 additions & 2 deletions ElementX/Sources/Screens/HomeScreen/HomeScreenModels.swift
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ struct HomeScreenViewState: BindableState {
return rooms
}

var bindings = HomeScreenViewStateBindings()
var bindings: HomeScreenViewStateBindings

var placeholderRooms: [HomeScreenRoom] {
(1...10).map { _ in
Expand All @@ -136,7 +136,7 @@ struct HomeScreenViewState: BindableState {
}

struct HomeScreenViewStateBindings {
var filtersState = RoomListFiltersState()
var filtersState: RoomListFiltersState
var searchQuery = ""
var isSearchFieldFocused = false

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol

roomSummaryProvider = userSession.clientProxy.roomSummaryProvider

super.init(initialViewState: .init(userID: userSession.clientProxy.userID),
super.init(initialViewState: .init(userID: userSession.clientProxy.userID,
bindings: .init(filtersState: .init(appSettings: appSettings))),
mediaProvider: userSession.mediaProvider)

userSession.clientProxy.userAvatarURLPublisher
Expand Down Expand Up @@ -232,6 +233,8 @@ class HomeScreenViewModel: HomeScreenViewModelType, HomeScreenViewModelProtocol
if state.bindings.isSearchFieldFocused {
roomSummaryProvider?.setFilter(.search(query: state.bindings.searchQuery))
} else {
// Apply active filters - the low priority exclusion logic will be handled
// at the SDK level when the low priority filter is implemented
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment feels unnecessary, perhaps we should name the app side filter better e.g. onlyLowPriority

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lol oh yeah that one is very random, removing it.

I get what you're saying about the name, but technically they're all only, e.g. onlyFavourites, onlyRooms etc so I'm not sure how much that would help. I think it's clear from the comment on .lowPriority's rustFilter value.

roomSummaryProvider?.setFilter(.all(filters: state.bindings.filtersState.activeFilters.set))
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ enum RoomListFilter: Int, CaseIterable, Identifiable {
case rooms
case favourites
case invites
case lowPriority

static var availableFilters: [RoomListFilter] {
RoomListFilter.allCases
Expand All @@ -38,6 +39,8 @@ enum RoomListFilter: Int, CaseIterable, Identifiable {
return L10n.screenRoomlistFilterFavourites
case .invites:
return L10n.screenRoomlistFilterInvites
case .lowPriority:
return L10n.screenRoomlistFilterLowPriority
}
}

Expand All @@ -50,10 +53,11 @@ enum RoomListFilter: Int, CaseIterable, Identifiable {
case .unreads:
return [.invites]
case .favourites:
// When we will have Low Priority we may need to return it here
return [.invites]
return [.invites, .lowPriority]
case .invites:
return [.rooms, .people, .unreads, .favourites]
return [.rooms, .people, .unreads, .favourites, .lowPriority]
case .lowPriority:
return [.favourites, .invites]
}
}

Expand All @@ -69,20 +73,29 @@ enum RoomListFilter: Int, CaseIterable, Identifiable {
return .all(filters: [.favourite, .joined])
case .invites:
return .invite
case .lowPriority:
// Note: When not activated, the setFilter method automatically applies the .nonLowPriority filter.
return .all(filters: [.lowPriority, .joined])
}
}
}

struct RoomListFiltersState {
private(set) var activeFilters: OrderedSet<RoomListFilter>
private let appSettings: AppSettings

init(activeFilters: OrderedSet<RoomListFilter> = []) {
init(activeFilters: OrderedSet<RoomListFilter> = [], appSettings: AppSettings) {
self.activeFilters = .init(activeFilters)
self.appSettings = appSettings
}

var availableFilters: [RoomListFilter] {
var availableFilters = OrderedSet(RoomListFilter.availableFilters)

if !appSettings.lowPriorityFilterEnabled {
availableFilters.remove(.lowPriority)
}

for filter in activeFilters {
availableFilters.remove(filter)
filter.incompatibleFilters.forEach { availableFilters.remove($0) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,6 @@ struct RoomListFilterView: View {
}
}

struct RoomListFilterView_Previews: PreviewProvider, TestablePreview {
static var previews: some View {
RoomListFilterView(filter: .people, isActive: .constant(false))
RoomListFilterView(filter: .people, isActive: .constant(true))
}
}

private struct FilterToggleStyle: ToggleStyle {
private func strokeColor(isOn: Bool) -> Color {
isOn ? .compound.bgActionPrimaryRest : .compound.borderInteractiveSecondary
Expand Down Expand Up @@ -59,3 +52,12 @@ private struct FilterToggleStyle: ToggleStyle {
}
}
}

// MARK: - Previews

struct RoomListFilterView_Previews: PreviewProvider, TestablePreview {
static var previews: some View {
RoomListFilterView(filter: .people, isActive: .constant(false))
RoomListFilterView(filter: .people, isActive: .constant(true))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ struct RoomListFiltersEmptyStateView: View {
return L10n.screenRoomlistFilterFavouritesEmptyStateTitle
case .invites:
return L10n.screenRoomlistFilterInvitesEmptyStateTitle
case .lowPriority:
return L10n.screenRoomlistFilterLowPriorityEmptyStateTitle
}
}
return L10n.screenRoomlistFilterMixedEmptyStateTitle
Expand Down Expand Up @@ -55,9 +57,11 @@ struct RoomListFiltersEmptyStateView_Previews: PreviewProvider, TestablePreview
static var previews: some View {
VStack(spacing: 24) {
ForEach(RoomListFilter.allCases) { filter in
RoomListFiltersEmptyStateView(state: .init(activeFilters: [filter]))
RoomListFiltersEmptyStateView(state: .init(activeFilters: [filter],
appSettings: ServiceLocator.shared.settings))
}
RoomListFiltersEmptyStateView(state: .init(activeFilters: [.people, .favourites]))
RoomListFiltersEmptyStateView(state: .init(activeFilters: [.people, .favourites],
appSettings: ServiceLocator.shared.settings))
}
.padding(.bottom)
.previewLayout(.sizeThatFits)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@ struct RoomListFiltersView: View {

struct RoomListFiltersView_Previews: PreviewProvider, TestablePreview {
static var previews: some View {
RoomListFiltersView(state: .constant(.init()))
RoomListFiltersView(state: .constant(.init(activeFilters: [.rooms, .favourites])))
RoomListFiltersView(state: .constant(.init(appSettings: ServiceLocator.shared.settings)))
RoomListFiltersView(state: .constant(.init(activeFilters: [.rooms, .favourites],
appSettings: ServiceLocator.shared.settings)))
RoomListFiltersView(state: .constant(.init(activeFilters: [.lowPriority],
appSettings: ServiceLocator.shared.settings)))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ protocol DeveloperOptionsProtocol: AnyObject {

var publicSearchEnabled: Bool { get set }
var fuzzyRoomListSearchEnabled: Bool { get set }
var lowPriorityFilterEnabled: Bool { get set }
var knockingEnabled: Bool { get set }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ struct DeveloperOptionsScreen: View {
Toggle(isOn: $context.fuzzyRoomListSearchEnabled) {
Text("Fuzzy searching")
}

Toggle(isOn: $context.lowPriorityFilterEnabled) {
Text("Low priority filter")
}
}

Section("Timeline") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,14 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
}
_ = listUpdatesSubscriptionResult?.controller().setFilter(kind: .all(filters: filters))
case let .all(filters):
var filters = filters.map(\.rustFilter)
filters.append(contentsOf: [.nonLeft, .nonSpace, .deduplicateVersions])
_ = listUpdatesSubscriptionResult?.controller().setFilter(kind: .all(filters: filters))
var rustFilters = filters.map(\.rustFilter)
rustFilters.append(contentsOf: [.nonLeft, .nonSpace, .deduplicateVersions])

if !filters.contains(.lowPriority), appSettings.lowPriorityFilterEnabled {
rustFilters.append(.all(filters: [.nonLowPriority, .joined]))
}

_ = listUpdatesSubscriptionResult?.controller().setFilter(kind: .all(filters: rustFilters))
}
}

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
42 changes: 36 additions & 6 deletions UnitTests/Sources/RoomListFiltersStateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,25 @@ import XCTest
@testable import ElementX

final class RoomListFiltersStateTests: XCTestCase {
var appSettings: AppSettings!

var state: RoomListFiltersState!
var allCasesWithoutLowPriority = RoomListFilter.allCases.filter { $0 != .lowPriority }

override func setUp() {
state = RoomListFiltersState()
AppSettings.resetAllSettings()
appSettings = AppSettings()
state = RoomListFiltersState(appSettings: appSettings)
}

override func tearDown() {
AppSettings.resetAllSettings()
}

func testInitialState() {
XCTAssertFalse(state.isFiltering)
XCTAssertEqual(state.activeFilters, [])
XCTAssertEqual(state.availableFilters, RoomListFilter.allCases)
XCTAssertEqual(state.availableFilters, allCasesWithoutLowPriority)
}

func testSetAndUnsetFilters() {
Expand All @@ -30,7 +39,7 @@ final class RoomListFiltersStateTests: XCTestCase {
state.deactivateFilter(.unreads)
XCTAssertFalse(state.isFiltering)
XCTAssertEqual(state.activeFilters, [])
XCTAssertEqual(state.availableFilters, RoomListFilter.allCases)
XCTAssertEqual(state.availableFilters, allCasesWithoutLowPriority)
}

func testMutuallyExclusiveFilters() {
Expand All @@ -42,7 +51,7 @@ final class RoomListFiltersStateTests: XCTestCase {
state.deactivateFilter(.people)
XCTAssertFalse(state.isFiltering)
XCTAssertEqual(state.activeFilters, [])
XCTAssertEqual(state.availableFilters, RoomListFilter.allCases)
XCTAssertEqual(state.availableFilters, allCasesWithoutLowPriority)

state.activateFilter(.rooms)
XCTAssertTrue(state.isFiltering)
Expand Down Expand Up @@ -71,7 +80,7 @@ final class RoomListFiltersStateTests: XCTestCase {
state.clearFilters()
XCTAssertFalse(state.isFiltering)
XCTAssertEqual(state.activeFilters, [])
XCTAssertEqual(state.availableFilters, RoomListFilter.allCases)
XCTAssertEqual(state.availableFilters, allCasesWithoutLowPriority)
}

func testOrder() {
Expand All @@ -81,7 +90,7 @@ final class RoomListFiltersStateTests: XCTestCase {

state.deactivateFilter(.favourites)
XCTAssertEqual(state.activeFilters, [])
XCTAssertEqual(state.availableFilters, RoomListFilter.allCases)
XCTAssertEqual(state.availableFilters, allCasesWithoutLowPriority)

state.activateFilter(.rooms)
XCTAssertEqual(state.activeFilters, [.rooms])
Expand All @@ -95,4 +104,25 @@ final class RoomListFiltersStateTests: XCTestCase {
XCTAssertEqual(state.activeFilters, [.rooms])
XCTAssertEqual(state.availableFilters, [.unreads, .favourites])
}

// MARK: Low Priority feature flag

// Don't forget to add .lowPriority into the mix above when enabling the feature.
func testWithLowPriorityFeature() {
enableLowPriorityFeature()
XCTAssertFalse(state.isFiltering)
XCTAssertEqual(state.activeFilters, [])
XCTAssertEqual(state.availableFilters, RoomListFilter.allCases)

state.activateFilter(.lowPriority)
XCTAssertEqual(state.activeFilters, [.lowPriority])
XCTAssertEqual(state.availableFilters, [.unreads, .people, .rooms])
}

// MARK: - Helpers

private func enableLowPriorityFeature() {
appSettings.lowPriorityFilterEnabled = true
state = RoomListFiltersState(appSettings: appSettings)
}
}
Loading
Loading