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

Adding Dynamic Colors Material 3 Material You Theme #21

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hanthor
Copy link

@hanthor hanthor commented Jul 18, 2024

  • add support to the RichTextEditorStyle to distiguish between the two kinds of messages you and others
  • maybe add pureblack background option?

@SpiritCroc
Copy link
Member

Nice, you found ScPrefs 👍

One of the most important aspects for SchildiChat these days is keeping code easily maintainable in a long term, while avoiding upstream merge conflicts as much as possible. So your changes to ColorAliases.kt would in the current state be a blocker.

  • Added functions can go into a file called ScColorAliasesExtensions.kt in the same package, so they can easily be used the same way while not having to touch the upstream file.
  • For all the color overrides, these should each be possible with only one line modified. Never delete upstream code lines, only modify or comment out if really necessary. My recommended one-liner way would be something like
    get() = scOverride() ?: if (isLight) ...
    such that you only add scFunction() ?: in front of Element code. Then you can define the sc function in ScColorAliasesExtensions.kt, and make it return null for the cases we do not need/want to modify such that it falls back to upstream code just in that case. Similar to 9b712b4 where I don't use a function but an exposed value directly, but I guess you need a function here.

More precisely, I could imagine it something like this (untested code, may need some modifications):
Your code:

val SemanticColors.pinDigitBg
    get() = if (isLight) {
    @Composable
    get() = if (DynamicColorPreferences.isDynamicColorEnabled) {
        colorScheme.onPrimaryContainer
    } else if (isLight) {
        Color(0xFFF0F2F5)
    } else {
        Color(0xFF26282D)

gets into

val SemanticColors.pinDigitBg
    get() = scSemColorOverride(colorScheme.onPrimaryContainer) ?: if (isLight) {
        // We want LightDesignTokens.colorGray300
        Color(0xFFF0F2F5)
    } else {
        // We want DarkDesignTokens.colorGray400
        Color(0xFF26282D)
    }

where only scSemColorOverride(colorScheme.onPrimaryContainer) ?: was inserted, rest is still upstream,
with

@Composable
fun scSemColorOverride(dynamicThemeColor: Color) = dynamicThemeColor.takeIf { DynamicColorPreferences.isDynamicColorEnabled }

in an SC-owned file.

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

Successfully merging this pull request may close these issues.

2 participants