Home always showing all rooms
#6666
Conversation
…lter param - we were passing null which meant no filter was being applied
ouchadam
added
the
Z-NextRelease
| @@ -144,7 +144,7 @@ class UnreadMessagesSharedViewModel @AssistedInject constructor( | |||
| this.memberships = listOf(Membership.JOIN) | |||
| this.spaceFilter = SpaceFilter.OrphanRooms.takeIf { | |||
| !vectorPreferences.prefSpacesShowAllRoomInHome() | |||
| } | |||
| } ?: SpaceFilter.NoFilter | |||
There was a problem hiding this comment.
In this case I think a if/else would be clearer:
if (vectorPreferences.prefSpacesShowAllRoomInHome()) {
SpaceFilter.NoFilter
} else {
SpaceFilter.OrphanRooms
}There was a problem hiding this comment.
agreed, will update 👍
| @@ -100,7 +100,7 @@ class SpaceListViewModel @AssistedInject constructor( | |||
| this.memberships = listOf(Membership.JOIN) | |||
| this.spaceFilter = SpaceFilter.OrphanRooms.takeIf { | |||
| !vectorPreferences.prefSpacesShowAllRoomInHome() | |||
| } | |||
| } ?: SpaceFilter.NoFilter | |||
| @@ -117,7 +117,7 @@ class SpaceListViewModel @AssistedInject constructor( | |||
| this.memberships = listOf(Membership.JOIN) | |||
| this.spaceFilter = SpaceFilter.OrphanRooms.takeIf { | |||
| !vectorPreferences.prefSpacesShowAllRoomInHome() | |||
| } | |||
| } ?: SpaceFilter.NoFilter | |||
| @@ -86,7 +86,7 @@ data class RoomSummaryQueryParams( | |||
| /** | |||
| * Used to filter room using the current space. | |||
| */ | |||
| val spaceFilter: SpaceFilter?, | |||
| val spaceFilter: SpaceFilter, | |||
There was a problem hiding this comment.
Maybe mention this API change in a file 6666.sdk
| @@ -106,13 +106,8 @@ class HomeRoomListViewModel @AssistedInject constructor( | |||
| } | |||
| .onEach { selectedSpaceOption -> | |||
| val selectedSpace = selectedSpaceOption.orNull() | |||
| val strategy = if (!vectorPreferences.prefSpacesShowAllRoomInHome()) { | |||
There was a problem hiding this comment.
cc @fedrunov this condition looked inverted and has been updated to match the usages in other screens
when {
vectorPreferences.prefSpacesShowAllRoomInHome() -> selectedSpaceId.toActiveSpaceOrNoFilter()
else -> selectedSpaceId.toActiveSpaceOrOrphanRooms()
}
| @@ -134,6 +130,11 @@ class SpaceListViewModel @AssistedInject constructor( | |||
| .launchIn(viewModelScope) | |||
| } | |||
|
|
|||
| private fun roomsInSpaceFilter() = when { | |||
There was a problem hiding this comment.
Please do not change it now, but the fun name is a bit strange to me.
Maybe something like getSpaceFilter() would be better.
On the fund, I do not really get why we have such filter here, since IIUC this ViewModel is used by the screen which displays only Spaces. So having orphans rooms here is strange to me.
There was a problem hiding this comment.
Ah, OK, it's to update the notification count. Please ignore me!
|
going to skip waiting for the instrumentation tests to complete so that the RC process can start |
|
SonarCloud Quality Gate failed. |
Type of change
Content
toActiveSpaceOrOrphanRoomsusages to avoid the nullsafe fallback, the extension already handles the null case for us (this is the fix!)SpaceFilterin favour of a dedicatedNoFiltertype, should help address the ambiguity of the different statesMotivation and context
Fixes #6665
Screenshots / GIFs
Tests
Show all rooms in Homepreference has no effectTested devices