Rework UI of settings wear favorite screen to use Material3 - #7311
Rework UI of settings wear favorite screen to use Material3#7311aishsidya0402-netizen wants to merge 5 commits into
Conversation
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
There was a problem hiding this comment.
Pull request overview
This PR continues the app’s Material3 migration by updating the Wear settings “favorites” screen top app bar to use the existing Material3 HATopBar composable from :common, and adds a Compose UI test intended to validate the app bar behavior.
Changes:
- Replaced the Material2
TopAppBarimplementation inSettingsWearTopAppBarwith:common’s Material3HATopBar. - Added a new Compose test for
SettingsWearTopAppBarbehavior (help icon visibility, back click, title). - Added a duplicate Gradle property and introduced a new committed IDE Markdown settings file.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/src/full/kotlin/io/homeassistant/companion/android/settings/wear/views/SettingsWearHomeView.kt | Migrates the Wear settings top bar to the shared Material3 HATopBar. |
| wear/src/test/kotlin/io/homeassistant/companion/android/settings/wear/views/SettingsWearTopAppBarTest.kt | Adds Compose tests for the top bar behavior, but currently targets the wrong module/source set and contains an unreliable assertion. |
| gradle.properties | Adds a duplicate org.gradle.tooling.parallel entry. |
| .idea/markdown.xml | Adds an IDE-specific Markdown preview provider setting that may not be appropriate to commit. |
Files not reviewed (1)
- .idea/markdown.xml: Generated file
Suppressed comments (1)
wear/src/test/kotlin/io/homeassistant/companion/android/settings/wear/views/SettingsWearTopAppBarTest.kt:63
assert(backClicked)uses JVM assertions, which are typically disabled for Gradle test tasks unless explicitly enabled; this can cause the test to pass even whenbackClickedis false. Use a JUnit assertion instead so the test reliably fails when the callback isn't invoked.
assert(backClicked) { "Expected onBackClicked to be invoked after tapping the back icon" }
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@home-assistant recheck |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
@home-assistant recheck |
|
👋 Thanks for contributing! The issue you're referencing involves more work than you're doing now, and in fact a completely different screen. Are you going to expand this PR over time? You can review the entity widget settings screen for a recent example of our Compose patterns and what is expected. For new Compose work, please make sure to add screenshot testing as well. The screen you're changing lives in the
You can copy our mock files for testing, see the developer documentation. |
|
Hi Joris, thanks for the detailed feedback! To clarify scope: this PR focuses specifically on migrating the top app bar ( Re: the test module, fixed, moved Re: Re: screenshot testing, given I'm on a tight course deadline tonight, I likely won't be able to add this before submission, but I'm glad to follow up with it afterward if that works, following the guide you linked. Thanks again for taking the time to review! |
I'm referring to the favorites screen, that is the issue you've linked and mentioned in your PR title. You're now adjusting the parent which holds the navigation for all Wear OS settings in the phone app, which will also be shown on other screens, doesn't fully change the favorites screen, and results in a combination of Material 2 and 3 components which should be avoided.
Did you rename the mock file? I see my link wasn't to the specific page for it, the correct link is: https://developers.home-assistant.io/docs/android/get_started#setting-up-a-mock-firebase-project. For us to consider this, we would need this scoped to the favorites screen and do the full screen, so not a component shared across multiple screens creating a combination of M2 and M3, and also with the updated architecture I pointed out (example: #7007 + #7302). |
Summary
This PR migrates the Wear OS settings "favorites" screen's top app bar (
SettingsWearTopAppBarinSettingsWearHomeView.kt) from Material2 to Material3, using the app's existingHATopBarcomposable (common/compose/composable/HATopBar.kt). This brings the screen in line with the broader Material3 migration effort tracked in #5420.The screen was still importing
androidx.compose.material.IconButtonandandroidx.compose.material.TopAppBar(Material2), while most of the rest of the app has already migrated to Material3. Rather than manually swapping inandroidx.compose.material3components, this PR delegates to the app's existingHATopBarcomposable, which already wrapsTopAppBarwith Material3 theming and supports optional back/close/help buttons. This removes now-unused Material2 and iconics imports, and the public signature ofSettingsWearTopAppBar(title, onBackClicked, modifier, docsLink) was kept unchanged, so all 5 existing call sites inLoadSettingsHomeView's nav graph required no changes.Closes #6300
Checklist
Select exactly one option that describes AI usage in this contribution:
Screenshots
On-device screenshots are pending; I'll add these as a follow-up comment once local emulator verification is complete.
Any other notes
I added
SettingsWearTopAppBarTest.ktcovering help-icon visibility (shown/hidden based ondocsLink), back-icon click behavior, and title rendering. The test resolves correctly and is recognized by the JUnit test runner, but I haven't been able to execute it locally yet. Thewearmodule's debug build variant requires agoogle-services.jsonwith a client entry matching the debug-suffixed package name, which isn't included in the public repo. If there's a documented local workaround for this, I'd really appreciate a pointer; otherwise, happy to confirm it passes in CI.