Late review comments for entity widget configuration

app/src/main/kotlin/io/homeassistant/companion/android/widgets/entity/EntityWidgetConfigureScreen.kt

@@ -255,7 +255,6 @@ private fun ServerSelector(
         label = stringResource(commonR.string.server_select),
         placeholder = stringResource(commonR.string.server_select),
         modifier = Modifier.formControlWidth(),
-        enabled = items.isNotEmpty(),
     )
 }
 

app/src/main/kotlin/io/homeassistant/companion/android/widgets/entity/EntityWidgetConfigureState.kt

@@ -12,6 +12,8 @@ import io.homeassistant.companion.android.common.data.servers.ServerManager
 import io.homeassistant.companion.android.database.widget.WidgetBackgroundType
 import io.homeassistant.companion.android.database.widget.WidgetTapAction
 
+private const val DEFAULT_TEXT_SIZE = "30"
+
 @Stable
 internal data class EntityWidgetConfigureState(
     val selectedServerId: Int = ServerManager.SERVER_ID_ACTIVE,
@@ -65,5 +67,3 @@ internal data class EntityWidgetConfigureState(
         entityDisplayState = EntityDisplayState.Loading,
     )
 }
-
-internal const val DEFAULT_TEXT_SIZE = "30"

app/src/main/kotlin/io/homeassistant/companion/android/widgets/entity/EntityWidgetConfigureViewModel.kt

@@ -89,42 +89,6 @@ class EntityWidgetConfigureViewModel @AssistedInject constructor(
         }
     }
 
-    /**
-     * Restores the configuration of an existing widget, or falls back to the active server for a new one.
-     */
-    private suspend fun restoreConfiguration() {
-        val widget = if (widgetId != AppWidgetManager.INVALID_APPWIDGET_ID && _state.value.selectedEntityId == null) {
-            staticWidgetDao.get(widgetId)
-        } else {
-            null
-        }
-
-        if (widget != null) {
-            _state.update {
-                it.copy(
-                    selectedServerId = widget.serverId,
-                    selectedEntityId = widget.entityId,
-                    selectedAttributeIds = widget.attributeIds.toAttributeIdsList(),
-                    label = widget.label.orEmpty(),
-                    textSize = widget.textSize.toInt().toString(),
-                    stateSeparator = widget.stateSeparator,
-                    attributeSeparator = widget.attributeSeparator,
-                    selectedTapAction = widget.tapAction,
-                    selectedBackgroundType = widget.backgroundType,
-                    textColorHex = widget.textColor,
-                    isUpdateWidget = true,
-                )
-            }
-        } else {
-            _state.update {
-                it.copy(selectedServerId = serverManager.getServer()?.id ?: ServerManager.SERVER_ID_ACTIVE)
-            }
-        }
-
-        loadEntities(_state.value.selectedServerId)
-        loadAttributes(_state.value.selectedEntityId)
-    }
-
     fun onServerSelected(serverId: Int) {
         if (serverId == _state.value.selectedServerId) return
 
@@ -207,10 +171,135 @@ class EntityWidgetConfigureViewModel @AssistedInject constructor(
         _state.update { it.copy(selectedBackgroundType = backgroundType) }
     }
 
-    internal fun onTextColorSelected(colorHex: String) {
+    fun onTextColorSelected(colorHex: String) {
         _state.update { it.copy(textColorHex = colorHex) }
     }
 
+    /**
+     * Persists the current configuration, reporting through [errors] and returning false when it
+     * cannot be saved.
+     */
+    suspend fun updateWidgetConfiguration(): Boolean {
+        if (widgetId == AppWidgetManager.INVALID_APPWIDGET_ID) {
+            Timber.e("Cannot save the widget configuration, the widget ID is invalid")
+            _errors.emit(commonR.string.widget_update_error)
+            return false
+        }
+        val widget = getPendingDaoEntity()
+        if (widget == null) {
+            _errors.emit(commonR.string.widget_update_error)
+            return false
+        }
+
+        staticWidgetDao.add(widget)
+        return true
+    }
+
+    /** Asks the already placed widgets to redraw with the configuration that was just saved. */
+    fun updateWidget(context: Context) {
+        context.sendBroadcast(
+            Intent(context, EntityWidget::class.java).apply {
+                action = BaseWidgetProvider.UPDATE_WIDGETS
+            },
+        )
+    }
+
+    /**
+     * Asks the launcher to pin the configured widget and suspends until it is added, reporting
+     * through [errors] and returning false when the widget cannot be requested at all.
+     */
+    @SuppressLint("NewApi") // The API 26 requirement is checked below before touching the pinning APIs.
+    suspend fun requestWidgetCreation(context: Context): Boolean {
+        if (!SdkVersion.isAtLeast(Build.VERSION_CODES.O)) {
+            Timber.e("Cannot pin the widget, pinning requires API ${Build.VERSION_CODES.O}")
+            _errors.emit(commonR.string.widget_creation_error)
+            return false
+        }
+
+        val appWidgetManager = AppWidgetManager.getInstance(context)
+        val pinningSupported = try {
+            appWidgetManager.isRequestPinAppWidgetSupported
+        } catch (e: RemoteException) {
+            Timber.e(e, "Unable to read isRequestPinAppWidgetSupported")
+            false
+        }
+        if (!pinningSupported) {
+            Timber.e("Cannot pin the widget, the launcher does not support it")
+            _errors.emit(commonR.string.widget_creation_error)
+            return false
+        }
+
+        val widget = getPendingDaoEntity()
+        if (widget == null) {
+            _errors.emit(commonR.string.widget_creation_error)
+            return false
+        }
+
+        var requestAccepted = false
+        staticWidgetDao.getWidgetCountFlow()
+            // We drop the first value since we only care about knowing when the widget is actually added
+            .drop(1)
+            .onStart {
+                requestAccepted = appWidgetManager.requestPinAppWidget(
+                    ComponentName(context, EntityWidget::class.java),
+                    null,
+                    PendingIntent.getBroadcast(
+                        context,
+                        System.currentTimeMillis().toInt(),
+                        Intent(context, EntityWidget::class.java).apply {
+                            action = ACTION_APPWIDGET_CREATED
+                            putExtra(EXTRA_WIDGET_ENTITY, widget)
+                        },
+                        PendingIntent.FLAG_MUTABLE,
+                    ),
+                )
+                // A rejected request never adds a widget, so emit to stop waiting for one
+                if (!requestAccepted) emit(0)
+            }.first()
+
+        if (!requestAccepted) {
+            Timber.e("The launcher rejected the widget pin request")
+            _errors.emit(commonR.string.widget_creation_error)
+        }
+        return requestAccepted
+    }
+
+    /**
+     * Restores the configuration of an existing widget, or falls back to the active server for a new one.
+     */
+    private suspend fun restoreConfiguration() {
+        val widget = if (widgetId != AppWidgetManager.INVALID_APPWIDGET_ID && _state.value.selectedEntityId == null) {
+            staticWidgetDao.get(widgetId)
+        } else {
+            null
+        }
+
+        if (widget != null) {
+            _state.update {
+                it.copy(
+                    selectedServerId = widget.serverId,
+                    selectedEntityId = widget.entityId,
+                    selectedAttributeIds = widget.attributeIds.toAttributeIdsList(),
+                    label = widget.label.orEmpty(),
+                    textSize = widget.textSize.toInt().toString(),
+                    stateSeparator = widget.stateSeparator,
+                    attributeSeparator = widget.attributeSeparator,
+                    selectedTapAction = widget.tapAction,
+                    selectedBackgroundType = widget.backgroundType,
+                    textColorHex = widget.textColor,
+                    isUpdateWidget = true,
+                )
+            }
+        } else {
+            _state.update {
+                it.copy(selectedServerId = serverManager.getServer()?.id ?: ServerManager.SERVER_ID_ACTIVE)
+            }
+        }
+
+        loadEntities(_state.value.selectedServerId)
+        loadAttributes(_state.value.selectedEntityId)
+    }
+
     private fun loadEntities(serverId: Int) {
         loadEntitiesJob?.cancel()
         loadEntitiesJob = viewModelScope.launch {
@@ -276,39 +365,10 @@ class EntityWidgetConfigureViewModel @AssistedInject constructor(
             current.selectedEntity != null
     }
 
-    /**
-     * Persists the current configuration, reporting through [errors] and returning false when it
-     * cannot be saved.
-     */
-    suspend fun updateWidgetConfiguration(): Boolean {
-        if (widgetId == AppWidgetManager.INVALID_APPWIDGET_ID) {
-            Timber.e("Cannot save the widget configuration, the widget ID is invalid")
-            _errors.emit(commonR.string.widget_update_error)
-            return false
-        }
-        val widget = getPendingDaoEntity()
-        if (widget == null) {
-            _errors.emit(commonR.string.widget_update_error)
-            return false
-        }
-
-        staticWidgetDao.add(widget)
-        return true
-    }
-
-    /** Asks the already placed widgets to redraw with the configuration that was just saved. */
-    fun updateWidget(context: Context) {
-        context.sendBroadcast(
-            Intent(context, EntityWidget::class.java).apply {
-                action = BaseWidgetProvider.UPDATE_WIDGETS
-            },
-        )
-    }
-
     /**
      * Builds the widget to persist from the current configuration, or null when it is incomplete.
      */
-    internal suspend fun getPendingDaoEntity(): StaticWidgetEntity? {
+    private suspend fun getPendingDaoEntity(): StaticWidgetEntity? {
         if (!isValidSelection()) {
             Timber.e("Cannot build the widget, the current configuration is invalid")
             return null
@@ -340,66 +400,6 @@ class EntityWidgetConfigureViewModel @AssistedInject constructor(
         )
     }
 
-    /**
-     * Asks the launcher to pin the configured widget and suspends until it is added, reporting
-     * through [errors] and returning false when the widget cannot be requested at all.
-     */
-    @SuppressLint("NewApi") // The API 26 requirement is checked below before touching the pinning APIs.
-    suspend fun requestWidgetCreation(context: Context): Boolean {
-        if (!SdkVersion.isAtLeast(Build.VERSION_CODES.O)) {
-            Timber.e("Cannot pin the widget, pinning requires API ${Build.VERSION_CODES.O}")
-            _errors.emit(commonR.string.widget_creation_error)
-            return false
-        }
-
-        val appWidgetManager = AppWidgetManager.getInstance(context)
-        val pinningSupported = try {
-            appWidgetManager.isRequestPinAppWidgetSupported
-        } catch (e: RemoteException) {
-            Timber.e(e, "Unable to read isRequestPinAppWidgetSupported")
-            false
-        }
-        if (!pinningSupported) {
-            Timber.e("Cannot pin the widget, the launcher does not support it")
-            _errors.emit(commonR.string.widget_creation_error)
-            return false
-        }
-
-        val widget = getPendingDaoEntity()
-        if (widget == null) {
-            _errors.emit(commonR.string.widget_creation_error)
-            return false
-        }
-
-        var requestAccepted = false
-        staticWidgetDao.getWidgetCountFlow()
-            // We drop the first value since we only care about knowing when the widget is actually added
-            .drop(1)
-            .onStart {
-                requestAccepted = appWidgetManager.requestPinAppWidget(
-                    ComponentName(context, EntityWidget::class.java),
-                    null,
-                    PendingIntent.getBroadcast(
-                        context,
-                        System.currentTimeMillis().toInt(),
-                        Intent(context, EntityWidget::class.java).apply {
-                            action = ACTION_APPWIDGET_CREATED
-                            putExtra(EXTRA_WIDGET_ENTITY, widget)
-                        },
-                        PendingIntent.FLAG_MUTABLE,
-                    ),
-                )
-                // A rejected request never adds a widget, so emit to stop waiting for one
-                if (!requestAccepted) emit(0)
-            }.first()
-
-        if (!requestAccepted) {
-            Timber.e("The launcher rejected the widget pin request")
-            _errors.emit(commonR.string.widget_creation_error)
-        }
-        return requestAccepted
-    }
-
     @AssistedFactory
     interface Factory {
         fun create(widgetId: Int, preselectedEntityId: String?): EntityWidgetConfigureViewModel

app/src/screenshotTest/kotlin/io/homeassistant/companion/android/widgets/entity/EntityWidgetConfigureScreenshotTest.kt

@@ -24,7 +24,7 @@ class EntityWidgetConfigureScreenshotTest {
     fun `EntityWidgetConfigureContent selected entity`() {
         HAThemeForPreview {
             EntityWidgetConfigureContent(
-                state = previewEntityWidgetConfigureState,
+                state = previewConfigureState,
                 snackbarHostState = remember { SnackbarHostState() },
                 canNavigateBack = false,
                 onNavigate = {},
@@ -52,7 +52,10 @@ class EntityWidgetConfigureScreenshotTest {
     fun `EntityWidgetConfigureContent no selected entity`() {
         HAThemeForPreview {
             EntityWidgetConfigureContent(
-                state = previewEntityWidgetConfigureState.copy(selectedEntityId = null),
+                state = previewConfigureState.copy(
+                    serversDropdownItems = previewConfigureState.serversDropdownItems.take(1),
+                    selectedEntityId = null,
+                ),
                 snackbarHostState = remember { SnackbarHostState() },
                 canNavigateBack = false,
                 onNavigate = {},
@@ -75,7 +78,7 @@ class EntityWidgetConfigureScreenshotTest {
     }
 }
 
-private val previewEntityWidgetConfigureState = EntityWidgetConfigureState(
+private val previewConfigureState = EntityWidgetConfigureState(
     selectedServerId = previewServer1.id,
     serversDropdownItems = listOf(previewServer1, previewServer2).map {
         HADropdownItem(key = it.id, label = it.friendlyName)

app/src/test/kotlin/io/homeassistant/companion/android/widgets/entity/EntityWidgetConfigureViewModelTest.kt

@@ -32,6 +32,9 @@ import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.assertNull
 import org.junit.jupiter.api.extension.ExtendWith
 
+/** Hex of `colorWidgetButtonLabelBlack`, which is what the widget persists. */
+private const val BLACK_HEX = "#3A3A3A"
+
 @OptIn(ExperimentalCoroutinesApi::class)
 @ExtendWith(MainDispatcherJUnit5Extension::class)
 class EntityWidgetConfigureViewModelTest {
@@ -245,21 +248,16 @@ class EntityWidgetConfigureViewModelTest {
         textColor = BLACK_HEX,
     )
 
-    companion object {
-        /** Hex of `colorWidgetButtonLabelBlack`, which is what the widget persists. */
-        private const val BLACK_HEX = "#3A3A3A"
-
-        private fun displayStateOf(vararg items: EntityDisplayWithContext) = EntityDisplayState.Loaded(items.toList())
+    private fun createEntity(entityId: String, attributes: Map<String, Any?>) = Entity(
+        entityId = entityId,
+        state = "on",
+        attributes = attributes,
+        lastChanged = LocalDateTime.MIN,
+        lastUpdated = LocalDateTime.MIN,
+    )
 
-        /** Display name comes from the entity registry in production, so it is set explicitly here. */
-        private fun Entity.toDisplayItem(name: String) = EntityDisplayWithContext(EntityDisplayWithoutContext(this, name = name))
+    private fun displayStateOf(vararg items: EntityDisplayWithContext) = EntityDisplayState.Loaded(items.toList())
 
-        private fun createEntity(entityId: String, attributes: Map<String, Any?>) = Entity(
-            entityId = entityId,
-            state = "on",
-            attributes = attributes,
-            lastChanged = LocalDateTime.MIN,
-            lastUpdated = LocalDateTime.MIN,
-        )
-    }
+    /** Display name comes from the entity registry in production, so it is set explicitly here. */
+    private fun Entity.toDisplayItem(name: String) = EntityDisplayWithContext(EntityDisplayWithoutContext(this, name = name))
 }