google-developer-training / basic-android-kotlin-compose-training-woof

Apache License 2.0
55 stars 87 forks source link

Material Theming: Android Basics in Compose (a better Dark Theme preview?) #1

Closed correabuscar closed 1 year ago

correabuscar commented 2 years ago

URL of codelab https://developer.android.com/codelabs/basic-android-kotlin-compose-material-theming?continue=https%3A%2F%2Fdeveloper.android.com%2Fcourses%2Fpathways%2Fandroid-basics-compose-unit-3-pathway-3%23codelab-https%3A%2F%2Fdeveloper.android.com%2Fcodelabs%2Fbasic-android-kotlin-compose-material-theming#3

In which task and step of the codelab can this issue be found? 4. Add color in the section titled Add dark theme which is at about 80% scroll down.

Describe the problem It seems a bit unnecessary to duplicate code in order to show the dark theme, why not use another @Preview annotation instead?

This is what's currently there:

@Preview
@Composable
fun WoofPreview() {
    WoofTheme(darkTheme = false) {
        WoofApp()
    }
}

@Preview
@Composable
fun WoofDarkThemePreview() {
    WoofTheme(darkTheme = true) {
        WoofApp()
    }
}

And this is what I'm suggesting(perhaps in addition to the above, at least mentioning it in the codelab as a possibility?) by avoiding code duplication:

@Preview(
    name="Light Theme",
)
@Preview(
    uiMode = UI_MODE_NIGHT_YES,
    name="Dark Theme",
)
@Composable
fun WoofPreview() {
    WoofTheme() {
        WoofApp()
    }
}

(see the screenshot below to see how it looks)

Versions Android Studio version: Android Studio Electric Eel | 2022.1.1 Canary 9 Build #AI-221.5921.22.2211.8881706, built on July 28, 2022 Runtime version: 11.0.13+0-b1751.21-8125866 amd64 VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o. Linux 5.18.16-gentoo-x86_64 GC: G1 Young Generation, G1 Old Generation Memory: 2048M Cores: 12 Registry: external.system.auto.import.disabled=true ide.text.editor.with.preview.show.floating.toolbar=false

Current Desktop: LXQt

API version of the emulator: doesn't matter.

Additional information The simpler, perhaps also more idiomatic, way without duplicating code: screen-2022-08-07-12-27-00

osuleymanova commented 1 year ago

Hello @correabuscar,

Thank you so much for reaching out to us and sharing your feedback- I'll pass it on to the team for the review and possibly the future maintenance.