android / nowinandroid

A fully functional Android app built entirely with Kotlin and Jetpack Compose
Apache License 2.0
15.9k stars 2.8k forks source link

[Bug]: Onboarding "Done" text is tiny #407

Closed dturner closed 10 months ago

dturner commented 1 year ago

Is there an existing issue for this?

Is there a StackOverflow question about this issue?

What happened?

The text on the "Done" button is tiny. It shouldn't be.

Screenshot_20221110-130133

Relevant logcat output

No response

Code of Conduct

SimonMarquis commented 1 year ago

What would be the appropriate style?

https://github.com/android/nowinandroid/blob/e72e31b1f013676a10b4ecc6f83606a51141e862/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt#L270-L272

MaterialTheme.typography.bodyMedium maybe?

SimonMarquis commented 1 year ago
Default bodySmall bodyMedium bodyLarge
image image image image
SimonMarquis commented 1 year ago

Though it seems like the NiaFilledButton (and NiaOutlinedButton) are always using MaterialTheme.typography.labelSmall without checking the value of the boolean parameter small.

https://github.com/android/nowinandroid/blob/e72e31b1f013676a10b4ecc6f83606a51141e862/core/designsystem/src/main/java/com/google/samples/apps/nowinandroid/core/designsystem/component/Button.kt#L73-L77

Maybe the better fix would be to do:

        content = {
-            ProvideTextStyle(value = MaterialTheme.typography.labelSmall) {
+            ProvideTextStyle(value = if (small) MaterialTheme.typography.labelSmall else MaterialTheme.typography.labelMedium) {
                content()
            }
        }
SimonMarquis commented 1 year ago

@dturner this has been fixed, you can close the issue :) image

SimonMarquis commented 10 months ago

This issue has been fixed, we can close the issue.