From 82391aa28128cacf9b67572fcdd58a75cf7f275e Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Wed, 13 Jul 2022 13:48:54 +0200 Subject: [PATCH 1/4] Replacing ViewEvent by a ViewState property --- .../home/room/detail/RoomDetailViewEvents.kt | 2 -- .../home/room/detail/RoomDetailViewState.kt | 3 ++- .../features/home/room/detail/TimelineFragment.kt | 14 +++++++------- .../features/home/room/detail/TimelineViewModel.kt | 4 ++-- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewEvents.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewEvents.kt index 4d57647a1d..dcfee2d919 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewEvents.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewEvents.kt @@ -84,6 +84,4 @@ sealed class RoomDetailViewEvents : VectorViewEvents { data class StartChatEffect(val type: ChatEffect) : RoomDetailViewEvents() object StopChatEffects : RoomDetailViewEvents() object RoomReplacementStarted : RoomDetailViewEvents() - - data class ChangeLocationIndicator(val isVisible: Boolean) : RoomDetailViewEvents() } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewState.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewState.kt index 3ec7166739..8500d1ed96 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewState.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/RoomDetailViewState.kt @@ -75,7 +75,8 @@ data class RoomDetailViewState( val switchToParentSpace: Boolean = false, val rootThreadEventId: String? = null, val threadNotificationBadgeState: ThreadNotificationBadgeState = ThreadNotificationBadgeState(), - val typingUsers: List? = null + val typingUsers: List? = null, + val isSharingLiveLocation: Boolean = false, ) : MavericksState { constructor(args: TimelineArgs) : this( diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt index e86a7fe227..31c1004ef9 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt @@ -498,7 +498,6 @@ class TimelineFragment @Inject constructor( RoomDetailViewEvents.StopChatEffects -> handleStopChatEffects() is RoomDetailViewEvents.DisplayAndAcceptCall -> acceptIncomingCall(it) RoomDetailViewEvents.RoomReplacementStarted -> handleRoomReplacement() - is RoomDetailViewEvents.ChangeLocationIndicator -> handleChangeLocationIndicator(it) } } @@ -663,10 +662,6 @@ class TimelineFragment @Inject constructor( ) } - private fun handleChangeLocationIndicator(event: RoomDetailViewEvents.ChangeLocationIndicator) { - views.locationLiveStatusIndicator.isVisible = event.isVisible - } - private fun displayErrorMessage(error: RoomDetailViewEvents.Failure) { if (error.showInDialog) displayErrorDialog(error.throwable) else showErrorInSnackbar(error.throwable) } @@ -1686,6 +1681,11 @@ class TimelineFragment @Inject constructor( } else if (mainState.asyncInviter.complete) { vectorBaseActivity.finish() } + updateLiveLocationIndicator(mainState.isSharingLiveLocation) + } + + private fun updateLiveLocationIndicator(isSharingLiveLocation: Boolean) { + views.locationLiveStatusIndicator.isVisible = isSharingLiveLocation } private fun FragmentTimelineBinding.hideComposerViews() { @@ -1706,7 +1706,7 @@ class TimelineFragment @Inject constructor( private fun renderToolbar(roomSummary: RoomSummary?) { when { - isLocalRoom() -> { + isLocalRoom() -> { views.includeRoomToolbar.roomToolbarContentView.isVisible = false views.includeThreadToolbar.roomToolbarThreadConstraintLayout.isVisible = false setupToolbar(views.roomToolbar) @@ -1724,7 +1724,7 @@ class TimelineFragment @Inject constructor( } views.includeThreadToolbar.roomToolbarThreadTitleTextView.text = resources.getText(R.string.thread_timeline_title) } - else -> { + else -> { views.includeRoomToolbar.roomToolbarContentView.isVisible = true views.includeThreadToolbar.roomToolbarThreadConstraintLayout.isVisible = false if (roomSummary == null) { diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt index e305ccbec1..1d2b778764 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt @@ -1295,11 +1295,11 @@ class TimelineViewModel @AssistedInject constructor( } override fun onLocationServiceRunning() { - _viewEvents.post(RoomDetailViewEvents.ChangeLocationIndicator(isVisible = true)) + setState { copy(isSharingLiveLocation = true) } } override fun onLocationServiceStopped() { - _viewEvents.post(RoomDetailViewEvents.ChangeLocationIndicator(isVisible = false)) + setState { copy(isSharingLiveLocation = false) } // Bind again in case user decides to share live location without leaving the room locationSharingServiceConnection.bind(this) } From d2d24cbcbeaae44d3372ad04d8ed9dbea42fdb70 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Wed, 13 Jul 2022 13:58:21 +0200 Subject: [PATCH 2/4] Adding a changelog entry --- changelog.d/6537.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6537.bugfix diff --git a/changelog.d/6537.bugfix b/changelog.d/6537.bugfix new file mode 100644 index 0000000000..688fd5104c --- /dev/null +++ b/changelog.d/6537.bugfix @@ -0,0 +1 @@ +[Location Share] - Wrong room live location status bar visibility in timeline From 33714b850f5d22ccea6d7eeed31def34400322ad Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Wed, 13 Jul 2022 14:42:48 +0200 Subject: [PATCH 3/4] Make the status bar only visible in rooms where there is an active live --- .../home/room/detail/TimelineViewModel.kt | 4 ++-- .../location/LocationSharingAndroidService.kt | 7 +++++++ .../LocationSharingServiceConnection.kt | 17 ++++++++++++++--- .../live/map/LocationLiveMapViewModel.kt | 2 +- 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt index 1d2b778764..8075ba5b7a 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineViewModel.kt @@ -1294,8 +1294,8 @@ class TimelineViewModel @AssistedInject constructor( _viewEvents.post(RoomDetailViewEvents.OnNewTimelineEvents(eventIds)) } - override fun onLocationServiceRunning() { - setState { copy(isSharingLiveLocation = true) } + override fun onLocationServiceRunning(roomIds: Set) { + setState { copy(isSharingLiveLocation = roomId in roomIds) } } override fun onLocationServiceStopped() { diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidService.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidService.kt index 69ffc0e89e..6fd7f27ece 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidService.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidService.kt @@ -193,11 +193,13 @@ class LocationSharingAndroidService : VectorAndroidService(), LocationTracker.Ca private fun addRoomArgs(beaconEventId: String, roomArgs: RoomArgs) { Timber.i("adding roomArgs for beaconEventId: $beaconEventId") roomArgsMap[beaconEventId] = roomArgs + callback?.onRoomIdsUpdate(getRoomIdsOfActiveLives()) } private fun removeRoomArgs(beaconEventId: String) { Timber.i("removing roomArgs for beaconEventId: $beaconEventId") roomArgsMap.remove(beaconEventId) + callback?.onRoomIdsUpdate(getRoomIdsOfActiveLives()) } private fun listenForLiveSummaryChanges(roomId: String, beaconEventId: String) { @@ -220,6 +222,10 @@ class LocationSharingAndroidService : VectorAndroidService(), LocationTracker.Ca ) } + fun getRoomIdsOfActiveLives(): Set { + return roomArgsMap.map { it.value.roomId }.toSet() + } + override fun onBind(intent: Intent?): IBinder { return binder } @@ -229,6 +235,7 @@ class LocationSharingAndroidService : VectorAndroidService(), LocationTracker.Ca } interface Callback { + fun onRoomIdsUpdate(roomIds: Set) fun onServiceError(error: Throwable) } diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt index 495b780188..9feffb7554 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt @@ -31,7 +31,7 @@ class LocationSharingServiceConnection @Inject constructor( LocationSharingAndroidService.Callback { interface Callback { - fun onLocationServiceRunning() + fun onLocationServiceRunning(roomIds: Set) fun onLocationServiceStopped() fun onLocationServiceError(error: Throwable) } @@ -44,7 +44,7 @@ class LocationSharingServiceConnection @Inject constructor( addCallback(callback) if (isBound) { - callback.onLocationServiceRunning() + callback.onLocationServiceRunning(getRoomIdsOfActiveLives()) } else { Intent(context, LocationSharingAndroidService::class.java).also { intent -> context.bindService(intent, this, 0) @@ -56,12 +56,15 @@ class LocationSharingServiceConnection @Inject constructor( removeCallback(callback) } + private fun getRoomIdsOfActiveLives(): Set { + return locationSharingAndroidService?.getRoomIdsOfActiveLives() ?: emptySet() + } + override fun onServiceConnected(className: ComponentName, binder: IBinder) { locationSharingAndroidService = (binder as LocationSharingAndroidService.LocalBinder).getService().also { it.callback = this } isBound = true - onCallbackActionNoArg(Callback::onLocationServiceRunning) } override fun onServiceDisconnected(className: ComponentName) { @@ -71,6 +74,10 @@ class LocationSharingServiceConnection @Inject constructor( onCallbackActionNoArg(Callback::onLocationServiceStopped) } + override fun onRoomIdsUpdate(roomIds: Set) { + forwardRoomIdsToCallbacks(roomIds) + } + override fun onServiceError(error: Throwable) { forwardErrorToCallbacks(error) } @@ -87,6 +94,10 @@ class LocationSharingServiceConnection @Inject constructor( callbacks.toList().forEach(action) } + private fun forwardRoomIdsToCallbacks(roomIds: Set) { + callbacks.toList().forEach { it.onLocationServiceRunning(roomIds) } + } + private fun forwardErrorToCallbacks(error: Throwable) { callbacks.toList().forEach { it.onLocationServiceError(error) } } diff --git a/vector/src/main/java/im/vector/app/features/location/live/map/LocationLiveMapViewModel.kt b/vector/src/main/java/im/vector/app/features/location/live/map/LocationLiveMapViewModel.kt index 15c76b083e..44d39862f9 100644 --- a/vector/src/main/java/im/vector/app/features/location/live/map/LocationLiveMapViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/location/live/map/LocationLiveMapViewModel.kt @@ -87,7 +87,7 @@ class LocationLiveMapViewModel @AssistedInject constructor( } } - override fun onLocationServiceRunning() { + override fun onLocationServiceRunning(roomIds: Set) { // NOOP } From ecbd2d48a71cc0e6d91a1ba052cde915ad9aaec4 Mon Sep 17 00:00:00 2001 From: Maxime NATUREL Date: Tue, 19 Jul 2022 14:43:13 +0200 Subject: [PATCH 4/4] Replacing callback by a SharedFlow to notify of roomIds updates --- .../location/LocationSharingAndroidService.kt | 10 ++++--- .../LocationSharingServiceConnection.kt | 26 ++++++++++++++----- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidService.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidService.kt index 6fd7f27ece..f3be651ea3 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidService.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingAndroidService.kt @@ -29,6 +29,8 @@ import im.vector.app.features.notifications.NotificationUtils import im.vector.app.features.session.coroutineScope import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.distinctUntilChangedBy import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.launchIn @@ -66,6 +68,9 @@ class LocationSharingAndroidService : VectorAndroidService(), LocationTracker.Ca private val jobs = mutableListOf() private var startInProgress = false + private val _roomIdsOfActiveLives = MutableSharedFlow>(replay = 1) + val roomIdsOfActiveLives = _roomIdsOfActiveLives.asSharedFlow() + override fun onCreate() { super.onCreate() Timber.i("onCreate") @@ -193,13 +198,13 @@ class LocationSharingAndroidService : VectorAndroidService(), LocationTracker.Ca private fun addRoomArgs(beaconEventId: String, roomArgs: RoomArgs) { Timber.i("adding roomArgs for beaconEventId: $beaconEventId") roomArgsMap[beaconEventId] = roomArgs - callback?.onRoomIdsUpdate(getRoomIdsOfActiveLives()) + launchWithActiveSession { _roomIdsOfActiveLives.emit(getRoomIdsOfActiveLives()) } } private fun removeRoomArgs(beaconEventId: String) { Timber.i("removing roomArgs for beaconEventId: $beaconEventId") roomArgsMap.remove(beaconEventId) - callback?.onRoomIdsUpdate(getRoomIdsOfActiveLives()) + launchWithActiveSession { _roomIdsOfActiveLives.emit(getRoomIdsOfActiveLives()) } } private fun listenForLiveSummaryChanges(roomId: String, beaconEventId: String) { @@ -235,7 +240,6 @@ class LocationSharingAndroidService : VectorAndroidService(), LocationTracker.Ca } interface Callback { - fun onRoomIdsUpdate(roomIds: Set) fun onServiceError(error: Throwable) } diff --git a/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt b/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt index 9feffb7554..3be73e9fd4 100644 --- a/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt +++ b/vector/src/main/java/im/vector/app/features/location/LocationSharingServiceConnection.kt @@ -21,14 +21,19 @@ import android.content.Context import android.content.Intent import android.content.ServiceConnection import android.os.IBinder +import im.vector.app.core.di.ActiveSessionHolder +import im.vector.app.features.session.coroutineScope +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import javax.inject.Inject import javax.inject.Singleton @Singleton class LocationSharingServiceConnection @Inject constructor( - private val context: Context -) : ServiceConnection, - LocationSharingAndroidService.Callback { + private val context: Context, + private val activeSessionHolder: ActiveSessionHolder +) : ServiceConnection, LocationSharingAndroidService.Callback { interface Callback { fun onLocationServiceRunning(roomIds: Set) @@ -61,12 +66,21 @@ class LocationSharingServiceConnection @Inject constructor( } override fun onServiceConnected(className: ComponentName, binder: IBinder) { - locationSharingAndroidService = (binder as LocationSharingAndroidService.LocalBinder).getService().also { - it.callback = this + locationSharingAndroidService = (binder as LocationSharingAndroidService.LocalBinder).getService().also { service -> + service.callback = this + getActiveSessionCoroutineScope()?.let { scope -> + service.roomIdsOfActiveLives + .onEach(::onRoomIdsUpdate) + .launchIn(scope) + } } isBound = true } + private fun getActiveSessionCoroutineScope(): CoroutineScope? { + return activeSessionHolder.getSafeActiveSession()?.coroutineScope + } + override fun onServiceDisconnected(className: ComponentName) { isBound = false locationSharingAndroidService?.callback = null @@ -74,7 +88,7 @@ class LocationSharingServiceConnection @Inject constructor( onCallbackActionNoArg(Callback::onLocationServiceStopped) } - override fun onRoomIdsUpdate(roomIds: Set) { + private fun onRoomIdsUpdate(roomIds: Set) { forwardRoomIdsToCallbacks(roomIds) }