From aa4ea06a101b1a0f70b6d6294282e0aea3afcf7a Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Thu, 9 Oct 2025 16:56:54 +0100 Subject: [PATCH] The space tweaks continue! (#4606) * Update a string on the space announcement sheet. * Don't show space rooms after using the join button in the space room list. * Allow the Deselect All button to Select (almost) All as well. * Fix the cells in the leave space sheet when there are only admin rooms. --- .../en.lproj/Localizable.strings | 6 ++--- ElementX/Sources/Generated/Strings.swift | 10 ++++----- .../SpaceScreen/SpaceScreenModels.swift | 3 +-- .../SpaceScreen/SpaceScreenViewModel.swift | 14 +++++------- .../View/LeaveSpaceRoomDetailsCell.swift | 19 +++++++++++++++- .../SpaceScreen/View/LeaveSpaceView.swift | 22 +++++++++++-------- .../Spaces/SpaceScreen/View/SpaceScreen.swift | 4 ++-- Package.resolved | 6 ++--- ...eSpaceView.Only-Admin-Rooms-iPad-en-GB.png | 4 ++-- ...SpaceView.Only-Admin-Rooms-iPad-pseudo.png | 4 ++-- ...eView.Only-Admin-Rooms-iPhone-16-en-GB.png | 4 ++-- ...View.Only-Admin-Rooms-iPhone-16-pseudo.png | 4 ++-- ...acesAnnouncementSheetView.iPad-en-GB-0.png | 4 ++-- ...nnouncementSheetView.iPhone-16-en-GB-0.png | 4 ++-- .../Sources/SpaceScreenViewModelTests.swift | 18 --------------- 15 files changed, 63 insertions(+), 63 deletions(-) diff --git a/ElementX/Resources/Localizations/en.lproj/Localizable.strings b/ElementX/Resources/Localizations/en.lproj/Localizable.strings index 4f73220ae..c1953a022 100644 --- a/ElementX/Resources/Localizations/en.lproj/Localizable.strings +++ b/ElementX/Resources/Localizations/en.lproj/Localizable.strings @@ -72,6 +72,7 @@ "action_decline" = "Decline"; "action_decline_and_block" = "Decline and block"; "action_delete_poll" = "Delete Poll"; +"action_deselect_all" = "Deselect all"; "action_disable" = "Disable"; "action_discard" = "Discard"; "action_dismiss" = "Dismiss"; @@ -132,6 +133,7 @@ "action_retry_decryption" = "Retry decryption"; "action_save" = "Save"; "action_search" = "Search"; +"action_select_all" = "Select all"; "action_send" = "Send"; "action_send_edited_message" = "Send edited message"; "action_send_message" = "Send message"; @@ -193,7 +195,6 @@ "common_date_this_month" = "This month"; "common_decryption_error" = "Decryption error"; "common_description" = "Description"; -"common_deselect_all" = "Deselect all"; "common_developer_options" = "Developer options"; "common_device_id" = "Device ID"; "common_direct_chat" = "Direct chat"; @@ -281,7 +282,6 @@ "common_search_results" = "Search results"; "common_security" = "Security"; "common_seen_by" = "Seen by"; -"common_select_all" = "Select all"; "common_send_to" = "Send to"; "common_sending" = "Sending…"; "common_sending_failed" = "Sending failed"; @@ -657,7 +657,7 @@ "screen_space_announcement_item3" = "Discover any rooms you can join in your spaces"; "screen_space_announcement_item4" = "Join public spaces"; "screen_space_announcement_item5" = "Leave any spaces you’ve joined"; -"screen_space_announcement_notice" = "Creating and managing spaces is coming soon."; +"screen_space_announcement_notice" = "Filtering, creating and managing spaces is coming soon."; "screen_space_announcement_subtitle" = "Welcome to the beta version of Spaces! With this first version you can:"; "screen_space_announcement_title" = "Introducing Spaces"; "screen_space_list_description" = "Spaces you have created or joined."; diff --git a/ElementX/Sources/Generated/Strings.swift b/ElementX/Sources/Generated/Strings.swift index 39ca587dc..d1a4d6d50 100644 --- a/ElementX/Sources/Generated/Strings.swift +++ b/ElementX/Sources/Generated/Strings.swift @@ -178,6 +178,8 @@ internal enum L10n { internal static var actionDeclineAndBlock: String { return L10n.tr("Localizable", "action_decline_and_block") } /// Delete Poll internal static var actionDeletePoll: String { return L10n.tr("Localizable", "action_delete_poll") } + /// Deselect all + internal static var actionDeselectAll: String { return L10n.tr("Localizable", "action_deselect_all") } /// Disable internal static var actionDisable: String { return L10n.tr("Localizable", "action_disable") } /// Discard @@ -302,6 +304,8 @@ internal enum L10n { internal static var actionSave: String { return L10n.tr("Localizable", "action_save") } /// Search internal static var actionSearch: String { return L10n.tr("Localizable", "action_search") } + /// Select all + internal static var actionSelectAll: String { return L10n.tr("Localizable", "action_select_all") } /// Send internal static var actionSend: String { return L10n.tr("Localizable", "action_send") } /// Send edited message @@ -430,8 +434,6 @@ internal enum L10n { internal static var commonDecryptionError: String { return L10n.tr("Localizable", "common_decryption_error") } /// Description internal static var commonDescription: String { return L10n.tr("Localizable", "common_description") } - /// Deselect all - internal static var commonDeselectAll: String { return L10n.tr("Localizable", "common_deselect_all") } /// Developer options internal static var commonDeveloperOptions: String { return L10n.tr("Localizable", "common_developer_options") } /// Device ID @@ -638,8 +640,6 @@ internal enum L10n { internal static var commonSecurity: String { return L10n.tr("Localizable", "common_security") } /// Seen by internal static var commonSeenBy: String { return L10n.tr("Localizable", "common_seen_by") } - /// Select all - internal static var commonSelectAll: String { return L10n.tr("Localizable", "common_select_all") } /// Send to internal static var commonSendTo: String { return L10n.tr("Localizable", "common_send_to") } /// Sending… @@ -2938,7 +2938,7 @@ internal enum L10n { internal static var screenSpaceAnnouncementItem4: String { return L10n.tr("Localizable", "screen_space_announcement_item4") } /// Leave any spaces you’ve joined internal static var screenSpaceAnnouncementItem5: String { return L10n.tr("Localizable", "screen_space_announcement_item5") } - /// Creating and managing spaces is coming soon. + /// Filtering, creating and managing spaces is coming soon. internal static var screenSpaceAnnouncementNotice: String { return L10n.tr("Localizable", "screen_space_announcement_notice") } /// Welcome to the beta version of Spaces! With this first version you can: internal static var screenSpaceAnnouncementSubtitle: String { return L10n.tr("Localizable", "screen_space_announcement_subtitle") } diff --git a/ElementX/Sources/Screens/Spaces/SpaceScreen/SpaceScreenModels.swift b/ElementX/Sources/Screens/Spaces/SpaceScreen/SpaceScreenModels.swift index 65f75de1a..c97135fe9 100644 --- a/ElementX/Sources/Screens/Spaces/SpaceScreen/SpaceScreenModels.swift +++ b/ElementX/Sources/Screens/Spaces/SpaceScreen/SpaceScreenModels.swift @@ -27,8 +27,6 @@ struct SpaceScreenViewState: BindableState { var isSpaceManagementEnabled = false var bindings = SpaceScreenViewStateBindings() - - var spaceName: String { space.name ?? L10n.commonSpace } } struct SpaceScreenViewStateBindings { @@ -39,6 +37,7 @@ enum SpaceScreenViewAction { case spaceAction(SpaceRoomCell.Action) case leaveSpace case deselectAllLeaveRoomDetails + case selectAllLeaveRoomDetails case toggleLeaveSpaceRoomDetails(id: String) case confirmLeaveSpace case spaceSettings diff --git a/ElementX/Sources/Screens/Spaces/SpaceScreen/SpaceScreenViewModel.swift b/ElementX/Sources/Screens/Spaces/SpaceScreen/SpaceScreenViewModel.swift index bfd044570..afe13ad8d 100644 --- a/ElementX/Sources/Screens/Spaces/SpaceScreen/SpaceScreenViewModel.swift +++ b/ElementX/Sources/Screens/Spaces/SpaceScreen/SpaceScreenViewModel.swift @@ -95,6 +95,11 @@ class SpaceScreenViewModel: SpaceScreenViewModelType, SpaceScreenViewModelProtoc for room in leaveHandle.rooms { room.isSelected = false } + case .selectAllLeaveRoomDetails: + guard let leaveHandle = state.bindings.leaveHandle else { fatalError("The leave handle should be available.") } + for room in leaveHandle.rooms where !room.isLastAdmin { + room.isSelected = true + } case .toggleLeaveSpaceRoomDetails(let spaceRoomID): guard let room = state.bindings.leaveHandle?.rooms.first(where: { $0.spaceRoomProxy.id == spaceRoomID }) else { fatalError("The space room to toggle is not in the list of rooms to leave.") @@ -125,14 +130,7 @@ class SpaceScreenViewModel: SpaceScreenViewModelType, SpaceScreenViewModelProtoc return } - // If multiple join operations are running, then only show the last one. - guard state.joiningRoomIDs == [spaceRoomProxy.id] else { return } - - if spaceRoomProxy.isSpace { - await selectSpace(spaceRoomProxy) - } else { - actionsSubject.send(.selectRoom(roomID: spaceRoomProxy.id)) - } + // We don't want to show the space room after joining it this way 🤷‍♂️ } private func selectSpace(_ spaceRoomProxy: SpaceRoomProxyProtocol) async { diff --git a/ElementX/Sources/Screens/Spaces/SpaceScreen/View/LeaveSpaceRoomDetailsCell.swift b/ElementX/Sources/Screens/Spaces/SpaceScreen/View/LeaveSpaceRoomDetailsCell.swift index a77643cf9..18c4df7fe 100644 --- a/ElementX/Sources/Screens/Spaces/SpaceScreen/View/LeaveSpaceRoomDetailsCell.swift +++ b/ElementX/Sources/Screens/Spaces/SpaceScreen/View/LeaveSpaceRoomDetailsCell.swift @@ -12,6 +12,7 @@ struct LeaveSpaceRoomDetailsCell: View { @Environment(\.dynamicTypeSize) private var dynamicTypeSize let room: LeaveSpaceRoomDetails + var hideSelection = false let mediaProvider: MediaProviderProtocol? let action: () -> Void @@ -53,7 +54,9 @@ struct LeaveSpaceRoomDetailsCell: View { .frame(maxWidth: .infinity, alignment: .leading) .padding(.vertical, 8) - ListRowAccessory.multiSelection(room.isSelected) + if !hideSelection { + ListRowAccessory.multiSelection(room.isSelected) + } } .padding(.horizontal, 16) } @@ -99,6 +102,13 @@ struct LeaveSpaceRoomDetailsCell_Previews: PreviewProvider, TestablePreview { isLastAdmin: true, isSelected: false), mediaProvider: MediaProviderMock(configuration: .init())) { } + LeaveSpaceRoomDetailsCell(room: .init(spaceRoomProxy: SpaceRoomProxyMock(.init(id: "2", + name: "My Space", + isSpace: true)), + isLastAdmin: true, + isSelected: false), + hideSelection: true, + mediaProvider: MediaProviderMock(configuration: .init())) { } LeaveSpaceRoomDetailsCell(room: .init(spaceRoomProxy: SpaceRoomProxyMock(.init(id: "3", name: "Room", isSpace: false)), @@ -111,6 +121,13 @@ struct LeaveSpaceRoomDetailsCell_Previews: PreviewProvider, TestablePreview { isLastAdmin: true, isSelected: false), mediaProvider: MediaProviderMock(configuration: .init())) { } + LeaveSpaceRoomDetailsCell(room: .init(spaceRoomProxy: SpaceRoomProxyMock(.init(id: "4", + name: "My Room", + isSpace: false)), + isLastAdmin: true, + isSelected: false), + hideSelection: true, + mediaProvider: MediaProviderMock(configuration: .init())) { } } } } diff --git a/ElementX/Sources/Screens/Spaces/SpaceScreen/View/LeaveSpaceView.swift b/ElementX/Sources/Screens/Spaces/SpaceScreen/View/LeaveSpaceView.swift index e39dee311..ea6190fb8 100644 --- a/ElementX/Sources/Screens/Spaces/SpaceScreen/View/LeaveSpaceView.swift +++ b/ElementX/Sources/Screens/Spaces/SpaceScreen/View/LeaveSpaceView.swift @@ -42,7 +42,7 @@ struct LeaveSpaceView: View { BigIcon(icon: \.errorSolid, style: .alertSolid) VStack(spacing: 8) { - Text(leaveHandle.title(spaceName: context.viewState.spaceName)) + Text(leaveHandle.title(spaceName: context.viewState.space.name)) .font(.compound.headingMDBold) .foregroundStyle(.compound.textPrimary) .multilineTextAlignment(.center) @@ -64,20 +64,24 @@ struct LeaveSpaceView: View { LazyVStack(spacing: 0, pinnedViews: .sectionHeaders) { Section { ForEach(leaveHandle.rooms, id: \.spaceRoomProxy.id) { room in - LeaveSpaceRoomDetailsCell(room: room, mediaProvider: context.mediaProvider) { + LeaveSpaceRoomDetailsCell(room: room, + hideSelection: leaveHandle.mode == .onlyAdminRooms, + mediaProvider: context.mediaProvider) { context.send(viewAction: .toggleLeaveSpaceRoomDetails(id: room.spaceRoomProxy.id)) } .disabled(room.isLastAdmin) } } header: { - Button(L10n.commonDeselectAll) { - context.send(viewAction: .deselectAllLeaveRoomDetails) + if leaveHandle.mode == .manyRooms { + Button(leaveHandle.selectedCount > 0 ? L10n.actionDeselectAll : L10n.actionSelectAll) { + context.send(viewAction: leaveHandle.selectedCount > 0 ? .deselectAllLeaveRoomDetails : .selectAllLeaveRoomDetails) + } + .buttonStyle(.compound(.textLink, size: .small)) + .frame(maxWidth: .infinity, alignment: .trailing) + .padding(.horizontal, 16) + .padding(.bottom, 8) + .background(Color.compound.bgCanvasDefault.ignoresSafeArea()) } - .buttonStyle(.compound(.textLink, size: .small)) - .frame(maxWidth: .infinity, alignment: .trailing) - .padding(.horizontal, 16) - .padding(.bottom, 8) - .background(Color.compound.bgCanvasDefault.ignoresSafeArea()) } } } diff --git a/ElementX/Sources/Screens/Spaces/SpaceScreen/View/SpaceScreen.swift b/ElementX/Sources/Screens/Spaces/SpaceScreen/View/SpaceScreen.swift index 1069690ac..dfeab735e 100644 --- a/ElementX/Sources/Screens/Spaces/SpaceScreen/View/SpaceScreen.swift +++ b/ElementX/Sources/Screens/Spaces/SpaceScreen/View/SpaceScreen.swift @@ -20,7 +20,7 @@ struct SpaceScreen: View { } } .background(Color.compound.bgCanvasDefault.ignoresSafeArea()) - .navigationTitle(context.viewState.spaceName) + .navigationTitle(context.viewState.space.name) .navigationBarTitleDisplayMode(.inline) .toolbar { toolbar } .sheet(item: $context.leaveHandle) { leaveHandle in @@ -50,7 +50,7 @@ struct SpaceScreen: View { // Use the same trick as the RoomScreen for a leading title view that // also hides the navigation title. ToolbarItem(placement: .principal) { - RoomHeaderView(roomName: context.viewState.spaceName, + RoomHeaderView(roomName: context.viewState.space.name, roomAvatar: context.viewState.space.avatar, mediaProvider: context.mediaProvider) } diff --git a/Package.resolved b/Package.resolved index f8ce7fd80..295415dfe 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "81e8144c17486250f9157eb0e84dfe04af3b41d5924c8c7d77616ac85025046a", + "originHash" : "8fc5521127eb479d8784a8d77e682b092e09920246eaefaccc1b1151c08e0f3e", "pins" : [ { "identity" : "swift-argument-parser", @@ -23,8 +23,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/jpsim/Yams", "state" : { - "revision" : "d41ba4e7164c0838c6d48351f7575f7f762151fe", - "version" : "6.1.0" + "revision" : "51b5127c7fb6ffac106ad6d199aaa33c5024895f", + "version" : "6.2.0" } } ], diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPad-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPad-en-GB.png index 27da8e03c..bea2eda21 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPad-en-GB.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPad-en-GB.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:3fc79ce67766fa49b1be89a432a26123ed46d1422ebbc0340100df1df762695f -size 151198 +oid sha256:4ff955a433b413f91ce08fdc54b9097f59bdd28fdbd3af9dd0374564eb1beb24 +size 144012 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPad-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPad-pseudo.png index bb3750717..ef4094719 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPad-pseudo.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPad-pseudo.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:dca0726cbea6d22bd94757903e793ac31ce563cd85cc51236e3d7fad1b046022 -size 181645 +oid sha256:910ba40be143efd2bcf7043918063ee13c31fb5d9d04db2691f13bc9d4cced90 +size 171504 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPhone-16-en-GB.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPhone-16-en-GB.png index 106f62726..5b1aba97e 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPhone-16-en-GB.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPhone-16-en-GB.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:861d84b03ae9fa5dc6a8fe62fe107e08075eafd838858dbc1bd512fbabe0dcc2 -size 100419 +oid sha256:6ee340c15db21c874e45553294f98fea342502bf08b28b286c337ddc2d243a6b +size 94684 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPhone-16-pseudo.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPhone-16-pseudo.png index 4b89df592..78468139a 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPhone-16-pseudo.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/leaveSpaceView.Only-Admin-Rooms-iPhone-16-pseudo.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:bb3231bf157a44784baae01316b8c86b633301ad7715ba64152f4185f8fce3a4 -size 129281 +oid sha256:5244fdcad023960dd896206cb51196f310b34251321e1cf6e17d9348c6fdf622 +size 123733 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/spacesAnnouncementSheetView.iPad-en-GB-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/spacesAnnouncementSheetView.iPad-en-GB-0.png index e6b0052c8..1a50f1392 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/spacesAnnouncementSheetView.iPad-en-GB-0.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/spacesAnnouncementSheetView.iPad-en-GB-0.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:eccd2ad5a68030a787644ec2f4e08f279ba633065101ef28f6db3363a0efd3b0 -size 146453 +oid sha256:e6ed7f06c34e88d9b96a8850d29a0eca85abda01eeb8d508960a684059eec708 +size 147149 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/spacesAnnouncementSheetView.iPhone-16-en-GB-0.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/spacesAnnouncementSheetView.iPhone-16-en-GB-0.png index 7b4cdcbd2..365373143 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/spacesAnnouncementSheetView.iPhone-16-en-GB-0.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/spacesAnnouncementSheetView.iPhone-16-en-GB-0.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:07d585ad557174655a0998f915ce48eb497178482b8493cc7184179f734f9d2a -size 96164 +oid sha256:cf9785802d572f019d96ddb1f52c166e0ae80cd5a84b875ab557204e8f969470 +size 97907 diff --git a/UnitTests/Sources/SpaceScreenViewModelTests.swift b/UnitTests/Sources/SpaceScreenViewModelTests.swift index 42e6bfad7..49ea56da6 100644 --- a/UnitTests/Sources/SpaceScreenViewModelTests.swift +++ b/UnitTests/Sources/SpaceScreenViewModelTests.swift @@ -149,21 +149,12 @@ class SpaceScreenViewModelTests: XCTestCase { expectation.fulfill() return .success(()) } - let deferred = deferFulfillment(viewModel.actionsPublisher) { _ in true } let deferredState = deferFulfillment(viewModel.context.observe(\.viewState.joiningRoomIDs), transitionValues: [[selectedSpace.id], []]) viewModel.context.send(viewAction: .spaceAction(.join(selectedSpace))) await fulfillment(of: [expectation]) try await deferredState.fulfill() - let action = try await deferred.fulfill() - - switch action { - case .selectSpace(let spaceRoomListProxy) where spaceRoomListProxy.spaceRoomProxy.id == selectedSpace.id: - break - default: - XCTFail("The join should finish by selecting the space.") - } } func testJoiningRoom() async throws { @@ -176,21 +167,12 @@ class SpaceScreenViewModelTests: XCTestCase { expectation.fulfill() return .success(()) } - let deferred = deferFulfillment(viewModel.actionsPublisher) { _ in true } let deferredState = deferFulfillment(viewModel.context.observe(\.viewState.joiningRoomIDs), transitionValues: [[selectedRoom.id], []]) viewModel.context.send(viewAction: .spaceAction(.join(selectedRoom))) await fulfillment(of: [expectation]) try await deferredState.fulfill() - let action = try await deferred.fulfill() - - switch action { - case .selectRoom(let roomID) where roomID == selectedRoom.id: - break - default: - XCTFail("The join should finish by selecting the room.") - } } func testLeavingSpace() async throws {