Skip to content
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

[BUG] Some surface color roles are derived from the wrong palette #5194

Open
HarmfulBreeze opened this issue Jan 22, 2025 · 3 comments
Open
Labels

Comments

@HarmfulBreeze
Copy link
Contributor

HarmfulBreeze commented Jan 22, 2025

Describe the bug

Currently, many surface color roles (On surface, Container, and Add-on) are derived from the Neutral Variant tonal palette. ComposeColorScheme.kt
According to the M3 website, they should instead be derived from the Neutral palette.

I believe this is what causes the not-so-great contrast between the background and the preferences on the settings screen.

Steps to reproduce

Steps to reproduce the behavior:

  1. Open Lawnchair.
  2. Long-press the home screen.
  3. Tap Settings.
  4. Observe the contrast between the preferences and the background, and compare it to the Settings app in light mode.

Expected behavior

The surface color roles should be derived from the correct tonal palette.

Screenshots

No response

Device information

Shouldn't be relevant?

Additional context

I couldn't figure out why the Neutral palette isn't used for these color roles. If this was intended, please let me know :)

@SuperDragonXD
Copy link
Collaborator

Hello! Most of the color choices are based on Google's official Jetpack Compose default colors, which uses neutralVariant for most of the surface colors. Except for the tweak I made in ebd28fa to somewhat match Android settings, Lawnchair also follows these guidelines.

When refactoring Lawnchair Settings' theme, I was actually conflicted on whether to:

  1. Follow m3.material.io guidelines
  2. Use the colors as provided by the official library
  3. Follow AOSP settings colors (it's slightly out-of-date from the latest guidelines; can't give links rn)

I initially started with 2 (can be seen at the latest beta) but after internal discussion, we plan to somewhat emulate the AOSP settings theme & styling. I'll admit that it does require some additional tweaks to improve contrast, but that can be implemented later on.

I guess the best solution for this problem is to just test what's the best colors for each theme (as the light theme isn't really great as of now). I'll look into using the M3 colors again to see if it looks fine.

@HarmfulBreeze
Copy link
Contributor Author

Thank you for your answer, I wasn't aware that Compose used neutralVariant for these surface colors when generating color schemes.
However, it looks like it hasn't always been the case and it looks more like a bug fix than anything to me, as this change was only applied to SDKs 31-33. SDKs 34 and up were left unchanged. Please see the following commit: https://cs.android.com/androidx/platform/frameworks/support/+/7c0046970f78baa9e285bce5b02c9ec23fc60089

Regarding option 3, on A15 (with homepage_revamp aconfig flag on), AFAIK it's just system colors that are used, which matches the latest Compose behavior for SDK 34+: frameworks/base/packages/SettingsLib/SettingsTheme/res/values-v35/colors.xml

My two cents are that neutralVariant should only be used for SDKs up to 30, and above, default Compose schemes should be used instead. What do you think? (Am I missing something? 😅)

@SuperDragonXD
Copy link
Collaborator

I'll do some internal testing tommorow to see which one fits the best. Will test using the colors from the M3 website first, then do tweaks as necessary.

I could also add a temporary developer (debug) option to change the color schemes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants