Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate "Manage Sensors" to Compose and ViewModels #2359

Merged
merged 14 commits into from Mar 15, 2022

Conversation

jpelgrom
Copy link
Member

@jpelgrom jpelgrom commented Mar 9, 2022

Summary

This PR migrates the "Manage Sensors" screen in the settings (and the detail/configuration screen for individual sensors) to use Compose and ViewModels with flows for showing realtime data instead of large Fragments and XML preferences.

Scrolling performance for Compose with long lists isn't great with debug builds, but is as smooth as the previous PreferenceFragmentCompat-based screens when using a release build. Not sure why this happens but I built a release build to verify and it really helps.

There is also quite a lot of complexity in these screens, so I haven't made any significant changes to the design for now to keep this PR easier to review.

Screenshots

As mentioned above, the screens haven't changed significantly and unless you compare side-by-side most users probably won't notice the difference.

Light Dark
'Manage sensors' list image image
Details for a sensor that is off image image
Details for a sensor that is on with attributes/settings image image
Details for a sensor that is on without attributes/settings image image
Sensor setting: toggle (first setting), and disabled settings (all except first and last setting) image image
Sensor setting: string/number image image
Sensor setting: list (single choice) image image
Sensor setting: list-* (multiple choices) image image

Link to pull request in Documentation repository

Not needed

Any other notes

 - Start of migrating the list of sensors and their individual on/off settings to Compose by moving the list over
 - Aims to replicate original design of the page for now
 - Add enabled switch and description + permission handling
 - Update main list to use new Compose-based fragment
 - Add new fragments to factory to prevent crash on recreation
 - Add more properties and attributes for the sensor (basically anything that doesn't require a pop-up)
 - Move enabled check with permissions to ViewModel because it also needs to be updated in the database
 - Fix collecting sensor settings
 - Added a framework for setting sensor settings; toggle builds on the existing on/off toggle, and added a MutableState for settings that require the app to show a dialog
 - Support for changing sensor settings that must be toggled
 - Support for changing sensor settings where text or number input is required
 - Add support for setting list-based settings
 - Dynamic maximum height for dialog in order to fill most of the screen
 - Remove old fragment and XML since all functionality should now be covered
 - Show translated setting for list settings
 - Fix update of list setting removing the list options in the future, because they were stored in the DB entry but a new entry without any values was created
 - Fix toggle sensor setting (removed the space)
 - Don't use two line row height if there is no summary
 - Remove onCheckedChange from Switch so that it is handled by the row
 - Prevent text from getting too close to switch
 - Fix search query still being set when returning to the main sensors list after viewing a sensor's details by clearing the query in the view model if the SearchView is closed
 - Move sorting the sensors for each manager to the UI where the sensors are queried, instead of in an internal variable
 - Set secondary color to allow Switches, RadioButtons and Checkboxes to use the blue HA tint without overriding each one manually
 - Add stable IDs to main sensor list to improve recomposition for search
@dshokouhi
Copy link
Member

Very nice speed improvements! I noticed the toggles in some settings are now so fast you don't even see them on for a few seconds. This was not easy to accomplish since there is so much logic but hats off to you!

Haven't had a chance to dive in to the code yet but the brief tests with the debug build were great.

 - Move the dialog that is used to a separate file because it was already based on a custom dialog for the Manage Widgets screen.
 - Improve max height for dialog by using weight, to make sure it still works correctly for longer titles
 - When using radio button items, don't require the user to press save but instead save it when an item is selected
 - Google says using the same color as the surface with a minimal shaodw is by design, https://issuetracker.google.com/issues/185418990, but it just results in bad contrast (probably why they switched to a different switch design in Android 12), so manually copied the color code to use
@jpelgrom
Copy link
Member Author

Made a few small design tweaks, one of which includes the Switch 'thumb color' (circle) when off. The switch in onboarding already had a different color set so clearly someone agrees that the default is bad even if it is by design according to Google. I've used an eyedropper/pipette tool to get the color from non-Compose switches, and updated the switch in onboarding to match as well.

@JBassett JBassett merged commit 3e928bd into home-assistant:master Mar 15, 2022
@jpelgrom jpelgrom deleted the compose-sensors branch March 15, 2022 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants