feat(Sensors): Add search filter to sensor setting allow list dialog by danielgomezrico · Pull Request #6567 · home-assistant/android

danielgomezrico · 2026-03-12T19:40:32Z

Summary

Add a search TextField to SensorDetailSettingDialog so users can quickly filter long app lists (e.g., Last Notification Allow List)
Filtering is local to the composable — no ViewModel changes, selections persist across searches
Works for all list-type sensor settings (LIST_APPS, LIST_BLUETOOTH, LIST_ZONES, etc.)

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

Before

No search bar, hard to read and search:

After

Copilot

Pull request overview

Adds an in-dialog search field to SensorDetailSettingDialog so users can locally filter long list-type sensor setting entries (apps, Bluetooth devices, zones, etc.) without changing ViewModel/state handling.

Changes:

Added a search TextField with clear button to filter list-type setting dialog entries.
Introduced filterSettingEntries(...) helper to apply case-insensitive filtering.
Added unit tests covering filtering behavior and edge cases.

Reviewed changes

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

File	Description
app/src/main/kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailView.kt	Adds search UI + filtering logic for list dialogs; introduces `filterSettingEntries` helper
app/src/test/kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailSettingDialogFilterTest.kt	Adds JUnit tests validating filtering behavior

You can also share your feedback on Copilot code review. Take the survey.

jpelgrom · 2026-03-13T20:14:13Z

Can you provide screenshots for the change?

Did you consider only showing search when there is a long list? Showing search when there are only 3 items is a bit weird.

This dialog isn't really suited for a large list anyway, I wonder if it should be changed to a different picker like the bottom sheet/dialog we have implemented for entities recently.

Both comments generated by Copilot are valid, please address them (try doing it yourself without AI to understand the change you're making).

danielgomezrico · 2026-03-13T22:52:54Z

Can you provide screenshots for the change?

Sure! updated.

Did you consider only showing search when there is a long list? Showing search when there are only 3 items is a bit weird.

Oh that Idea is cool I will adapt it for that case

This dialog isn't really suited for a large list anyway, I wonder if it should be changed to a different picker like the
bottom sheet/dialog we have implemented for entities recently.

It makes total sense, but in the mean while it would help, the list of apps could be pretty long and having to search in that dialog is pretty hard.

TimoPtr · 2026-03-16T09:15:39Z

I'm aligned with @jpelgrom I would prefer to see a bottom sheet here. You already have the needed components to do so. You can check the Entity picker like @jpelgrom suggested.

You might also want to merge main in your branch since we've fixed some issue with ktlint recently.

danielgomezrico · 2026-03-17T00:03:58Z

I'm aligned with @jpelgrom I would prefer to see a bottom sheet here. You already have the needed components to do so. You can check the Entity picker like @jpelgrom suggested.

You might also want to merge main in your branch since we've fixed some issue with ktlint recently.

Got it, Ill check if I can make it for this case.

One thing I've noticed is that these types of dialogs are used in other places; should the bottom sheet be limited to this case?

jpelgrom · 2026-03-17T06:31:52Z

One thing I've noticed is that these types of dialogs are used in other places; should the bottom sheet be limited to this case?

Just this specific dialog, with a long list of apps that you may want to search. Replacing all other dialogs makes this PR too big and also doesn't make sense as a pattern when there are only 2-3 options.

danielgomezrico · 2026-03-24T00:18:10Z

@jpelgrom @TimoPtr I made the update to show it in a bottom sheet, WDYT?

TimoPtr · 2026-03-25T13:34:57Z

@danielgomezrico could you address the lint issue and when you are ready put the PR in ready for review.

danielgomezrico · 2026-04-06T19:45:18Z

@TimoPtr can you help me aproving the CI 🙏🏻 ?

TimoPtr

The UX needs to be adjust the rest of the comments are mostly to adjust with the project guidelines.

home-assistant · 2026-04-13T13:44:50Z

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.

…bleForTesting

…bleForTesting - Wrap setting entries in `@Immutable SettingEntries` data class at the composable boundary so the Compose Compiler can infer parameter stability for `rememberFilteredSettingEntries` (lint id 6307/6309). - Drop `@VisibleForTesting` from `parseSettingLabel`; it is genuinely shared production helper consumed by `SensorDetailView.kt` (lint id 6306/6308). Function stays `internal`. - Add `SettingEntriesTest` exercising the new wrapper so a future regression of the stability fix fails loudly.

Split the sheet body into header/list/footer composables, replace the `SettingEntries` stability wrapper with a plain `SettingEntry` data class, hoist horizontal padding to the parent column, drop the injectable dispatcher in favour of `Dispatchers.Default`, and use `toMutableStateList()` for the selection state. Filter tests updated for the new data class; wrapper-only tests removed.

The bottom sheet relies on `LocalHAColorScheme` and `HATextStyle`, both of which are populated by `HATheme`. The enclosing `SensorDetailView` has not migrated to the shared design system yet, so wrap the sheet call site explicitly. TODO references issue home-assistant#6839 for the eventual view-level migration.

… stability Resolves four ComposeUnstableCollections lint errors on SensorDetailSettingSheet.kt by wrapping the list in a SettingEntryList @immutable data class so the Compose Compiler can treat the parameter as stable.

The outer Box centered both the loading spinner and the LazyColumn, so short result lists rendered vertically centered instead of starting at the top. Center only the loading indicator; let the list fill from the top.

@VisibleForTesting

- Animate sheet collapse: invoke bottomSheetState.hide() inside a rememberCoroutineScope before propagating cancel/save so the dismissal is reflected visually before the dialog state mutates. - Drop the @immutable SettingEntryList wrapper (did not actually freeze the underlying list); pass List<SettingEntry> directly and baseline the resulting ComposeUnstableCollections warning, matching how other list-of-data-class composables in this module already handle it. - Hide the SettingEntry data class behind @VisibleForTesting. - Extract parseSettingLabel into ParsedSettingLabel.kt and return a named (primary, secondary) data class instead of Pair, with an explicit parts.size == 2 guard. Shared by the new sheet and the legacy Material 2 row. - Inline SheetHeader onto its ColumnScope and rely on the parent Column's Arrangement.spacedBy for inter-section spacing; remove the per-child vertical padding. - Set role = Role.Checkbox on the row's .clickable so screen readers announce the toggle correctly. - Render the primary line of both rows with HATextStyle.Body so the legacy dialog inherits the same hierarchy as the new sheet. - Pass viewModel::cancelSettingWithDialog and onDialogSettingSubmitted as method references instead of wrapping lambdas, and point the HATheme TODO at the live tracking issue (home-assistant#6839). - Drop SettingDialogStateImmutabilityTest now that the invariant is obvious from the call site. - Parameterize ParseSettingLabelTest cases via @MethodSource.

- Promote DEFAULT_DEBOUNCE to @VisibleForTesting/internal so test code can reference the same source of truth instead of duplicating 300ms. - Document that the LocalInspectionMode branch in rememberHAModalBottomSheetState produces a no-op state with no animation or hide/expand semantics — it only exists so previews and screenshot tests can render the sheet content.

… Modifier EntityPicker and SensorDetailSettingSheet were each declaring an inline NestedScrollConnection plus a pointerInput(detectVerticalDragGestures) block — same code, same intent: stop the modal bottom sheet from interpreting list scrolls or flings as collapse gestures. Extract that into `Modifier.consumeSheetScrollFling()` next to HAModalBottomSheet so future sheets reuse it instead of copying the pattern.

Replace the per-call `composed { remember { ... } }` wrapper with a stateless singleton NestedScrollConnection. The object had no per-call state, so hoisting it to a top-level `val` is semantically identical and clears the slack ComposeModifierComposed lint failure introduced when this Modifier was extracted into :common.

- Rename SettingDialogState.loading to isLoading - Use HALoading instead of raw CircularProgressIndicator - Hoist row onClick; SheetEntryList takes selectedIds + onToggle instead of a SnapshotStateList - Use HAColorScheme palette for the checkbox - Extract SensorDetailSettingSheetContent for previewability

Robolectric Compose test covering clear-icon show/hide, clear behavior, and debounce timing

Cover with/without search, loading, and empty-filter states across device specs

Compose cannot infer stability of Set/List parameters, failing lint. Pass an isSelected lambda instead of selectedIds Set, and inline the filtered-entries helper so no Composable takes an unstable collection.

…sheet The lambda refactor deleted rememberFilteredSettingEntries, leaving a baseline entry pointing at a removed function. Replace it with the two SensorDetailSettingSheetContent/SheetEntryList entries: List<SettingEntry> params that are actually flagged, so the baseline stays honest.

Render a centered "No results" message when the filtered entry list is empty instead of a blank area. Also drop the redundant comment above BottomSheetSettingRow per review. Addresses review threads on SensorDetailSettingSheet.

Merge the two empty-field render checks into one, fold the clear-icon behaviour (cleared field, reappearing label, empty emit) into a single test that also asserts "chrome" was emitted before clearing, and combine the debounce before/after assertions into one timing test. Drop the obsolete ConsoleLogRule, replaced upstream by the global ConsoleLogPlatformListener, so the suite compiles after the merge. Addresses review threads on HASearchFieldTest.

joinToString() never emits the surrounding brackets that the legacy List.toString() persistence produced, so the .replace("[","").replace("]","") calls were a no-op for the package names, MAC addresses and zone IDs this function joins. Remove them, correct the KDoc, and update the parity test to assert brackets now pass through unchanged.

Address review feedback on the shared compose composables: - HAModalBottomSheet: roll back the over-expanded rememberHAModalBottomSheetState and ConsumeSheetScrollFlingConnection docs, keeping only the new skipPartiallyExpanded parameter note. - HASearchField: rename DEFAULT_DEBOUNCE to SEARCH_FIELD_DEFAULT_DEBOUNCE, make debouncedSearchUpdate private and move it below the composable, and trim its doc. - Drop HASearchFieldDebounceTest; the debounce timing is already covered by the HASearchField composable test.

Address review feedback on the allow-list setting sheet: - Drop the redundant inline filtering comment and the inner-content extraction rationale from SensorDetailSettingSheetContent's doc. - Render the real HAModalBottomSheet in the screenshot test instead of the bare extracted content, so the snapshots exercise the actual sheet chrome (scrim, drag handle, rounded corners). Reference images regenerated.

…nshots Replace the hand-built SheetState in SensorDetailSettingSheetScreenshotTest with the shared rememberHAModalBottomSheetState helper so the screenshot harness exercises the same bottom sheet code as production, fixing the broken merge in the rendered references. Regenerate the affected reference images.

Copilot

Pull request overview

Copilot reviewed 16 out of 40 changed files in this pull request and generated 2 comments.

+package io.homeassistant.companion.android.common.compose.composable
+
+import androidx.activity.ComponentActivity
+import androidx.compose.ui.test.assertIsDisplayed


+    val entries = remember(state.entries) {
+        state.entries.map { (id, label) -> SettingEntry(id, label) }
+    }
+    val checkedValue = remember { state.entriesSelected.toMutableStateList() }


TimoPtr · 2026-06-29T12:16:00Z

+ * @param modifier Optional [Modifier] applied to the root [Column].
+ */
+@Composable
+internal fun SensorDetailSettingSheetContent(


This one should be private.

I kept this one internal because the screenshot test renders it directly (following your suggestion to screenshot the real sheet). The other composables in the file are private.

TimoPtr · 2026-06-29T12:36:18Z

+ * @param debounce Delay before non-empty queries are propagated to [onQueryChange].
+ */
+@Composable
+fun HASearchField(


This could have been a small PR on its own.

#7151

Ill rebase when it is merged

FYI, the linked PR was merged

rsheptolut

Hey, thanks for your contribution! Please consider a couple of ideas below. I opened #7129 a couple days ago, which addresses the same underlying UX issue, but when TimoPtr got to it he closed it due to this one being a better foundation, and suggested I review this one. So just throwing in some ideas from that PR.

…n thread - SettingDialogState now holds SettingEntry items, a showSearch flag and the selected IDs; the sheet no longer maps or joins values itself - open the dialog immediately in loading state and enumerate entries on Dispatchers.Default so long app lists no longer freeze the UI - sort selected entries first when a multi-select sheet opens - merge label parsing into SettingEntry and reuse it in SensorDetailSettingRow - expand the sheet fully on open and use HACheckbox for rows

Copilot AI review requested due to automatic review settings March 12, 2026 19:40

home-assistant Bot added the cla-signed label Mar 12, 2026

Copilot started reviewing on behalf of danielgomezrico March 12, 2026 19:41 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

Comment thread ...src/main/kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailView.kt Outdated

Comment thread ...src/main/kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailView.kt Outdated

danielgomezrico force-pushed the feature/search-sensor-allow-list branch from 41b5f66 to 2baef1b Compare March 13, 2026 22:51

danielgomezrico changed the title ~~feat(sensors): add search filter to sensor setting allow list dialog~~ feat(Sensors): Add search filter to sensor setting allow list dialog Mar 13, 2026

github-advanced-security AI found potential problems Mar 13, 2026

View reviewed changes

TimoPtr marked this pull request as draft March 16, 2026 09:15

github-advanced-security AI found potential problems Mar 24, 2026

View reviewed changes

danielgomezrico marked this pull request as ready for review March 29, 2026 14:02

github-advanced-security AI found potential problems Mar 29, 2026

View reviewed changes

TimoPtr marked this pull request as draft March 30, 2026 08:01

danielgomezrico marked this pull request as ready for review April 2, 2026 23:51

TimoPtr reviewed Apr 13, 2026

View reviewed changes

Comment thread ...src/main/kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailView.kt Outdated

TimoPtr reviewed Apr 13, 2026

View reviewed changes

Comment thread ...src/main/kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailView.kt Outdated

TimoPtr reviewed Apr 13, 2026

View reviewed changes

Comment thread ...src/main/kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailView.kt Outdated

TimoPtr reviewed Apr 13, 2026

View reviewed changes

Comment thread ...src/main/kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailView.kt Outdated

TimoPtr requested changes Apr 13, 2026

View reviewed changes

home-assistant Bot marked this pull request as draft April 13, 2026 13:44

danielgomezrico requested a review from TimoPtr May 4, 2026 02:34

danielgomezrico added 20 commits June 26, 2026 01:01

fix(Sensors): top-align entry list in allow-list sheet

7912947

The outer Box centered both the loading spinner and the LazyColumn, so short result lists rendered vertically centered instead of starting at the top. Center only the loading indicator; let the list fill from the top.

test(common): add HASearchField Compose UI test

beee2ae

Robolectric Compose test covering clear-icon show/hide, clear behavior, and debounce timing

test(Sensors): add multi-select bottom sheet screenshot tests

2d36d40

Cover with/without search, loading, and empty-filter states across device specs

refactor(Sensors): fix ComposeUnstableCollections lint in setting sheet

5b27228

Compose cannot infer stability of Set/List parameters, failing lint. Pass an isSelected lambda instead of selectedIds Set, and inline the filtered-entries helper so no Composable takes an unstable collection.

Copilot AI reviewed Jun 26, 2026

View reviewed changes

danielgomezrico force-pushed the feature/search-sensor-allow-list branch from 96c6da4 to 34a348d Compare June 26, 2026 06:06

TimoPtr requested changes Jun 29, 2026

View reviewed changes

home-assistant Bot marked this pull request as draft June 29, 2026 12:36

TimoPtr mentioned this pull request Jul 6, 2026

Improve UX when 'Sensor detail' dialog has many entries #7129

Closed

4 tasks

rsheptolut reviewed Jul 7, 2026

View reviewed changes

Comment thread .../kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailSettingSheet.kt

Comment thread .../kotlin/io/homeassistant/companion/android/settings/sensor/views/SensorDetailSettingSheet.kt Outdated

TimoPtr mentioned this pull request Jul 9, 2026

Introduce HASearchField #7151

Merged

4 tasks

danielgomezrico added 3 commits July 9, 2026 18:05

fix(EntityPicker): remove committed merge conflict marker

848be14

docs(common): restore skipPartiallyExpanded param doc wording

468a0ba

Uh oh!

Uh oh!

Conversation

danielgomezrico commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Screenshots

Before

After

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!

jpelgrom commented Mar 13, 2026

Uh oh!

danielgomezrico commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimoPtr commented Mar 16, 2026

Uh oh!

danielgomezrico commented Mar 17, 2026

Uh oh!

jpelgrom commented Mar 17, 2026

Uh oh!

danielgomezrico commented Mar 24, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimoPtr commented Mar 25, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danielgomezrico commented Apr 6, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimoPtr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danielgomezrico commented Mar 12, 2026 •

edited

Loading

danielgomezrico commented Mar 13, 2026 •

edited

Loading