From 1c2413d4dc9c873f4bc1b5924f5afe017dbe4f35 Mon Sep 17 00:00:00 2001 From: Brian Yencho Date: Wed, 22 Nov 2023 09:40:21 -0600 Subject: [PATCH] BIT-1204: Improve avatar color visiblity (#270) --- .../ui/platform/base/util/ColorExtensions.kt | 39 ++++++++++++ .../components/BitwardenAccountActionItem.kt | 3 +- .../components/BitwardenAccountSwitcher.kt | 7 +-- .../components/model/AccountSummary.kt | 8 +++ .../platform/base/util/ColorExtensionsTest.kt | 61 +++++++++++++++++++ .../ui/platform/base/BaseComposeTest.kt | 19 ++++++ 6 files changed, 132 insertions(+), 5 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/ui/platform/base/util/ColorExtensions.kt create mode 100644 app/src/test/java/com/x8bit/bitwarden/data/platform/base/util/ColorExtensionsTest.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/base/util/ColorExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/base/util/ColorExtensions.kt new file mode 100644 index 0000000000..51db1400e0 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/base/util/ColorExtensions.kt @@ -0,0 +1,39 @@ +package com.x8bit.bitwarden.ui.platform.base.util + +import androidx.compose.material3.MaterialTheme +import androidx.compose.runtime.Composable +import androidx.compose.ui.graphics.Color +import androidx.compose.ui.graphics.luminance + +/** + * A fractional luminance value beyond which we will consider the associated color to be light + * enough to require a dark overlay to be used. + */ +private const val DARK_OVERLAY_LUMINANCE_THRESHOLD = 0.65f + +/** + * Returns `true` if the given [Color] would require a light color to be used in any kind of + * overlay when high contrast is important. + */ +val Color.isLightOverlayRequired: Boolean + get() = this.luminance() < DARK_OVERLAY_LUMINANCE_THRESHOLD + +/** + * Returns a [Color] within the current theme that can safely be overlaid on top of the given + * [Color]. + */ +@Composable +fun Color.toSafeOverlayColor(): Color { + val surfaceColor = MaterialTheme.colorScheme.surface + val onSurfaceColor = MaterialTheme.colorScheme.onSurface + val lightColor: Color + val darkColor: Color + if (surfaceColor.luminance() > onSurfaceColor.luminance()) { + lightColor = surfaceColor + darkColor = onSurfaceColor + } else { + lightColor = onSurfaceColor + darkColor = surfaceColor + } + return if (this.isLightOverlayRequired) lightColor else darkColor +} diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenAccountActionItem.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenAccountActionItem.kt index 06328a1d06..484fe14a49 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenAccountActionItem.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenAccountActionItem.kt @@ -15,6 +15,7 @@ import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import com.x8bit.bitwarden.R +import com.x8bit.bitwarden.ui.platform.base.util.toSafeOverlayColor import com.x8bit.bitwarden.ui.platform.base.util.toUnscaledTextUnit /** @@ -48,7 +49,7 @@ fun BitwardenAccountActionItem( fontFamily = FontFamily(Font(R.font.sf_pro)), fontWeight = FontWeight.W400, ), - color = colorResource(id = R.color.white), + color = color.toSafeOverlayColor(), ) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenAccountSwitcher.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenAccountSwitcher.kt index 0ac8f0f7ad..26266336e0 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenAccountSwitcher.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/BitwardenAccountSwitcher.kt @@ -29,15 +29,14 @@ import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.drawBehind -import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.lerp import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.clearAndSetSemantics import androidx.compose.ui.unit.dp -import androidx.core.graphics.toColorInt import com.x8bit.bitwarden.R import com.x8bit.bitwarden.ui.platform.base.util.lowercaseWithCurrentLocal +import com.x8bit.bitwarden.ui.platform.base.util.toSafeOverlayColor import com.x8bit.bitwarden.ui.platform.base.util.toUnscaledTextUnit import com.x8bit.bitwarden.ui.platform.components.model.AccountSummary import com.x8bit.bitwarden.ui.vault.feature.vault.util.iconRes @@ -185,7 +184,7 @@ private fun AccountSummaryItem( Icon( painter = painterResource(id = R.drawable.ic_account_initials_container), contentDescription = null, - tint = Color(accountSummary.avatarColorHex.toColorInt()), + tint = accountSummary.avatarColor, modifier = Modifier.size(40.dp), ) @@ -194,7 +193,7 @@ private fun AccountSummaryItem( style = MaterialTheme.typography.titleMedium // Do not allow scaling .copy(fontSize = 16.dp.toUnscaledTextUnit()), - color = MaterialTheme.colorScheme.surface, + color = accountSummary.avatarColor.toSafeOverlayColor(), modifier = Modifier.clearAndSetSemantics { }, ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/model/AccountSummary.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/model/AccountSummary.kt index 28cef9850a..ad9907d579 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/model/AccountSummary.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/components/model/AccountSummary.kt @@ -1,6 +1,8 @@ package com.x8bit.bitwarden.ui.platform.components.model import android.os.Parcelable +import androidx.compose.ui.graphics.Color +import com.x8bit.bitwarden.ui.platform.base.util.hexToColor import kotlinx.parcelize.Parcelize /** @@ -21,6 +23,12 @@ data class AccountSummary( val status: Status, ) : Parcelable { + /** + * The [avatarColorHex] represented as a [Color]. + */ + val avatarColor: Color + get() = avatarColorHex.hexToColor() + /** * Describes the status of the given account. */ diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/base/util/ColorExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/base/util/ColorExtensionsTest.kt new file mode 100644 index 0000000000..1dbc06224f --- /dev/null +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/base/util/ColorExtensionsTest.kt @@ -0,0 +1,61 @@ +package com.x8bit.bitwarden.data.platform.base.util + +import androidx.compose.material3.MaterialTheme +import androidx.compose.ui.graphics.Color +import com.x8bit.bitwarden.ui.platform.base.BaseComposeTest +import com.x8bit.bitwarden.ui.platform.base.util.isLightOverlayRequired +import com.x8bit.bitwarden.ui.platform.base.util.toSafeOverlayColor +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +class ColorExtensionsTest : BaseComposeTest() { + @Suppress("MaxLineLength") + @Test + fun `isLightOverlayRequired for a color with luminance below the light threshold should return true`() { + assertTrue(Color.Blue.isLightOverlayRequired) + } + + @Suppress("MaxLineLength") + @Test + fun `isLightOverlayRequired for a color with luminance above the light threshold should return false`() { + assertFalse(Color.Yellow.isLightOverlayRequired) + } + + @Test + fun `toSafeOverlayColor for a dark color in light mode should use the surface color`() = + runTestWithTheme(isDarkTheme = false) { + assertEquals( + MaterialTheme.colorScheme.surface, + Color.Blue.toSafeOverlayColor(), + ) + } + + @Test + fun `toSafeOverlayColor for a dark color in dark mode should use the onSurface color`() = + runTestWithTheme(isDarkTheme = true) { + assertEquals( + MaterialTheme.colorScheme.onSurface, + Color.Blue.toSafeOverlayColor(), + ) + } + + @Test + fun `toSafeOverlayColor for a light color in light mode should use the onSurface color`() = + runTestWithTheme(isDarkTheme = false) { + assertEquals( + MaterialTheme.colorScheme.onSurface, + Color.Yellow.toSafeOverlayColor(), + ) + } + + @Test + fun `toSafeOverlayColor for a light color in dark mode should use the surface color`() = + runTestWithTheme(isDarkTheme = true) { + assertEquals( + MaterialTheme.colorScheme.surface, + Color.Yellow.toSafeOverlayColor(), + ) + } +} diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/base/BaseComposeTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/base/BaseComposeTest.kt index 06c3db60a2..1fae5c1805 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/base/BaseComposeTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/base/BaseComposeTest.kt @@ -1,6 +1,8 @@ package com.x8bit.bitwarden.ui.platform.base +import androidx.compose.runtime.Composable import androidx.compose.ui.test.junit4.createComposeRule +import com.x8bit.bitwarden.ui.platform.theme.BitwardenTheme import dagger.hilt.android.testing.HiltTestApplication import org.junit.Rule import org.junit.runner.RunWith @@ -24,4 +26,21 @@ abstract class BaseComposeTest { init { ShadowLog.stream = System.out } + + /** + * Helper for testing a basic Composable function that only requires a Composable environment + * with the [BitwardenTheme]. + */ + protected fun runTestWithTheme( + isDarkTheme: Boolean, + test: @Composable () -> Unit, + ) { + composeTestRule.setContent { + BitwardenTheme( + darkTheme = isDarkTheme, + ) { + test() + } + } + } }