-
-
Notifications
You must be signed in to change notification settings - Fork 80
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
Rewrite BatteryOptimizationsPage to M3 #747
Rewrite BatteryOptimizationsPage to M3 #747
Conversation
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
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.
We want the activity - in this case the IntroPage
to be as clean as possible. Hence we should move everything we can out into their own respective files.
In the viewModel we started to use an immutable UiState
data class to hold the viewmodel state. Following the guideline and for consistency we should do that everywhere.
Looks good otherwise tho :)
app/src/main/kotlin/at/bitfire/davdroid/ui/intro/BatteryOptimizationsPage.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/ui/intro/BatteryOptimizationsPage.kt
Outdated
Show resolved
Hide resolved
app/src/main/kotlin/at/bitfire/davdroid/ui/intro/BatteryOptimizationsPage.kt
Outdated
Show resolved
Hide resolved
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
Should be ready :) |
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.
Nice. Looks much tidier already.
Not sure if the requested changes introduced this, but I just found the dontShowBattery
checkbox is not selectable/updated when tapped anymore.
1ae5eb5
to
b15cb80
Compare
47ef18b
to
39f8f2e
Compare
I'm working on it |
Signed-off-by: Arnau Mora <arnyminerz@proton.me>
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.
Checkboxes still weren't selectable. Last commit fixes it :)
Please rebase to main-ose (better DI) |
# Conflicts: # app/src/main/kotlin/at/bitfire/davdroid/ui/intro/BatteryOptimizationsPage.kt
Signed-off-by: Arnau Mora Gras <arnyminerz@proton.me>
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.
Looks good; I just wonder whether we should combine hintBatteryOptimization
and hintAutostartOptimization
into the uiState
too. We can discuss it tomorrow :)
The PR should be in Draft state during development. As soon as it's finished, it should be marked as Ready for review and a reviewer should be chosen.
See also: Writing A Great Pull Request Description
Purpose
Update design of the battery optimizations intro page so that it uses M3.
Also updates the architecture of the activity-viewmodel-composables relationship so that it follows best practices as discussed internally, and as recommended by Google.
Short description
MaterialTheme.typography.body1
::MaterialTheme.typography.bodyLarge
MaterialTheme.typography.body2
::MaterialTheme.typography.bodyMedium
MaterialTheme.typography.h6
::MaterialTheme.typography.labelLarge
{}
by default so that it's not required in preview.Checklist