Address review comments

element-hq · Apr 15, 2023 · 3dbc173 · 3dbc173
1 parent 3171a5f
commit 3dbc173
Show file tree

Hide file tree

Showing 8 changed files with 92 additions and 73 deletions.
diff --git a/...sages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt b/...sages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt
@@ -69,7 +69,8 @@ class MessagesPresenter @Inject constructor(
             mutableStateOf(null)
         }
 
-        val networkConnectionStatus by networkMonitor.connectivity.collectAsState(initial = NetworkStatus.Online)
+        val networkConnectionStatus by networkMonitor.connectivity.collectAsState(initial = networkMonitor.currentConnectivityStatus)
+        println(networkConnectionStatus)
 
         LaunchedEffect(syncUpdateFlow) {
             roomAvatar.value =

diff --git a/...itor/api/src/main/kotlin/io/element/android/features/networkmonitor/api/NetworkMonitor.kt b/...itor/api/src/main/kotlin/io/element/android/features/networkmonitor/api/NetworkMonitor.kt
@@ -20,4 +20,5 @@ import kotlinx.coroutines.flow.Flow
 
 interface NetworkMonitor {
     val connectivity: Flow<NetworkStatus>
+    val currentConnectivityStatus: NetworkStatus
 }
diff --git a/...ain/kotlin/io/element/android/features/networkmonitor/api/ui/ConnectivityIndicatorView.kt b/...ain/kotlin/io/element/android/features/networkmonitor/api/ui/ConnectivityIndicatorView.kt
@@ -17,7 +17,10 @@
 package io.element.android.features.networkmonitor.api.ui
 
 import androidx.compose.animation.AnimatedVisibility
+import androidx.compose.animation.core.MutableTransitionState
 import androidx.compose.animation.expandVertically
+import androidx.compose.animation.fadeIn
+import androidx.compose.animation.fadeOut
 import androidx.compose.animation.shrinkVertically
 import androidx.compose.foundation.Image
 import androidx.compose.foundation.background
@@ -34,6 +37,10 @@ import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.outlined.WifiOff
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
+import androidx.compose.runtime.setValue
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.ColorFilter
@@ -51,43 +58,56 @@ fun ConnectivityIndicatorView(
     isOnline: Boolean,
     modifier: Modifier = Modifier
 ) {
+    val isIndicatorVisible = remember { MutableTransitionState(!isOnline) }.apply { targetState = !isOnline }
+    val isStatusBarPaddingVisible = remember { MutableTransitionState(isOnline) }.apply { targetState = isOnline }
+
     // Display the network indicator with an animation
     AnimatedVisibility(
-        visible = !isOnline,
-        enter = expandVertically(),
-        exit = shrinkVertically(),
+        visibleState = isIndicatorVisible,
+        enter = fadeIn() + expandVertically(),
+        exit = fadeOut() + shrinkVertically(),
     ) {
-        Row(
-            modifier
-                .fillMaxWidth()
-                .background(LocalColors.current.gray400)
-                .statusBarsPadding()
-                .padding(vertical = 6.dp),
-            horizontalArrangement = Arrangement.Center,
-            verticalAlignment = Alignment.Bottom,
-        ) {
-            val tint = MaterialTheme.colorScheme.primary
-            Image(
-                imageVector = Icons.Outlined.WifiOff,
-                contentDescription = null,
-                colorFilter = ColorFilter.tint(tint),
-                modifier = Modifier.size(16.dp),
-            )
-            Spacer(modifier = Modifier.width(8.dp))
-            Text(text = stringResource(StringR.string.common_offline), style = ElementTextStyles.Regular.bodyMD, color = tint)
-        }
+        Indicator(modifier)
     }
 
     // Show missing status bar padding when the indicator is not visible
     AnimatedVisibility(
-        visible = isOnline,
-        enter = expandVertically(),
-        exit = shrinkVertically(),
+        visibleState = isStatusBarPaddingVisible,
+        enter = fadeIn() + expandVertically(),
+        exit = fadeOut() + shrinkVertically(),
     ) {
-        Spacer(modifier = Modifier.statusBarsPadding())
+        StatusBarPaddingSpacer(modifier)
     }
 }
 
+@Composable
+private fun Indicator(modifier: Modifier = Modifier) {
+    Row(
+        modifier
+            .fillMaxWidth()
+            .background(LocalColors.current.gray400)
+            .statusBarsPadding()
+            .padding(vertical = 6.dp),
+        horizontalArrangement = Arrangement.Center,
+        verticalAlignment = Alignment.Bottom,
+    ) {
+        val tint = MaterialTheme.colorScheme.primary
+        Image(
+            imageVector = Icons.Outlined.WifiOff,
+            contentDescription = null,
+            colorFilter = ColorFilter.tint(tint),
+            modifier = Modifier.size(16.dp),
+        )
+        Spacer(modifier = Modifier.width(8.dp))
+        Text(text = stringResource(StringR.string.common_offline), style = ElementTextStyles.Regular.bodyMD, color = tint)
+    }
+}
+
+@Composable
+private fun StatusBarPaddingSpacer(modifier: Modifier = Modifier) {
+    Spacer(modifier = modifier.statusBarsPadding())
+}
+
 @Preview
 @Composable
 internal fun PreviewLightConnectivityIndicatorView() {

diff --git a/...mpl/src/main/kotlin/io/element/android/features/networkmonitor/impl/NetworkMonitorImpl.kt b/...mpl/src/main/kotlin/io/element/android/features/networkmonitor/impl/NetworkMonitorImpl.kt
@@ -22,67 +22,60 @@ import android.net.Network
 import android.net.NetworkCapabilities
 import android.net.NetworkRequest
 import com.squareup.anvil.annotations.ContributesBinding
+import io.element.android.features.networkmonitor.api.NetworkMonitor
+import io.element.android.features.networkmonitor.api.NetworkStatus
 import io.element.android.libraries.di.AppScope
 import io.element.android.libraries.di.ApplicationContext
 import io.element.android.libraries.di.SingleIn
-import io.element.android.features.networkmonitor.api.NetworkMonitor
-import io.element.android.features.networkmonitor.api.NetworkStatus
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
 import timber.log.Timber
 import javax.inject.Inject
 
-@ContributesBinding(scope = AppScope::class, boundType = NetworkMonitor::class)
+@ContributesBinding(scope = AppScope::class)
 @SingleIn(AppScope::class)
 class NetworkMonitorImpl @Inject constructor(
-    @ApplicationContext private val context: Context
+    @ApplicationContext context: Context
 ) : NetworkMonitor {
 
-    private val connectivityManager: ConnectivityManager by lazy {
-        context.getSystemService(ConnectivityManager::class.java)
-    }
+    private val connectivityManager: ConnectivityManager = context.getSystemService(ConnectivityManager::class.java)
 
-    private var callback: ConnectivityManager.NetworkCallback? = null
+    private val callback = object : ConnectivityManager.NetworkCallback() {
+        override fun onAvailable(network: Network) {
+            _connectivity.value = connectivityManager.currentConnectionStatus()
+            Timber.v("Connectivity status (available): ${connectivityManager.currentConnectionStatus()}")
+        }
 
-    private val _connectivity = MutableStateFlow(NetworkStatus.Online)
-    override val connectivity: Flow<NetworkStatus> = _connectivity
+        override fun onLost(network: Network) {
+            _connectivity.value = connectivityManager.currentConnectionStatus()
+            Timber.v("Connectivity status (lost): ${connectivityManager.currentConnectionStatus()}")
+        }
 
-    init {
-        subscribeToConnectionChanges()
+        override fun onCapabilitiesChanged(
+            network: Network,
+            networkCapabilities: NetworkCapabilities
+        ) {
+            _connectivity.value = connectivityManager.currentConnectionStatus()
+            Timber.v("Connectivity status (changed): ${connectivityManager.currentConnectionStatus()}")
+        }
     }
 
-    private fun subscribeToConnectionChanges() {
-        if (callback != null) return
-
-        val callback = object : ConnectivityManager.NetworkCallback() {
-            override fun onAvailable(network: Network) {
-                _connectivity.value = connectivityManager.currentConnectionStatus()
-                Timber.v("Connectivity status (available): ${connectivityManager.currentConnectionStatus()}")
-            }
+    private val _connectivity = MutableStateFlow(NetworkStatus.Online)
+    override val connectivity: Flow<NetworkStatus> = _connectivity
 
-            override fun onLost(network: Network) {
-                _connectivity.value = connectivityManager.currentConnectionStatus()
-                Timber.v("Connectivity status (lost): ${connectivityManager.currentConnectionStatus()}")
-            }
+    override val currentConnectivityStatus: NetworkStatus get() = _connectivity.value
 
-            override fun onCapabilitiesChanged(
-                network: Network,
-                networkCapabilities: NetworkCapabilities
-            ) {
-                _connectivity.value = connectivityManager.currentConnectionStatus()
-                Timber.v("Connectivity status (changed): ${connectivityManager.currentConnectionStatus()}")
-            }
-        }
-        this.callback = callback
+    init {
+        listenToConnectionChanges()
+    }
 
-        connectivityManager.registerNetworkCallback(
-            NetworkRequest.Builder()
-                .addTransportType(NetworkCapabilities.TRANSPORT_WIFI)
-                .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR)
-                .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
-                .build(),
-            callback
-        )
+    private fun listenToConnectionChanges() {
+        val request = NetworkRequest.Builder()
+            .addTransportType(NetworkCapabilities.TRANSPORT_WIFI)
+            .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR)
+            .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
+            .build()
+        connectivityManager.registerNetworkCallback(request, callback)
 
         _connectivity.tryEmit(connectivityManager.currentConnectionStatus())
     }

diff --git a/...est/src/main/kotlin/io/element/android/features/networkmonitor/test/FakeNetworkMonitor.kt b/...est/src/main/kotlin/io/element/android/features/networkmonitor/test/FakeNetworkMonitor.kt
@@ -19,6 +19,12 @@ package io.element.android.features.networkmonitor.test
 import io.element.android.features.networkmonitor.api.NetworkMonitor
 import io.element.android.features.networkmonitor.api.NetworkStatus
 import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.emptyFlow
+import kotlinx.coroutines.flow.MutableStateFlow
 
-class FakeNetworkMonitor(override val connectivity: Flow<NetworkStatus> = emptyFlow()) : NetworkMonitor
+class FakeNetworkMonitor(initialStatus: NetworkStatus = NetworkStatus.Online) : NetworkMonitor {
+    override val currentConnectivityStatus: NetworkStatus
+        get() = _connectivityStatus.value
+
+    private val _connectivityStatus: MutableStateFlow<NetworkStatus> = MutableStateFlow(initialStatus)
+    override val connectivity: Flow<NetworkStatus> = _connectivityStatus
+}
diff --git a/...mlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt b/...mlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt
@@ -74,7 +74,7 @@ class RoomListPresenter @Inject constructor(
             .roomSummaries()
             .collectAsState()
 
-        val networkConnectionStatus by networkMonitor.connectivity.collectAsState(initial = NetworkStatus.Online)
+        val networkConnectionStatus by networkMonitor.connectivity.collectAsState(initial = networkMonitor.currentConnectivityStatus)
 
         Timber.v("RoomSummaries size = ${roomSummaries.size}")
 

diff --git a/...s/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListView.kt b/...s/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListView.kt
@@ -159,7 +159,6 @@ fun RoomListContent(
                     onFilterChanged = { state.eventSink(RoomListEvents.UpdateFilter(it)) },
                     onOpenSettings = onOpenSettings,
                     scrollBehavior = scrollBehavior,
-                    useStatusBarInsets = state.hasNetworkConnection,
                 )
             }
         },

diff --git a/...pl/src/main/kotlin/io/element/android/features/roomlist/impl/components/RoomListTopBar.kt b/...pl/src/main/kotlin/io/element/android/features/roomlist/impl/components/RoomListTopBar.kt
@@ -73,7 +73,6 @@ fun RoomListTopBar(
     onFilterChanged: (String) -> Unit,
     onOpenSettings: () -> Unit,
     scrollBehavior: TopAppBarScrollBehavior,
-    useStatusBarInsets: Boolean,
     modifier: Modifier = Modifier,
 ) {
     LogCompositions(