-
Notifications
You must be signed in to change notification settings - Fork 177
feat: dynamic colors #864
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
base: master
Are you sure you want to change the base?
feat: dynamic colors #864
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,13 +19,15 @@ | |
| package dev.octoshrimpy.quik.common.util | ||
|
|
||
| import android.content.Context | ||
| import android.content.res.Configuration | ||
| import android.graphics.Color | ||
| import androidx.core.content.res.getColorOrThrow | ||
| import dev.octoshrimpy.quik.R | ||
| import dev.octoshrimpy.quik.common.util.extensions.getColorCompat | ||
| import dev.octoshrimpy.quik.model.Recipient | ||
| import dev.octoshrimpy.quik.util.Preferences | ||
| import io.reactivex.Observable | ||
| import io.reactivex.rxkotlin.Observables | ||
| import javax.inject.Inject | ||
| import javax.inject.Singleton | ||
| import kotlin.math.absoluteValue | ||
|
|
@@ -37,6 +39,9 @@ class Colors @Inject constructor( | |
| private val prefs: Preferences | ||
| ) { | ||
|
|
||
| val dynamicColorsSupported: Boolean | ||
| get() = context.resources.getBoolean(R.bool.dynamic_colors_supported) | ||
|
|
||
| data class Theme(val theme: Int, private val colors: Colors) { | ||
| val highlight by lazy { colors.highlightColorForTheme(theme) } | ||
| val textPrimary by lazy { colors.textPrimaryOnThemeForColor(theme) } | ||
|
|
@@ -80,7 +85,8 @@ class Colors @Inject constructor( | |
| fun theme(recipient: Recipient? = null): Theme { | ||
| val pref = prefs.theme(recipient?.id ?: 0) | ||
| val color = when { | ||
| recipient == null || !prefs.autoColor.get() || pref.isSet -> pref.get() | ||
| recipient == null -> dynamicThemeColor() ?: pref.get() | ||
| !prefs.autoColor.get() || pref.isSet -> pref.get() | ||
| else -> generateColor(recipient) | ||
| } | ||
| return Theme(color, this) | ||
|
|
@@ -92,10 +98,41 @@ class Colors @Inject constructor( | |
| prefs.autoColor.get() -> prefs.theme(recipient.id, generateColor(recipient)) | ||
| else -> prefs.theme(recipient.id, prefs.theme().get()) | ||
| } | ||
| return pref.asObservable() | ||
| val colors = when { | ||
| recipient == null -> Observables.combineLatest( | ||
| pref.asObservable(), | ||
| prefs.dynamicColors.asObservable() | ||
| ) { color, _ -> dynamicThemeColor() ?: color } | ||
| else -> pref.asObservable() | ||
| } | ||
| return colors | ||
| .map { color -> Theme(color, this) } | ||
| } | ||
|
|
||
| private fun dynamicThemeColor(): Int? { | ||
| val isNight = prefs.night.get() || | ||
| (context.resources.configuration.uiMode and Configuration.UI_MODE_NIGHT_MASK) == | ||
| Configuration.UI_MODE_NIGHT_YES | ||
|
|
||
| return dynamicColor( | ||
| android.R.color.system_accent1_600, | ||
| android.R.color.system_accent1_200, | ||
|
Comment on lines
+118
to
+119
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible not to hardcode these?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see if there is a more natural way to handle this when exploring the Material 3 migration. |
||
| isNight | ||
| ) | ||
| } | ||
|
|
||
| fun dynamicBackgroundColor(isNight: Boolean): Int? = dynamicColor( | ||
| android.R.color.system_neutral1_10, | ||
| android.R.color.system_neutral1_900, | ||
| isNight | ||
| ) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it possible to not hardcode these?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as #864 (comment). |
||
|
|
||
| private fun dynamicColor(lightColor: Int, darkColor: Int, isNight: Boolean): Int? { | ||
| if (!dynamicColorsSupported || !prefs.dynamicColors.get()) return null | ||
|
|
||
| return context.getColor(if (isNight) darkColor else lightColor) | ||
| } | ||
|
|
||
| fun highlightColorForTheme(theme: Int): Int = FloatArray(3) | ||
| .apply { Color.colorToHSV(theme, this) } | ||
| .let { hsv -> hsv.apply { set(2, 0.75f) } } // 75% value | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ package dev.octoshrimpy.quik.feature.qkreply | |
|
|
||
| import android.app.Activity | ||
| import android.content.Intent | ||
| import android.os.Build | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this import needed?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I missed cleaning that up from the initial use of https://developer.android.com/reference/android/os/Build.VERSION. Thanks for catching it! |
||
| import android.os.Bundle | ||
| import android.speech.RecognizerIntent | ||
| import android.view.GestureDetector | ||
|
|
@@ -177,6 +178,7 @@ class QkReplyActivity : QkThemedActivity(), QkReplyView { | |
|
|
||
| override fun getActivityThemeRes(black: Boolean) = when { | ||
| black -> R.style.AppThemeDialog_Black | ||
| colors.dynamicColorsSupported && prefs.dynamicColors.get() -> R.style.AppThemeDialog_Dynamic | ||
| else -> R.style.AppThemeDialog | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <resources> | ||
|
|
||
| <style name="AppTheme.Dynamic" parent="AppTheme"> | ||
| <item name="android:colorBackground">@android:color/system_neutral1_900</item> | ||
| <item name="android:navigationBarColor">@android:color/system_neutral1_900</item> | ||
| <item name="android:statusBarColor">@android:color/system_neutral1_900</item> | ||
| <item name="android:windowBackground">@android:color/system_neutral1_900</item> | ||
| <item name="android:queryBackground">@android:color/system_neutral2_800</item> | ||
| <item name="bubbleColor">@android:color/system_neutral2_800</item> | ||
| <item name="colorPrimary">@android:color/system_neutral1_900</item> | ||
| <item name="colorPrimaryDark">@android:color/system_neutral1_900</item> | ||
| </style> | ||
|
|
||
| <style name="AppThemeDialog.Dynamic" parent="AppThemeDialog"> | ||
| <item name="android:colorBackground">@android:color/system_neutral1_900</item> | ||
| <item name="android:windowBackground">@android:color/system_neutral1_900</item> | ||
| <item name="actionBarPopupTheme">@style/PopupTheme.Dynamic</item> | ||
| <item name="bubbleColor">@android:color/system_neutral2_800</item> | ||
| <item name="colorPrimary">@android:color/system_neutral1_900</item> | ||
| <item name="colorPrimaryDark">@android:color/system_neutral1_900</item> | ||
| </style> | ||
|
|
||
| <style name="PopupTheme.Dynamic" parent="PopupTheme"> | ||
| <item name="android:colorBackground">@android:color/system_neutral1_900</item> | ||
| <item name="android:textColor">@color/textPrimaryDark</item> | ||
| <item name="bubbleColor">@android:color/system_neutral2_800</item> | ||
| </style> | ||
|
|
||
| </resources> |
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the point of this file? If we are going to merge it, we intend to support it. I would prefer you remove this.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #864 (comment). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <resources> | ||
| <bool name="dynamic_colors_supported">true</bool> | ||
| </resources> |
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use Material Components attributes here instead? If you are unwilling to switch to Material colors, I can potentially do that. I would just prefer, if we are updating the theming engine, then maybe we should update to Material 3 as well?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll explore a larger migration/refactor soon, as mentioned in #864 (comment). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <resources> | ||
|
|
||
| <style name="AppTheme.Dynamic" parent="AppTheme"> | ||
| <item name="android:colorBackground">@android:color/system_neutral1_10</item> | ||
| <item name="android:navigationBarColor">@android:color/system_neutral1_10</item> | ||
| <item name="android:statusBarColor">@android:color/system_neutral1_10</item> | ||
| <item name="android:windowBackground">@android:color/system_neutral1_10</item> | ||
| <item name="android:queryBackground">@android:color/system_neutral2_100</item> | ||
| <item name="bubbleColor">@android:color/system_neutral2_100</item> | ||
| <item name="colorPrimary">@android:color/system_neutral1_10</item> | ||
| <item name="colorPrimaryDark">@android:color/system_neutral1_10</item> | ||
| </style> | ||
|
|
||
| <style name="AppThemeDialog.Dynamic" parent="AppThemeDialog"> | ||
| <item name="android:colorBackground">@android:color/system_neutral1_10</item> | ||
| <item name="android:windowBackground">@android:color/system_neutral1_10</item> | ||
| <item name="actionBarPopupTheme">@style/PopupTheme.Dynamic</item> | ||
| <item name="bubbleColor">@android:color/system_neutral2_100</item> | ||
| <item name="colorPrimary">@android:color/system_neutral1_10</item> | ||
| <item name="colorPrimaryDark">@android:color/system_neutral1_10</item> | ||
| </style> | ||
|
|
||
| <style name="PopupTheme.Dynamic" parent="PopupTheme"> | ||
| <item name="android:colorBackground">@android:color/system_neutral1_10</item> | ||
| <item name="bubbleColor">@android:color/system_neutral2_100</item> | ||
| </style> | ||
|
|
||
| </resources> |
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, not needed.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #864 (comment). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <resources> | ||
| <bool name="dynamic_colors_supported">false</bool> | ||
| </resources> |
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.
I don't think supported should matter based on a config, rather it should matter based on android version.
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.
Initially I had this as an OS version check, but this approach felt more natural to me.
My reasoning is that we need new SDK v31 (Android 12) theme files for Dynamic Color support, and they get automatically preferred based on the OS version. So by having this bool file, the OS level determines which resource files to use and this comes along with it. This boolean will be available everywhere that the values are.
I do see your point about the file just containing a boolean though. Wrapping a version check in a properly named function could probably also be sufficient.
I'm happy to go either way if you have a strong preference.
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.
I think the version check would be better, as that is the pattern in the rest of the codebase, and is more idiomatic to Android. Nice thought though.