mirror of
https://github.com/Novaloop-AG/element-x-ios.git
synced 2026-11-02 12:07:55 +00:00
Only offer joined rooms in the forward and share pickers (#6255)
* Initial plan * Offer only joined rooms in destination pickers Co-authored-by: Velin92 <34335419+Velin92@users.noreply.github.com> * Clean whitespace in share picker test Co-authored-by: Velin92 <34335419+Velin92@users.noreply.github.com> * Restrict the shared picker room list to joined rooms The alternate room summary provider, which backs the forward, share and Add rooms pickers and global search, now always includes the SDK's joined filter in its base filter. This replaces the per-call joinedOnly flags, so Add rooms no longer offers invited rooms or spaces and no screen can forget the flag. A filter set before the provider has its room list is now applied once the list is set, instead of being reset to the default. The share picker hit this on a cold launch from the share extension. The forward and share pickers also hide upgraded rooms, matching the Suggestions section. The SDK has no filter for these. Also registers the share picker's test file in the project, fixes the blank-line indentation SwiftFormat flagged, and re-records the picker snapshots without the upgraded room. * Check room membership for forwarding suggestions and search breadcrumbs RoomSummary now has isJoined, built from the room's membership. The forwarding suggestions and the search breadcrumbs used a missing joinRequestType to mean joined, which is also the case for rooms that have been left or banned from. It only worked because the room list's base filter already drops those. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Velin92 <34335419+Velin92@users.noreply.github.com> Co-authored-by: Mauro Romito <mauro.romito@element.io>
This commit is contained in:
Copilot
GitHub
@@ -43,8 +43,8 @@ extension RoomSummaryProviderMock {
|
||||
|
||||
setFilterClosure = { [initialRooms, roomListSubject] filter in
|
||||
switch filter {
|
||||
case let .search(query, joinedOnly):
|
||||
var rooms = joinedOnly ? initialRooms.filter { $0.joinRequestType == nil } : initialRooms
|
||||
case let .search(query):
|
||||
var rooms = initialRooms
|
||||
|
||||
if !query.isEmpty {
|
||||
rooms = rooms.filter { $0.name.localizedCaseInsensitiveContains(query) }
|
||||
@@ -73,10 +73,12 @@ extension RoomSummary {
|
||||
static func mock(id: String,
|
||||
name: String,
|
||||
isSpace: Bool = false,
|
||||
isJoined: Bool = true,
|
||||
canonicalAlias: String? = nil) -> RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(.init()),
|
||||
id: id,
|
||||
joinRequestType: nil,
|
||||
isJoined: isJoined,
|
||||
name: name,
|
||||
isDirect: false,
|
||||
isSpace: isSpace,
|
||||
@@ -105,6 +107,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "1",
|
||||
joinRequestType: nil,
|
||||
isJoined: true,
|
||||
name: "Foundation 🔭🪐🌌",
|
||||
isDirect: false,
|
||||
isSpace: false,
|
||||
@@ -128,6 +131,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "2",
|
||||
joinRequestType: nil,
|
||||
isJoined: true,
|
||||
name: "Foundation and Empire",
|
||||
isDirect: false,
|
||||
isSpace: false,
|
||||
@@ -151,6 +155,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "3",
|
||||
joinRequestType: nil,
|
||||
isJoined: true,
|
||||
name: "Second Foundation",
|
||||
isDirect: false,
|
||||
isSpace: false,
|
||||
@@ -174,6 +179,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "4",
|
||||
joinRequestType: nil,
|
||||
isJoined: true,
|
||||
name: "Foundation's Edge",
|
||||
isDirect: false,
|
||||
isSpace: false,
|
||||
@@ -197,6 +203,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "5",
|
||||
joinRequestType: nil,
|
||||
isJoined: true,
|
||||
name: "Foundation and Earth",
|
||||
isDirect: true,
|
||||
isSpace: false,
|
||||
@@ -220,6 +227,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "6",
|
||||
joinRequestType: nil,
|
||||
isJoined: true,
|
||||
name: "Prelude to Foundation",
|
||||
isDirect: true,
|
||||
isSpace: false,
|
||||
@@ -243,6 +251,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "7",
|
||||
joinRequestType: nil,
|
||||
isJoined: true,
|
||||
name: "Tombstoned",
|
||||
isDirect: false,
|
||||
isSpace: false,
|
||||
@@ -266,6 +275,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "0",
|
||||
joinRequestType: nil,
|
||||
isJoined: true,
|
||||
name: "Unknown",
|
||||
isDirect: false,
|
||||
isSpace: false,
|
||||
@@ -322,6 +332,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "someAwesomeRoomId1",
|
||||
joinRequestType: .invite(inviter: RoomMemberProxyMock.mockCharlie),
|
||||
isJoined: false,
|
||||
name: "First room",
|
||||
isDirect: false,
|
||||
isSpace: false,
|
||||
@@ -345,6 +356,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "someAwesomeRoomId2",
|
||||
joinRequestType: .invite(inviter: RoomMemberProxyMock.mockCharlie),
|
||||
isJoined: false,
|
||||
name: "Second room",
|
||||
isDirect: true,
|
||||
isSpace: false,
|
||||
@@ -371,6 +383,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "!space1:matrix.org",
|
||||
joinRequestType: .invite(inviter: RoomMemberProxyMock.mockCharlie),
|
||||
isJoined: false,
|
||||
name: "First space",
|
||||
isDirect: false,
|
||||
isSpace: true,
|
||||
@@ -394,6 +407,7 @@ extension Array where Element == RoomSummary {
|
||||
RoomSummary(room: RoomSDKMock(),
|
||||
id: "!space2:matrix.org",
|
||||
joinRequestType: .invite(inviter: RoomMemberProxyMock.mockCharlie),
|
||||
isJoined: false,
|
||||
name: "Second space",
|
||||
isDirect: false,
|
||||
isSpace: true,
|
||||
|
||||
@@ -218,6 +218,7 @@ private extension HomeScreenRoom {
|
||||
let summary = RoomSummary(room: RoomSDKMock(),
|
||||
id: "@someone:somewhere.com",
|
||||
joinRequestType: .invite(inviter: inviter),
|
||||
isJoined: false,
|
||||
name: "Some Guy",
|
||||
isDirect: true,
|
||||
isSpace: false,
|
||||
@@ -254,6 +255,7 @@ private extension HomeScreenRoom {
|
||||
let summary = RoomSummary(room: RoomSDKMock(),
|
||||
id: "@someone:somewhere.com",
|
||||
joinRequestType: .invite(inviter: inviter),
|
||||
isJoined: false,
|
||||
name: name,
|
||||
isDirect: false,
|
||||
isSpace: isSpace,
|
||||
|
||||
@@ -150,6 +150,7 @@ private extension HomeScreenRoom {
|
||||
let summary = RoomSummary(room: RoomSDKMock(),
|
||||
id: "@someone:somewhere.com",
|
||||
joinRequestType: .invite(inviter: inviter),
|
||||
isJoined: false,
|
||||
name: "Some Guy",
|
||||
isDirect: true,
|
||||
isSpace: false,
|
||||
@@ -183,6 +184,7 @@ private extension HomeScreenRoom {
|
||||
let summary = RoomSummary(room: RoomSDKMock(),
|
||||
id: "@someone:somewhere.com",
|
||||
joinRequestType: .invite(inviter: inviter),
|
||||
isJoined: false,
|
||||
name: "Awesome Room",
|
||||
isDirect: false,
|
||||
isSpace: false,
|
||||
|
||||
@@ -268,6 +268,7 @@ struct HomeScreenRoomCell_Previews: PreviewProvider, TestablePreview {
|
||||
let summary = RoomSummary(room: RoomSDKMock(),
|
||||
id: UUID().uuidString,
|
||||
joinRequestType: nil,
|
||||
isJoined: true,
|
||||
name: name,
|
||||
isDirect: heroes.count == 1,
|
||||
isSpace: false,
|
||||
|
||||
+2
-2
@@ -88,7 +88,7 @@ class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, Me
|
||||
|
||||
private func updateRooms() {
|
||||
state.rooms = roomSummaryProvider.roomListPublisher.value
|
||||
.filter { $0.id != forwardingPayload.roomID }
|
||||
.filter { $0.id != forwardingPayload.roomID && !$0.isTombstoned }
|
||||
.map(MessageForwardingRoom.init(summary:))
|
||||
}
|
||||
|
||||
@@ -99,7 +99,7 @@ class MessageForwardingScreenViewModel: MessageForwardingScreenViewModelType, Me
|
||||
state.suggestedRooms = roomIDs
|
||||
.filter { $0 != forwardingPayload.roomID }
|
||||
.compactMap(clientProxy.roomSummaryForIdentifier)
|
||||
.filter { $0.joinRequestType == nil && !$0.isSpace && !$0.isTombstoned }
|
||||
.filter { $0.isJoined && !$0.isSpace && !$0.isTombstoned }
|
||||
.prefix(Self.maxSuggestedRoomCount)
|
||||
.map(MessageForwardingRoom.init(summary:))
|
||||
}
|
||||
|
||||
@@ -77,7 +77,7 @@ class RoomSelectionScreenViewModel: RoomSelectionScreenViewModelType, RoomSelect
|
||||
private func updateRooms() {
|
||||
var rooms = [RoomSelectionRoom]()
|
||||
|
||||
for summary in roomSummaryProvider.roomListPublisher.value {
|
||||
for summary in roomSummaryProvider.roomListPublisher.value where !summary.isTombstoned {
|
||||
rooms.append(.init(id: summary.id,
|
||||
title: summary.name,
|
||||
description: summary.roomListDescription,
|
||||
|
||||
@@ -194,7 +194,7 @@ class SearchScreenViewModel: SearchScreenViewModelType, SearchScreenViewModelPro
|
||||
switch mode {
|
||||
case .rooms:
|
||||
searchedQueries[mode] = searchQuery
|
||||
roomSummaryProvider.setFilter(.search(query: searchQuery, joinedOnly: true))
|
||||
roomSummaryProvider.setFilter(.search(query: searchQuery))
|
||||
case .messages:
|
||||
setQueryTask = Task { [weak self] in
|
||||
// Debounce message queries; superseded keystrokes cancel this before it commits.
|
||||
@@ -215,7 +215,7 @@ class SearchScreenViewModel: SearchScreenViewModelType, SearchScreenViewModelPro
|
||||
return .query(query)
|
||||
case .room(let roomID):
|
||||
// Drop rooms the user has since left or is only invited to, we've got nothing to show for them.
|
||||
guard let summary = clientProxy.roomSummaryForIdentifier(roomID), summary.joinRequestType == nil else { return nil }
|
||||
guard let summary = clientProxy.roomSummaryForIdentifier(roomID), summary.isJoined else { return nil }
|
||||
return .room(SearchScreenRoom(summary))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1582,6 +1582,7 @@ private struct ClientProxyServices {
|
||||
alternateRoomSummaryProvider = RoomSummaryProvider(roomListService: roomListService,
|
||||
eventStringBuilder: eventStringBuilder,
|
||||
name: "AlternateAllRooms",
|
||||
includesOnlyJoinedRooms: true,
|
||||
notificationSettings: notificationSettings,
|
||||
userSettings: userSettings)
|
||||
|
||||
|
||||
@@ -149,7 +149,7 @@ protocol ClientProxyProtocol: AnyObject {
|
||||
|
||||
/// Used for listing rooms that shouldn't be affected by the main `roomSummaryProvider` filtering
|
||||
/// But can still be filtered by queries, since this may be shared across multiple views, remember to reset
|
||||
/// The filtering state when you are done with it
|
||||
/// The filtering state when you are done with it. Only lists joined rooms.
|
||||
var alternateRoomSummaryProvider: RoomSummaryProviderProtocol { get }
|
||||
|
||||
/// Used for listing rooms, can't be filtered nor its state observed
|
||||
|
||||
@@ -30,6 +30,7 @@ nonisolated struct RoomSummary {
|
||||
let id: String
|
||||
|
||||
let joinRequestType: JoinRequestType?
|
||||
let isJoined: Bool
|
||||
|
||||
let name: String
|
||||
let isDirect: Bool
|
||||
@@ -118,6 +119,7 @@ nonisolated extension RoomSummary {
|
||||
RoomSummary(room: room,
|
||||
id: room.id(),
|
||||
joinRequestType: nil,
|
||||
isJoined: false,
|
||||
name: room.id(),
|
||||
isDirect: false,
|
||||
isSpace: false,
|
||||
@@ -165,6 +167,7 @@ nonisolated extension RoomSummary {
|
||||
activeCallIntent = nil
|
||||
|
||||
joinRequestType = nil
|
||||
isJoined = true
|
||||
isMarkedUnread = false
|
||||
isFavourite = false
|
||||
isTombstoned = false
|
||||
|
||||
@@ -17,6 +17,7 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
|
||||
private let shouldUpdateVisibleRange: Bool
|
||||
private let notificationSettings: NotificationSettingsProxyProtocol
|
||||
private let userSettings: UserSettings
|
||||
private let baseFilter: [RoomListEntriesDynamicFilterKind]
|
||||
|
||||
private let roomListPageSize: UInt32
|
||||
/// Remember how many rooms we had on the previous requests so we can deduplicate
|
||||
@@ -62,10 +63,12 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
|
||||
/// - shouldUpdateVisibleRange: whether this summary provider should forward visible ranges
|
||||
/// to the room list service through the `applyInput(input: .viewport(ranges` api. Only useful for
|
||||
/// lists that need to update the visible range on Sliding Sync
|
||||
/// - includesOnlyJoinedRooms: whether to leave out invites and other rooms the user hasn't joined, whatever the filter
|
||||
init(roomListService: RoomListServiceProtocol,
|
||||
eventStringBuilder: RoomEventStringBuilder,
|
||||
name: String,
|
||||
shouldUpdateVisibleRange: Bool = false,
|
||||
includesOnlyJoinedRooms: Bool = false,
|
||||
roomListPageSize: UInt32 = 100,
|
||||
notificationSettings: NotificationSettingsProxyProtocol,
|
||||
userSettings: UserSettings) {
|
||||
@@ -77,6 +80,10 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
|
||||
self.userSettings = userSettings
|
||||
self.roomListPageSize = roomListPageSize
|
||||
|
||||
baseFilter = [.any(filters: [.all(filters: [.nonSpace, .nonLeft]),
|
||||
.all(filters: [.space, .invite])]),
|
||||
.deduplicateVersions] + (includesOnlyJoinedRooms ? [.joined] : [])
|
||||
|
||||
let (diffsStream, diffsContinuation) = AsyncStream<[RoomListEntriesUpdate]>.makeStream()
|
||||
self.diffsContinuation = diffsContinuation
|
||||
|
||||
@@ -120,8 +127,9 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
|
||||
})
|
||||
|
||||
// Forces the listener above to be called with the current state
|
||||
let filter = currentFilter ?? .all(filters: [])
|
||||
currentFilter = nil
|
||||
setFilter(.all(filters: []))
|
||||
setFilter(filter)
|
||||
|
||||
let stateUpdatesSubscriptionResult = try roomList.loadingState(listener: SDKListener { [loadingStateContinuation] state in
|
||||
loadingStateContinuation.yield(state)
|
||||
@@ -146,15 +154,11 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
|
||||
|
||||
currentFilter = filter
|
||||
|
||||
let baseFilter: [RoomListEntriesDynamicFilterKind] = [.any(filters: [.all(filters: [.nonSpace, .nonLeft]),
|
||||
.all(filters: [.space, .invite])]),
|
||||
.deduplicateVersions]
|
||||
|
||||
switch filter {
|
||||
case .excludeAll:
|
||||
_ = listUpdatesSubscriptionResult?.controller().setFilter(kind: .none)
|
||||
case let .search(query, joinedOnly):
|
||||
let filters = nameFilter(for: query) + baseFilter + (joinedOnly ? [.joined] : [])
|
||||
case let .search(query):
|
||||
let filters = nameFilter(for: query) + baseFilter
|
||||
_ = listUpdatesSubscriptionResult?.controller().setFilter(kind: .all(filters: filters))
|
||||
case .rooms(let roomIDs, let filters):
|
||||
var rustFilters = filters.map(\.rustFilter) + baseFilter
|
||||
@@ -372,6 +376,7 @@ class RoomSummaryProvider: RoomSummaryProviderProtocol {
|
||||
return RoomSummary(room: room,
|
||||
id: roomInfo.id,
|
||||
joinRequestType: joinRequestType,
|
||||
isJoined: roomInfo.membership == .joined,
|
||||
name: roomInfo.displayName ?? roomInfo.id,
|
||||
isDirect: roomInfo.isDirect,
|
||||
isSpace: roomInfo.isSpace,
|
||||
|
||||
@@ -36,8 +36,8 @@ enum RoomSummaryProviderState {
|
||||
enum RoomSummaryProviderFilter: Equatable {
|
||||
/// Filters out everything
|
||||
case excludeAll
|
||||
/// Includes only the items that satisfy the predicate logic, optionally restricted to joined rooms
|
||||
case search(query: String, joinedOnly: Bool = false)
|
||||
/// Includes only the items that satisfy the predicate logic
|
||||
case search(query: String)
|
||||
/// Includes only what satisfies the filters used
|
||||
case all(filters: Set<RoomListFilter>)
|
||||
/// Include only rooms from the given that satisfy the given filters
|
||||
|
||||
Reference in New Issue
Block a user