Improve room setting edition #2849
Conversation
It simplifies a bit the codebase.
|
📱 Scan the QR code below to install the build (arm64 only) for this PR. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2849 +/- ##
===========================================
+ Coverage 73.99% 74.06% +0.07%
===========================================
Files 1531 1530 -1
Lines 36526 36539 +13
Branches 7065 7054 -11
===========================================
+ Hits 27028 27064 +36
+ Misses 5796 5788 -8
+ Partials 3702 3687 -15 ☔ View full report in Codecov by Sentry. |
Also correctly handle the back press when this bottom sheet is opened, previously it was leaving the room edition screen. ModalBottomSheetLayout can now be deleted.
| val roomAvatarUri = room.avatarUrl()?.toUri() | ||
| var roomAvatarUriEdited by rememberSaveable { mutableStateOf<Uri?>(null) } | ||
| LaunchedEffect(roomAvatarUri) { | ||
| // Every time the roomAvatar change (from sync), we can set the new avatar. |
There was a problem hiding this comment.
Do we really want to have live values here? I'd expect to be static
There was a problem hiding this comment.
Yes, I think it's fine, having the value change during edition should not happen a lot, just during our cheeky tests :)
| roomSyncUpdateFlow.value, | ||
| roomName, | ||
| roomTopic, | ||
| roomRawNameTrimmed, |
There was a problem hiding this comment.
This is a bit weird to use remember with keys used in derivatedStateOf
There was a problem hiding this comment.
Yes, I tried to remove the keys, but this does not work as expected, for instance roomRawNameTrimmed is always empty.
There was a problem hiding this comment.
Because roomRawNameTrimmed is not backed by a compose state field.
But then I think the derivedStateOf doesn't make sense
There was a problem hiding this comment.
OK, so I have a mix in c3caff0. I think it's still relevant to use de derivedStateOf in this case.
bmarty
added
the
Record-Screenshots
github-actions
bot
removed
the
Record-Screenshots
| @Composable | ||
| fun AvatarActionBottomSheet( | ||
| actions: ImmutableList<AvatarAction>, | ||
| modalBottomSheetState: ModalBottomSheetState, | ||
| isVisible: MutableState<Boolean>, |
There was a problem hiding this comment.
Nice catch ElementBot! Please use a plain Boolean and a lambda parameter to update it outside. The isVisible should probably be hold in the relevant Presenter states
|
bmarty
added
the
Run-Maestro
github-actions
bot
removed
the
Run-Maestro
bmarty
added
the
Run-Maestro
github-actions
bot
removed
the
Run-Maestro
Type of change
Content
May be reviewed commit per commit, first commits are cleanup.
Ensure that when editing room setting, the actual
m.room.namevalue is used to fill the form, and not the computed name. Closes #2844, but there is an issue SDK side, rawName contains the displayName. It's discussed internally:Also make the room details (topic/name/avatar) live in this screen, in case it's changes by someone else, or by another session.
Motivation and context
Screenshots / GIFs
Tests
Tested devices
Checklist