Leave space - UI #5354

bmarty · 2025-09-15T14:25:25Z

Content

Add UI for the ability to leave space.

Waiting for new Rust SDK api to get the list of rooms to left when leaving the space to finalize the work, but this PR can be reviewed and merged, the entry point to leave the space has been commented out.

Sharing a space should work though.

Motivation and context

Closes #5300
Partially implement #5301

Screenshots / GIFs

See recorded ones

Tests

Check that the recorded screenshots match the design at https://www.figma.com/design/kcnHxunG1LDWXsJhaNuiHz/ER-145--Spaces-on-Element-X?node-id=3947-68767&t=bbP1NmiZAkxSSwld-0

Tested devices

Physical
Emulator
OS version(s):

Checklist

Changes have been tested on an Android device or Android emulator with API 24
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request title will be used in the release note, it clearly define what will change for the user
Pull request includes screenshots or videos if containing UI changes
You've made a self review of your PR

github-actions · 2025-09-15T14:32:23Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/1nSXY2

codecov · 2025-09-15T14:53:34Z

Codecov Report

❌ Patch coverage is 85.47945% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.97%. Comparing base (1e54633) to head (ef36922).
⚠️ Report is 15 commits behind head on develop.

Files with missing lines	Patch %	Lines
...d/features/space/impl/leave/LeaveSpacePresenter.kt	46.51%	23 Missing ⚠️
...ndroid/features/space/impl/leave/LeaveSpaceView.kt	92.63%	7 Missing and 7 partials ⚠️
...o/element/android/appnav/LoggedInEventProcessor.kt	0.00%	6 Missing ⚠️
...roid/features/space/impl/leave/LeaveSpaceEvents.kt	0.00%	5 Missing ⚠️
...ment/android/features/space/impl/root/SpaceView.kt	80.00%	4 Missing ⚠️
...atures/space/impl/leave/LeaveSpaceStateProvider.kt	98.59%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #5354      +/-   ##
===========================================
- Coverage    80.99%   80.97%   -0.02%     
===========================================
  Files         2306     2312       +6     
  Lines        63090    63448     +358     
  Branches      7977     8010      +33     
===========================================
+ Hits         51099    51378     +279     
- Misses        9060     9131      +71     
- Partials      2931     2939       +8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jmartinesp

Thanks! LGTM in general, a couple of comments but nothing blocking.

jmartinesp · 2025-09-15T14:56:25Z

features/space/impl/src/main/kotlin/io/element/android/features/space/impl/SpaceEvents.kt

+    data object StartLeaveSpace : SpaceEvents
+    data object LeaveSpace : SpaceEvents


Do we have some naming/structure we usually follow for these cases?

I think we'd usually have something like data class LeaveSpace(val needsConfirmation: Boolean) and use tell apart the 2 states. Or we'd also sometimes have LeaveSpace and some DoLeaveSpace events, if I'm not mistaken.

True, we also have AsyncAction with a confirmation state, which is maybe better here. I'll give it a try

jmartinesp · 2025-09-15T15:06:45Z

...x/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomMembershipObserver.kt

 class RoomMembershipObserver {
    data class RoomMembershipUpdate(
-        val roomId: RoomId,
+        val roomInfo: RoomInfo,


It seems a bit weird to bundle the whole RoomInfo here, is it just to have access to the isSpace property?

jmartinesp · 2025-09-15T15:08:25Z

...tests/src/test/snapshots/images/features.space.impl.leave_LeaveSpaceBottomSheet_Day_1_en.png

Nit: the phrasing here seems a bit odd, I'd expect something like "You'll be leaving all your rooms in this space too", which seems simpler. But if it was designed like this 🤷 .

The wording will probably get updated, I'll move this PR to draft.

ElementBot · 2025-09-25T21:05:07Z

	Warnings
⚠️	libraries/ui-strings/src/main/res/values-cs/translations.xml#L7 - For locale "cs" (Czech) the following quantity should also be defined: `many` (e.g. "10.0 dne")
⚠️	libraries/ui-strings/src/main/res/values-cs/translations.xml#L36 - For locale "cs" (Czech) the following quantity should also be defined: `many` (e.g. "10.0 dne")
⚠️	libraries/ui-strings/src/main/res/values-cs/translations.xml#L234 - For locale "cs" (Czech) the following quantity should also be defined: `many` (e.g. "10.0 dne")
⚠️	libraries/ui-strings/src/main/res/values-cs/translations.xml#L239 - For locale "cs" (Czech) the following quantity should also be defined: `many` (e.g. "10.0 dne")
⚠️	libraries/ui-strings/src/main/res/values-cs/translations.xml#L268 - For locale "cs" (Czech) the following quantity should also be defined: `many` (e.g. "10.0 dne")
⚠️	libraries/ui-strings/src/main/res/values-cs/translations.xml#L284 - For locale "cs" (Czech) the following quantities should also be defined: `few` (e.g. "2 dny"), `many` (e.g. "10.0 dne"), `one` (e.g. "1 den")
⚠️	libraries/ui-strings/src/main/res/values-cs/translations.xml#L295 - For locale "cs" (Czech) the following quantity should also be defined: `many` (e.g. "10.0 dne")
⚠️	libraries/ui-strings/src/main/res/values-cs/translations.xml#L323 - For locale "cs" (Czech) the following quantity should also be defined: `many` (e.g. "10.0 dne")
⚠️	libraries/ui-strings/src/main/res/values-cs/translations.xml#L430 - For locale "cs" (Czech) the following quantity should also be defined: `many` (e.g. "10.0 dne")

Generated by 🚫 dangerJS against ef36922

Translations will be back during the next Localazy sync.

bmarty · 2025-09-26T12:58:16Z

@jmartinesp the UI has changed a bit, and we may want to merge this PR even if the usage of the SDK is not finished. The entry point to leave Space has been commented out. This would need a brand new review I think. Sorry for the extra work!

jmartinesp

A few comments, I also haven't really tested the latest changes.

jmartinesp · 2025-09-26T13:28:19Z

...ace/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpacePresenter.kt

+            mutableStateOf<Set<RoomId>>(emptySet())
+        }
+        val joinedSpaceRooms by produceState(emptyList()) {
+            // TODO Get the joined room from the SDK, should also have the


This comment seems incomplete, so much suspense 🕵️ .

jmartinesp · 2025-09-26T13:31:15Z

...es/space/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpaceView.kt

+@Composable
+private fun LeaveSpaceButtons(
+    showLeaveButton: Boolean,
+    nbOfSelectedRooms: Int,


selectedRoomsCount?

jmartinesp · 2025-09-26T13:31:55Z

...es/space/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpaceView.kt

+            )
+            .clickable(
+                enabled = selectableSpaceRoom.isLastAdmin.not(),
+                // TODO


Missing TODO details here.

Oh yes. I'll address this comment in the next PR, it's about a11y.

jmartinesp · 2025-09-26T13:42:22Z

...s/space/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpaceState.kt

+    val showQuickAction = rooms
+        .any { !it.isLastAdmin }
+
+    /**
+     * True if there are rooms and they are all selected.
+     */
+    val areAllSelected = rooms
+        .filter { !it.isLastAdmin }


Not a big deal, but maybe we could reuse the rooms.filter here for the .any above and the .all below instead of filtering the list 3 times? Since it'll be a small list (probably) maybe it's not worth it.

jmartinesp · 2025-09-26T13:43:37Z

...ace/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpacePresenter.kt

                ),
            )
+            // By default select all rooms
+            selectedRoomIds.value = rooms.map { it.roomId }.toSet()


Should this be an immutable set so it won't trigger unnecessary recompositions? As in using ImmutableSet instead of Set for the value.

sonarqubecloud · 2025-09-26T14:38:25Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

bmarty requested a review from a team as a code owner September 15, 2025 14:25

bmarty added the PR-Wip For anything that isn't ready to ship and will be enabled at a later date label Sep 15, 2025

bmarty requested review from jmartinesp and removed request for a team September 15, 2025 14:25

jmartinesp approved these changes Sep 15, 2025

View reviewed changes

bmarty marked this pull request as draft September 15, 2025 15:53

bmarty force-pushed the feature/bma/leaveSpace branch 2 times, most recently from 7406259 to a916661 Compare September 25, 2025 20:51

bmarty added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Sep 25, 2025

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Sep 25, 2025

bmarty added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Sep 26, 2025

bmarty changed the title ~~Leave space~~ Leave space - UI Sep 26, 2025

bmarty added 9 commits September 26, 2025 14:14

Leave space - Add screen to leave a space.

74c2ee4

Remove translations (key values have changed).

cbd591e

Translations will be back during the next Localazy sync.

Add the (Admin) info.

9732d55

Add unit test on LeaveSpaceState

de6f52c

Select all rooms by default

f503bc4

Update UI

5afe213

Update tests

b4232a8

Cleanup to be able to merge.

c1678b5

Metro now have @AssistedInject.

4d1a503

bmarty force-pushed the feature/bma/leaveSpace branch from 5f88777 to 4d1a503 Compare September 26, 2025 12:19

bmarty added Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. and removed Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. labels Sep 26, 2025

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Sep 26, 2025

Update screenshots

6921117

bmarty requested a review from jmartinesp September 26, 2025 12:58

bmarty marked this pull request as ready for review September 26, 2025 13:46

Merge branch 'develop' into feature/bma/leaveSpace

0e3efaf

bmarty added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Sep 26, 2025

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Sep 26, 2025

jmartinesp reviewed Sep 26, 2025

View reviewed changes

ElementBot and others added 2 commits September 26, 2025 14:01

Update screenshots

c78aa7c

Address review comments.

ef36922

bmarty enabled auto-merge September 26, 2025 14:29

bmarty merged commit 59df60a into develop Sep 26, 2025
28 of 29 checks passed

bmarty deleted the feature/bma/leaveSpace branch September 26, 2025 14:58

		data object StartLeaveSpace : SpaceEvents
		data object LeaveSpace : SpaceEvents

Leave space - UI #5354

Leave space - UI #5354

Uh oh!

Conversation

bmarty commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

Uh oh!

github-actions bot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jmartinesp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bmarty Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ElementBot commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bmarty commented Sep 26, 2025

Uh oh!

jmartinesp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmartinesp Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Sep 26, 2025

Quality Gate passed

Uh oh!

Uh oh!

Uh oh!

bmarty commented Sep 15, 2025 •

edited

Loading

github-actions bot commented Sep 15, 2025 •

edited

Loading

codecov bot commented Sep 15, 2025 •

edited

Loading

bmarty Sep 15, 2025 •

edited

Loading

ElementBot commented Sep 25, 2025 •

edited

Loading

jmartinesp Sep 26, 2025 •

edited

Loading