Refactor ManageTilesView to Material3 by oungsi2000 · Pull Request #7052 · home-assistant/android

oungsi2000 · 2026-06-19T09:13:01Z

Summary

resolves #6301

Changes

Spinner-style DropdownMenu/ServerExposedDropdownMenu → HADropdownMenu
TextField → HATextField
Button/OutlinedButton/TextButton → HAFilledButton / HAPlainButton
Switch (with manual SwitchDefaults colors) → HASwitch
Divider → HAHorizontalDivider
M2 Scaffold/SnackbarHost → M3 equivalents; hardcoded sp/colorResource values replaced with HATextStyle / LocalHAColorScheme tokens
Bundled the display values into a single ManageTilesViewState holder to keep the composable signature small
Switched ManageTilesFragment from HomeAssistantAppTheme to HATheme

ViewModel is intentionally left untouched. State is extracted into ManageTilesViewState to prepare for a future migration, but refactoring the ViewModel itself felt out of scope for this PR, so it stays as-is.

Checklist

New or updated tests have been added to cover the changes following the testing guidelines.
The code follows the project's code style and best_practices.
The changes have been thoroughly tested, and edge cases have been considered.
Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Screenshots

Copilot

Pull request overview

Refactors the Quick Settings “Manage Tiles” UI to Material 3 and Home Assistant design-system Compose components, aligning the screen with issue #6301 and preparing for a future ViewModel migration by bundling UI inputs into a view-state holder.

Changes:

Migrated ManageTilesView from Material2 widgets to Material3 + HA design system components (dropdowns, text fields, switches, buttons, divider, scaffold/snackbar host)
Introduced ManageTilesViewState to keep the composable API smaller and decouple UI rendering from the ViewModel shape
Added screenshot tests for the “add tile” and “update tile” variants of the screen

Reviewed changes

Copilot reviewed 3 out of 17 changed files in this pull request and generated 3 comments.

File	Description
app/src/main/kotlin/io/homeassistant/companion/android/settings/qs/views/ManageTilesView.kt	Material3/HA component migration, state holder introduction, and new previews
app/src/main/kotlin/io/homeassistant/companion/android/settings/qs/ManageTilesFragment.kt	Switched the fragment Compose theme wrapper to `HATheme`
app/src/screenshotTest/kotlin/io/homeassistant/companion/android/settings/qs/views/ManageTilesViewScreenshotTest.kt	Added screenshot coverage for the refactored composable

TimoPtr · 2026-06-23T09:21:39Z

UX feedbacks before onto the code

First of all it does look way better than today 💪🏻

It's not visible enough that you can click on the Title icon, also you can click on it when the entity is not selected so you click on a white space (because you know it's there)
I like the use entity icon, but same here it's not very visible that it is a button
Add tile button should be centered
You should have a max size for all the input and be centered
Server selection seems broken
You have a top margin issue
Could you make the list of tile maybe contains the title ? Like Tile 1 - Super title so it's easier to navigate between the tile? You might need to add the server when multiple servers are available (CC @jpelgrom to confirm the suggestion)

home-assistant · 2026-06-23T09:48:49Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

oungsi2000 · 2026-06-25T08:10:47Z

@TimoPtr
Hello! Before the code review, I'd like to address the UX feedback first.

1000004099.mp4

Questions.

You should have a max size for all the input and be centered

Did you mean this should apply to all inputs (including labels and dropdowns) or just HATextField? I applied it only to HATextField for now, which makes the layout look a bit unbalanced

Server selection seems broken

This turned out to be a ViewModel issue. While debugging, I found that updating MutableState on an IO thread doesn't trigger recomposition. See also: https://youtrack.jetbrains.com/projects/CMP/issues/CMP-8595. Does serverManager.servers() need to be called on the IO thread?

Thanks!

TimoPtr · 2026-06-25T08:14:25Z

@TimoPtr Hello! Before the code review, I'd like to address the UX feedback first.
1000004099.mp4

Questions.

You should have a max size for all the input and be centered

Did you mean this should apply to all inputs (including labels and dropdowns) or just HATextField? I applied it only to HATextField for now, which makes the layout look a bit unbalanced

To all, you can check the onboarding screens for how I've done it in the past.

Server selection seems broken

This turned out to be a ViewModel issue. While debugging, I found that updating MutableState on an IO thread doesn't trigger recomposition. See also: youtrack.jetbrains.com/projects/CMP/issues/CMP-8595. Does serverManager.servers() need to be called on the IO thread?

It is main safe already, but I had other comments on this that might already address this.

Thanks!

* Add `ManageTiles multiple servers` preview test and associated state * Relocate reference screenshots to match the `ui` package structure * Update reference images for server discovery and tile management screens

* Add test tags to ManageTiles UI components to support automated testing * Update `LabeledSwitchRow` to allow passing a test tag to the switch component * Implement `ManageTilesTest` to verify UI visibility, state changes, and user interactions

…t Box wrapper

* Extract label and subtitle input fields into `TileLabelContent` * Move entity selection, icon configuration, and switch rows into `TileIconContent` * Improve code organization within `ManageTilesContent`

* Replace the text-based reset button with an icon button using the Restore icon * Refactor the layout to use a Row for icon selection and reset actions * Update tests to identify the reset icon by content description instead of test tag

…ject * Move `ManageTilesState` to `ManageTilesViewModel` and include derived UI logic as computed properties * Replace multiple `MutableState` properties in the ViewModel with a single `MutableStateFlow` * Expose `submitEnabled` as a derived `StateFlow` using `stateIn` * Update the `ManageTiles` composable to collect state using `collectAsStateWithLifecycle` * Implement explicit setter methods in the ViewModel for UI updates and configuration changes * Update tests and previews to reflect the new state structure and parameters

* Replace index-based tile selection with ID-based selection in `ManageTilesViewModel` and the UI layer. * Update `ManageTilesState` to store `selectedTileId` instead of a `TileSlot` object. * Move dropdown item mapping logic for tiles and servers into `ManageTilesState`. * Update `ManageTilesViewModel` and associated tests to support ID-based selection.

* Remove unused imports in `ManageTiles.kt` * Reformat `ManageTilesViewModel` and its state object for better readability * Apply minor formatting fixes in `ManageTilesViewModelTest`

* Simplify `ManageTilesState` by removing redundant UI flags and using `selectedTileId` * Update `ManageTiles` layout to center content horizontally * Update screenshot tests and relocate reference images to match the test class name * Explicitly pass `submitEnabled` as a parameter to the `ManageTiles` composable

TimoPtr · 2026-07-07T10:09:14Z

+        selectedEntityId = state.selectedEntityId,
+        onEntitySelectedId = onEntitySelectedId,
+        onEntityCleared = onEntityCleared,
+        addButtonText = "${stringResource(R.string.tile_entity)}*",


Suggested change

addButtonText = "${stringResource(R.string.tile_entity)}*",

addButtonText = "${stringResource(R.string.tile_entity)}",

TimoPtr · 2026-07-07T10:18:37Z

+    Row(
+        modifier = modifier
+            .fillMaxWidth()
+            .clip(RoundedCornerShape(HARadius.XL))


Don't clip here since it's for the full row

TimoPtr · 2026-07-07T10:21:47Z

+@Stable
+internal data class ManageTilesState(
+    val selectedTileId: String = "",
+    val servers: List<Server> = emptyList(),


Let's keep it like this but I think we could drop this line and look at the HaDropdownItem only since the key is the ID of the server.

I removed servers property

TimoPtr · 2026-07-07T10:26:35Z

+internal data class ManageTilesState(
+    val selectedTileId: String = "",
+    val servers: List<Server> = emptyList(),
+    val sortedEntities: List<Entity> = emptyList(),


Why is it called sorted? It is filtered for sure but not sorted.

I'm not sure too..I just used the name as is I'll change it

TimoPtr · 2026-07-07T10:30:13Z

-        selectedIcon = icon ?: sortedEntities.firstOrNull { it.entityId == selectedEntityId }?.getIcon(app)
+        _state.update {
+            val resolvedIcon =
+                icon ?: it.sortedEntities.firstOrNull { entity -> entity.entityId == it.selectedEntityId }?.getIcon(app)


This probably would benefits from being in Default if the list of entities is huge.

TimoPtr · 2026-07-07T10:30:54Z

-                            submitButtonLabel = commonR.string.tile_save
+                            withContext(Dispatchers.IO) {
+                                tileDao.add(tileData.copy(id = insertedId.toInt(), added = true))
+                            }


Suggested change

}

tileDao.add(tileData.copy(id = insertedId.toInt(), added = true))

DAO is main safe

TimoPtr · 2026-07-07T10:36:24Z

+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue


Suggested change

import org.junit.Assert.assertEquals

import org.junit.Assert.assertNull

import org.junit.Assert.assertTrue

import org.junit.jupiter.api.Assertions.assertEquals

import org.junit.jupiter.api.Assertions.assertTrue

import org.junit.jupiter.api.assertNull

Let's use jupiter assertions.

TimoPtr · 2026-07-07T10:36:50Z

+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue


Suggested change

import org.junit.Assert.assertEquals

import org.junit.Assert.assertTrue

import org.junit.jupiter.api.Assertions.assertEquals

import org.junit.jupiter.api.Assertions.assertTrue

TimoPtr · 2026-07-07T10:38:27Z

+    <string name="tile_entity">Select an entity to toggle or call</string>
+    <string name="tile_label">Tile label</string>
    <string name="tile_not_setup">Requires setup</string>
+    <string name="tile_required_field_hint">* Indicates a required field</string>


If you add the indicator here, I would like the indicator in the other strings CC @jpelgrom

Agreed; see #7052 (comment), I'd add the * indicator to each string.

TimoPtr

We are almost there

TimoPtr · 2026-07-07T10:40:43Z

+        viewModelScope.launch {
+            val previousEntityId = _state.value.selectedEntityId
+            _state.update { it.copy(selectedServerId = serverId) }
+            loadEntities(serverId)


You could have a race here if you quickly change servers and the first one did not finish to load entities. You could in loadEntities discard any previous Job so calling loadEntities from anywhere would discard any previous call.

Oh hadn't considered that.

TimoPtr · 2026-07-07T10:42:18Z

+    val showServerSelector = servers.size > 1 ||
+        servers.none { server -> server.id == selectedServerId }
+
+    val showResetIcon = selectedIconId != null && selectedEntityId.isNotBlank()
+
+    val submitEnabled = tileLabel.isNotBlank() &&
+        servers.any { it.id == selectedServerId } &&
+        sortedEntities.any { it.entityId == selectedEntityId }


Let's add some small unit tests for this logic

TimoPtr · 2026-07-07T10:43:24Z

    <string name="enable_change_log_popup_summary">Allow the change log dialog to automatically appear when the app is updated</string>
    <string name="tile_icon">Tile icon</string>
+    <string name="tile_reset_icon">Reset icon</string>
    <string name="tile_icon_original">Use entity icon</string>


Suggested change

<string name="tile_icon_original">Use entity icon</string>

* Wrap `IconDialog` in `HomeAssistantAppTheme` to ensure consistent styling and add a migration TODO * Remove visual indicators (asterisks) from required field labels for server, label, and entity pickers * Remove `@VisibleForTesting` constants and `testTag` modifiers from switch components * Update `TileIconRow` layout to use a `Box` for icon selection and update the reset button content description to "undo" * Center the submit button horizontally in the main column * Update `EntityPicker` to use `state.entities` instead of `state.sortedEntities` * Remove redundant clipping in `LabeledSwitchRow` and clean up preview mock data

* Rename `sortedEntities` to `entities` in `ManageTilesState` * Update `showServerSelector` and `submitEnabled` logic to rely on dropdown items instead of raw `Server` lists * Track entity loading in `ManageTilesViewModel` with a `Job` to ensure only the latest request completes and cancel stale tasks * Offload entity filtering and default icon resolution to `Dispatchers.Default` to avoid blocking the main thread * Add `ManageTilesStateTest` to verify UI state logic including `submitEnabled`, `showResetIcon`, and `showServerSelector`

- Add regression test to `ManageTilesViewModelTest` to ensure fast server switching does not result in stale entity registry data - Migrate assertions from JUnit 4 to JUnit 5 across Quick Settings test files - Remove unused `servers` list property from `ManageTilesState` and clean up `fakeServer` helpers in tests - Update `ManageTilesTest` to find vibrate and auth switches by localized text instead of test tags - Update UI test expectations for the reset icon content description from `tile_reset_icon` to `undo` - Refine tile slot assertion in `ManageTilesViewModelTest` to verify the exact count against resource arrays - Update screenshot test references to reflect UI and state changes

TimoPtr

Looks good for me let's wait for a final check from @jpelgrom and we are good to go

TimoPtr · 2026-07-13T15:20:53Z

+import androidx.lifecycle.compose.collectAsStateWithLifecycle
+import com.mikepenz.iconics.compose.Image
+import com.mikepenz.iconics.typeface.IIcon
+import io.homeassistant.companion.android.common.R


We usually do

Suggested change

import io.homeassistant.companion.android.common.R

import io.homeassistant.companion.android.common.R as commonR

jpelgrom · 2026-07-14T21:24:02Z

I don't want to change the entire architecture here last minute, also because @TimoPtr has some pending changes related to fetching the entity data, but my connection was a bit slower than usual so I noticed that it actually didn't show anything until loading data from the server finished.

tiles-loading.mp4

Artificially delayed loading entities and yes, it's even longer now.

tiles-loading-extra.mp4

This is even more problematic when switching tiles as it'll keep showing the previous' tile data until it's done, as can be seen in the second video.

Related: #7052 (comment). I think it should load the tile data from the database/system first and update state before waiting for network data, or block content with a spinner.

(Why are we even reloading everything when changing tiles, if I'm not changing the server? But that's something we can fix later, same behavior as the previous version.)

jpelgrom · 2026-07-14T21:26:10Z

@oungsi2000 Also please mark review comments as resolved when you're done with them so we can quickly see what you addressed and if there are any open questions from your end as well. (My last comment isn't a review comment, but most comments on this PR are.)

TimoPtr · 2026-07-15T06:13:39Z

I don't want to change the entire architecture here last minute, also because @TimoPtr has some pending changes related to fetching the entity data, but my connection was a bit slower than usual so I noticed that it actually didn't show anything until loading data from the server finished.
tiles-loading.mp4

Artificially delayed loading entities and yes, it's even longer now.
tiles-loading-extra.mp4

This is even more problematic when switching tiles as it'll keep showing the previous' tile data until it's done, as can be seen in the second video.

Related: (earlier comment). I think it should load the tile data from the database/system first and update state before waiting for network data, or block content with a spinner.

(Why are we even reloading everything when changing tiles, if I'm not changing the server? But that's something we can fix later, same behavior as the previous version.)

Either we manage to merge my changes on the entity before this PR and we need this PR to adjust and address the issue you've mentioned or I'll handle it in a next PR.

@oungsi2000 go over the last comments open, @jpelgrom anything you want to see before merging this?

jpelgrom · 2026-07-15T06:45:37Z

@jpelgrom anything you want to see before merging this?

#7052 (comment) at least, if we agree without addressing the rest we do need to fix the remainder before a production release in my opinion.

oungsi2000 · 2026-07-15T10:53:24Z

Should this PR be merged?? I think it's not over

TimoPtr · 2026-07-15T11:18:38Z

Should this PR be merged?? I think it's not over

I'm working on a second PR that changes the logic, I'll tag you there. If you think it misses something let's open a new PR for the rest 👍🏻, on top of the one I'm going to open in few hours.

oungsi2000 added 3 commits June 19, 2026 17:01

Migrate ManageTilesView to Material 3 and refactor for state hoisting.

13062f7

Add ManageTilesView screenshot tests

42bc0bf

Refactor ManageTilesView to use a state object and sub-composables

0ac8be8

Copilot AI review requested due to automatic review settings June 19, 2026 09:13

home-assistant Bot added the cla-signed label Jun 19, 2026

Copilot started reviewing on behalf of oungsi2000 June 19, 2026 09:13 View session

Copilot AI reviewed Jun 19, 2026

View reviewed changes

github-advanced-security AI found potential problems Jun 19, 2026

View reviewed changes

Comment thread app/src/main/kotlin/io/homeassistant/companion/android/settings/qs/views/ManageTilesView.kt Fixed

Comment thread app/src/main/kotlin/io/homeassistant/companion/android/settings/qs/views/ManageTilesView.kt Fixed

oungsi2000 and others added 3 commits June 19, 2026 18:20

Remove unused import in ManageTilesView

6ffa26c

Restore Server Discovery tablet screenshot references

f404e7d

Merge branch 'main' into refactor-manage-tiles-view

1337c97

TimoPtr reviewed Jun 23, 2026

View reviewed changes

Comment thread ...kotlin/io/homeassistant/companion/android/settings/qs/views/ManageTilesViewScreenshotTest.kt Outdated

TimoPtr requested changes Jun 23, 2026

View reviewed changes

home-assistant Bot marked this pull request as draft June 23, 2026 09:48

oungsi2000 added 12 commits June 25, 2026 17:54

Update ManageTilesView layout and icon selection UI

aa5ad24

Rename ManageTilesView to ManageTiles and move to ui package

32b736f

Add "multiple servers" test case to ManageTiles screenshot tests

f3be433

* Add `ManageTiles multiple servers` preview test and associated state * Relocate reference screenshots to match the `ui` package structure * Update reference images for server discovery and tile management screens

Use HADimens for spacing in ManageTilesView

fdd3f1d

Add UI tests for ManageTiles

fb40613

* Add test tags to ManageTiles UI components to support automated testing * Update `LabeledSwitchRow` to allow passing a test tag to the switch component * Implement `ManageTilesTest` to verify UI visibility, state changes, and user interactions

Refactor ManageTiles layout to simplify hierarchy and remove redundan…

9fc67f6

…t Box wrapper

Refactor ManageTiles view into sub-composables

a017d11

* Extract label and subtitle input fields into `TileLabelContent` * Move entity selection, icon configuration, and switch rows into `TileIconContent` * Improve code organization within `ManageTilesContent`

Clean up ManageTiles code and remove unused imports

36d00f8

* Remove unused imports in `ManageTiles.kt` * Reformat `ManageTilesViewModel` and its state object for better readability * Apply minor formatting fixes in `ManageTilesViewModelTest`

TimoPtr reviewed Jul 7, 2026

View reviewed changes

Comment thread app/src/main/kotlin/io/homeassistant/companion/android/settings/qs/ui/ManageTilesScreen.kt

TimoPtr reviewed Jul 7, 2026

View reviewed changes

Comment thread app/src/test/kotlin/io/homeassistant/companion/android/settings/qs/ManageTilesViewModelTest.kt

TimoPtr reviewed Jul 7, 2026

View reviewed changes

TimoPtr requested changes Jul 7, 2026

View reviewed changes

home-assistant Bot marked this pull request as draft July 7, 2026 10:43

oungsi2000 mentioned this pull request Jul 10, 2026

Rework UI of IconDialog to use Material3 #7156

Open

oungsi2000 added 3 commits July 10, 2026 14:46

oungsi2000 marked this pull request as ready for review July 10, 2026 06:27

home-assistant Bot requested a review from TimoPtr July 10, 2026 06:27

TimoPtr approved these changes Jul 13, 2026

View reviewed changes

TimoPtr requested a review from jpelgrom July 13, 2026 15:27

Adjust R imports

ad0681d

TimoPtr merged commit e9400f4 into home-assistant:main Jul 15, 2026
3 of 4 checks passed

TimoPtr mentioned this pull request Jul 15, 2026

Improve logic for tiles management #7180

Open

4 tasks

	addButtonText = "${stringResource(R.string.tile_entity)}*",
	addButtonText = "${stringResource(R.string.tile_entity)}",

	}
	tileDao.add(tileData.copy(id = insertedId.toInt(), added = true))

		import org.junit.Assert.assertEquals
		import org.junit.Assert.assertTrue

	import io.homeassistant.companion.android.common.R
	import io.homeassistant.companion.android.common.R as commonR

Uh oh!

Uh oh!

Conversation

oungsi2000 commented Jun 19, 2026

Summary

Checklist

Screenshots

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimoPtr commented Jun 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

home-assistant Bot commented Jun 23, 2026

Uh oh!

oungsi2000 commented Jun 25, 2026

Questions.

Uh oh!

TimoPtr commented Jun 25, 2026

Questions.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oungsi2000 Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimoPtr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimoPtr left a comment

Choose a reason for hiding this comment

oungsi2000 Jul 10, 2026 •

edited

Loading