-
-
Notifications
You must be signed in to change notification settings - Fork 958
Migrate configure camera widget to compose #6499
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
base: main
Are you sure you want to change the base?
Changes from all commits
889724e
059ba69
944b43a
de2cfda
5fe252a
992bc17
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,7 @@ package io.homeassistant.companion.android.util.compose | |||||
|
|
||||||
| import androidx.annotation.StringRes | ||||||
| import androidx.compose.foundation.layout.fillMaxWidth | ||||||
| import androidx.compose.foundation.text.selection.TextSelectionColors | ||||||
| import androidx.compose.material.DropdownMenuItem | ||||||
| import androidx.compose.material.ExperimentalMaterialApi | ||||||
| import androidx.compose.material.ExposedDropdownMenuBox | ||||||
|
|
@@ -21,6 +22,17 @@ import io.homeassistant.companion.android.common.R as commonR | |||||
| import io.homeassistant.companion.android.database.server.Server | ||||||
| import io.homeassistant.companion.android.database.widget.WidgetBackgroundType | ||||||
| import io.homeassistant.companion.android.widgets.common.WidgetUtils | ||||||
| import androidx.compose.material3.DropdownMenuItem as DropdownMenuItemM3 | ||||||
| import androidx.compose.material3.ExperimentalMaterial3Api | ||||||
| import androidx.compose.material3.ExposedDropdownMenuBox as ExposedDropdownMenuBoxM3 | ||||||
| import androidx.compose.material3.ExposedDropdownMenuDefaults as ExposedDropdownMenuDefaultsM3 | ||||||
| import androidx.compose.material3.MenuAnchorType as MenuAnchorTypeM3 | ||||||
| import androidx.compose.material3.MenuItemColors | ||||||
| import androidx.compose.material3.TextField as TextFieldM3 | ||||||
| import androidx.compose.material3.Text as TextM3 | ||||||
| import androidx.compose.ui.graphics.Color | ||||||
| import io.homeassistant.companion.android.common.compose.composable.HATextField | ||||||
|
Comment on lines
+33
to
+34
|
||||||
| import androidx.compose.ui.graphics.Color | |
| import io.homeassistant.companion.android.common.compose.composable.HATextField |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not do that in this PR. We can't replace the ServerExposedDropdown like this because it has an impact everywhere.
It is a nice goal to migrate to M3, but we should make sure to not mix M2 and M3 and today this dropdown is used in a lot of place that are still in M2.
So I would suggest to make a first PR about making a new version of the ServerExposedDropdownMenu close to the other HA* composable like the HAButton. Inspire yourself from the other components (it has tests and integration within the compose catalog and documentation).
Then a second PR for the configure camera widget compose to M3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually another contributor is also migrating a screen to compose in this PR #6444 he took the approach to first migrate to compose and then do the migration to M3 later.
I feel that both of you are going to miss the Menu for the server and color. If you don't do it I might help you by making the component so you can use it directly.
I think you would learn some stuff by looking at the comments I left in the other PR that could help you on that one.
Check failure
Code scanning / Android Lint
Immutable collections should ideally be used in Composables Error
You should use Kotlinx Immutable Collections instead: keys: ImmutableList<String> or create an @Immutable wrapper for this class: @Immutable data class KeysList(val items: List<String>)
See https://slackhq.github.io/compose-lints/rules/#avoid-using-unstable-collections for more information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can ignore the Immutable collections with an annotation for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check the other existing components HATextField for instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are unused imports here (e.g., TextSelectionColors). These will be flagged by ktlint/IDE and should be removed.