diff --git a/app/src/main/java/io/theficos/ereader/ui/reader/FontSettingsSheet.kt b/app/src/main/java/io/theficos/ereader/ui/reader/FontSettingsSheet.kt index 6081655..86f40a2 100644 --- a/app/src/main/java/io/theficos/ereader/ui/reader/FontSettingsSheet.kt +++ b/app/src/main/java/io/theficos/ereader/ui/reader/FontSettingsSheet.kt @@ -125,17 +125,19 @@ fun FontSettingsSheet( } Text("Theme", style = MaterialTheme.typography.bodyMedium) - Row(verticalAlignment = Alignment.CenterVertically) { + // A column, not a row: four themes with labels as long as "Dark sepia" overflow one + // line on a narrow screen, and this matches the font-family picker just below. + Column { ReaderTheme.values().forEach { t -> Row( verticalAlignment = Alignment.CenterVertically, - modifier = Modifier.padding(end = 16.dp), + modifier = Modifier.padding(vertical = 2.dp), ) { RadioButton( selected = prefs.theme == t, onClick = { onChange(prefs.copy(theme = t)) }, ) - Text(t.name.lowercase().replaceFirstChar { it.uppercase() }) + Text(t.displayLabel()) } } } diff --git a/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt index 615919f..b9325e1 100644 --- a/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt +++ b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderScreen.kt @@ -130,7 +130,7 @@ fun ReaderScreen(viewModel: ReaderViewModel, onClose: () -> Unit) { // Scoped to the Open state so Loading/Error keep normal, themed system bars. ImmersiveWindowEffects( immersive = preferences.immersiveReading, - lightBarsForTheme = preferences.theme != io.theficos.ereader.reader.ReaderTheme.DARK, + lightBarsForTheme = !preferences.theme.isDark, chromeVisible = chromeVisible, onBeforeResize = viewModel::beginViewportResize, onResizeSettled = viewModel::completeViewportResize, diff --git a/app/src/main/java/io/theficos/ereader/ui/reader/ReaderThemeLabel.kt b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderThemeLabel.kt new file mode 100644 index 0000000..b5681d2 --- /dev/null +++ b/app/src/main/java/io/theficos/ereader/ui/reader/ReaderThemeLabel.kt @@ -0,0 +1,12 @@ +package io.theficos.ereader.ui.reader + +import io.theficos.ereader.reader.ReaderTheme + +/** + * Human-readable name for a theme: `DARK_SEPIA` -> "Dark sepia". + * + * Mirrors how the font-family pickers format [io.theficos.ereader.reader.ReaderFontFamily], and + * keeps display strings out of the reader module, which has no business knowing about labels. + */ +fun ReaderTheme.displayLabel(): String = + name.replace('_', ' ').lowercase().replaceFirstChar { it.uppercase() } diff --git a/app/src/main/java/io/theficos/ereader/ui/settings/SettingsScreen.kt b/app/src/main/java/io/theficos/ereader/ui/settings/SettingsScreen.kt index 4c22697..3da4dc8 100644 --- a/app/src/main/java/io/theficos/ereader/ui/settings/SettingsScreen.kt +++ b/app/src/main/java/io/theficos/ereader/ui/settings/SettingsScreen.kt @@ -44,6 +44,7 @@ import io.theficos.ereader.reader.ReaderFontFamily import io.theficos.ereader.reader.ReaderTheme import io.theficos.ereader.ui.components.QuireCard import io.theficos.ereader.ui.components.SectionLabel +import io.theficos.ereader.ui.reader.displayLabel import java.time.Instant import java.time.format.DateTimeParseException @@ -190,14 +191,17 @@ fun SettingsScreen( } Column { Text("Theme", style = MaterialTheme.typography.bodyMedium) - Row(verticalAlignment = Alignment.CenterVertically) { + // A column, not a row: four themes with labels as long as "Dark sepia" + // overflow one line on a narrow screen, and this matches the font-family + // picker just below. + Column { ReaderTheme.values().forEach { t -> Row( verticalAlignment = Alignment.CenterVertically, - modifier = Modifier.padding(end = 16.dp), + modifier = Modifier.padding(vertical = 2.dp), ) { RadioButton(selected = reader.theme == t, onClick = { viewModel.setTheme(t) }) - Text(t.name.lowercase().replaceFirstChar { it.uppercase() }) + Text(t.displayLabel()) } } } diff --git a/app/src/test/java/io/theficos/ereader/ui/reader/ReaderThemeLabelTest.kt b/app/src/test/java/io/theficos/ereader/ui/reader/ReaderThemeLabelTest.kt new file mode 100644 index 0000000..554cf18 --- /dev/null +++ b/app/src/test/java/io/theficos/ereader/ui/reader/ReaderThemeLabelTest.kt @@ -0,0 +1,24 @@ +package io.theficos.ereader.ui.reader + +import com.google.common.truth.Truth.assertThat +import io.theficos.ereader.reader.ReaderTheme +import org.junit.Test + +class ReaderThemeLabelTest { + + @Test fun `multi-word theme names read as words, not as enum constants`() { + assertThat(ReaderTheme.DARK_SEPIA.displayLabel()).isEqualTo("Dark sepia") + } + + @Test fun `single-word theme names are unchanged`() { + assertThat(ReaderTheme.LIGHT.displayLabel()).isEqualTo("Light") + assertThat(ReaderTheme.DARK.displayLabel()).isEqualTo("Dark") + assertThat(ReaderTheme.SEPIA.displayLabel()).isEqualTo("Sepia") + } + + @Test fun `no theme label leaks an underscore into the UI`() { + ReaderTheme.values().forEach { theme -> + assertThat(theme.displayLabel()).doesNotContain("_") + } + } +} diff --git a/reader/src/main/java/io/theficos/ereader/reader/ReaderPreferences.kt b/reader/src/main/java/io/theficos/ereader/reader/ReaderPreferences.kt index 2fd7eb9..2175fe7 100644 --- a/reader/src/main/java/io/theficos/ereader/reader/ReaderPreferences.kt +++ b/reader/src/main/java/io/theficos/ereader/reader/ReaderPreferences.kt @@ -1,10 +1,48 @@ package io.theficos.ereader.reader import org.readium.r2.navigator.epub.EpubPreferences +import org.readium.r2.navigator.preferences.Color as ReadiumColor import org.readium.r2.navigator.preferences.FontFamily as ReadiumFontFamily import org.readium.r2.navigator.preferences.Theme -enum class ReaderTheme { LIGHT, DARK, SEPIA } +/** + * A reader colour scheme. + * + * [textColor]/[backgroundColor] are null for the three schemes Readium ships natively — those + * emit no colour override, so their rendering is unchanged. A scheme that sets them layers custom + * colours on top of a base appearance: ReadiumCSS applies `--USER__textColor`/`--USER__backgroundColor` + * unconditionally (unlike line height or paragraph indent, which are gated behind + * `readium-advanced-on`), so they hold whether or not publisher styles are on. + * + * [isDark] drives the system bars, not the page: it must be true whenever the background is dark, + * which is not the same question as "is the Readium theme DARK". + */ +enum class ReaderTheme( + val isDark: Boolean, + internal val textColor: Int? = null, + internal val backgroundColor: Int? = null, +) { + LIGHT(isDark = false), + DARK(isDark = true), + SEPIA(isDark = false), + + /** + * Warm text on a warm-dark background, for reading in the dark without the glare of SEPIA or + * the cold high contrast of DARK (issue #91). Colours are Calibre's own "sepia dark" scheme, + * which is what the request asked for by name; the pair is 11.36:1, comfortably past WCAG AAA. + */ + DARK_SEPIA( + isDark = true, + textColor = 0xFFF6F3E9.toInt(), + backgroundColor = 0xFF39322B.toInt(), + ), + ; + // Deliberately no Readium type in this enum: Readium's Theme initialises itself through + // android.graphics.Color, so holding one here would drag the Android framework into the class + // initializer and force every plain JVM test that so much as names a theme onto Robolectric. + // The mapping lives in toEpubPreferences() instead, where the exhaustive `when` still fails + // compilation if a new theme forgets to declare its appearance. +} enum class ReaderFontFamily(val readium: ReadiumFontFamily?) { SYSTEM(null), @@ -50,9 +88,14 @@ fun ReaderPreferences.toEpubPreferences(): EpubPreferences = EpubPreferences( fontSize = fontScale, theme = when (theme) { ReaderTheme.LIGHT -> Theme.LIGHT - ReaderTheme.DARK -> Theme.DARK ReaderTheme.SEPIA -> Theme.SEPIA + // DARK_SEPIA rides night mode and repaints it with its own colours below. Readium's SEPIA + // appearance would be the intuitive base, but it blends images with + // `mix-blend-mode: multiply`, which assumes a light backdrop and smears them on a dark one. + ReaderTheme.DARK, ReaderTheme.DARK_SEPIA -> Theme.DARK }, + textColor = theme.textColor?.let { ReadiumColor(it) }, + backgroundColor = theme.backgroundColor?.let { ReadiumColor(it) }, fontFamily = fontFamily.readium, lineHeight = lineSpacing, pageMargins = pageMargins, diff --git a/reader/src/test/java/io/theficos/ereader/reader/ReaderPreferencesStoreTest.kt b/reader/src/test/java/io/theficos/ereader/reader/ReaderPreferencesStoreTest.kt index 201505b..64701db 100644 --- a/reader/src/test/java/io/theficos/ereader/reader/ReaderPreferencesStoreTest.kt +++ b/reader/src/test/java/io/theficos/ereader/reader/ReaderPreferencesStoreTest.kt @@ -88,6 +88,22 @@ class ReaderPreferencesStoreTest { assertThat(store2.flow.value.immersiveReading).isFalse() } + @Test fun `theme round-trips through update and reload`() { + val store1 = freshStore() + store1.update { it.copy(theme = ReaderTheme.DARK_SEPIA) } + + val store2 = ReaderPreferencesStore(context()) + assertThat(store2.flow.value.theme).isEqualTo(ReaderTheme.DARK_SEPIA) + } + + @Test fun `an unknown stored theme falls back to LIGHT`() { + // What a downgrade looks like: an older build reads DARK_SEPIA and must not crash. + rawPrefs().edit().putString("theme", "NOT_A_THEME").apply() + + val store = ReaderPreferencesStore(context()) + assertThat(store.flow.value.theme).isEqualTo(ReaderTheme.LIGHT) + } + @Test fun `out-of-range stored paragraph values are clamped on load, never throw`() { rawPrefs().edit() .putFloat("paragraph_indent", 9.0f) diff --git a/reader/src/test/java/io/theficos/ereader/reader/ReaderThemeTest.kt b/reader/src/test/java/io/theficos/ereader/reader/ReaderThemeTest.kt new file mode 100644 index 0000000..81efbf6 --- /dev/null +++ b/reader/src/test/java/io/theficos/ereader/reader/ReaderThemeTest.kt @@ -0,0 +1,90 @@ +package io.theficos.ereader.reader + +import com.google.common.truth.Truth.assertThat +import kotlin.math.max +import kotlin.math.min +import kotlin.math.pow +import org.junit.Test +import org.junit.runner.RunWith +import org.readium.r2.navigator.preferences.Theme +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +// Robolectric, despite there being nothing Android-shaped in these assertions: Readium's Theme +// initialises its colours via android.graphics.Color.parseColor, which the stub android.jar in a +// plain JVM test throws on ("not mocked"). +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [33]) +class ReaderThemeTest { + + @Test fun `each theme maps to its Readium appearance`() { + assertThat(ReaderPreferences(theme = ReaderTheme.LIGHT).toEpubPreferences().theme) + .isEqualTo(Theme.LIGHT) + assertThat(ReaderPreferences(theme = ReaderTheme.DARK).toEpubPreferences().theme) + .isEqualTo(Theme.DARK) + assertThat(ReaderPreferences(theme = ReaderTheme.SEPIA).toEpubPreferences().theme) + .isEqualTo(Theme.SEPIA) + // Dark sepia is night mode plus custom colours — Readium's SEPIA appearance blends images + // with mix-blend-mode:multiply, which assumes a light backdrop. + assertThat(ReaderPreferences(theme = ReaderTheme.DARK_SEPIA).toEpubPreferences().theme) + .isEqualTo(Theme.DARK) + } + + @Test fun `the stock themes emit no colour override`() { + // Regression guard: adding DARK_SEPIA must leave the three original themes rendering + // exactly as before, which means Readium sees null for both colours. + listOf(ReaderTheme.LIGHT, ReaderTheme.DARK, ReaderTheme.SEPIA).forEach { theme -> + val prefs = ReaderPreferences(theme = theme).toEpubPreferences() + assertThat(prefs.textColor).isNull() + assertThat(prefs.backgroundColor).isNull() + } + } + + @Test fun `dark sepia emits Calibre's sepia-dark colours`() { + val prefs = ReaderPreferences(theme = ReaderTheme.DARK_SEPIA).toEpubPreferences() + assertThat(prefs.textColor?.int).isEqualTo(0xFFF6F3E9.toInt()) + assertThat(prefs.backgroundColor?.int).isEqualTo(0xFF39322B.toInt()) + } + + @Test fun `dark sepia clears WCAG AA for body text`() { + val ratio = contrastRatio( + requireNotNull(ReaderTheme.DARK_SEPIA.textColor), + requireNotNull(ReaderTheme.DARK_SEPIA.backgroundColor), + ) + // 4.5:1 is the AA floor for body text. The chosen pair is ~11.4:1; this asserts the floor + // so a future colour tweak can't quietly make the theme unreadable. + assertThat(ratio).isGreaterThan(4.5) + } + + @Test fun `isDark is set for every theme with a dark background`() { + // Drives the immersive system bars. Getting it wrong paints dark icons on a dark + // background, which reads as "the buttons vanished". + assertThat(ReaderTheme.LIGHT.isDark).isFalse() + assertThat(ReaderTheme.SEPIA.isDark).isFalse() + assertThat(ReaderTheme.DARK.isDark).isTrue() + assertThat(ReaderTheme.DARK_SEPIA.isDark).isTrue() + } + + @Test fun `every theme declaring colours also declares both of them`() { + // A background without a matching text colour (or vice versa) inherits the other from the + // base appearance and can land on unreadable pairings. + ReaderTheme.values().forEach { theme -> + assertThat(theme.textColor == null).isEqualTo(theme.backgroundColor == null) + } + } + + /** WCAG 2.1 contrast ratio between two opaque ARGB colours. */ + private fun contrastRatio(argbA: Int, argbB: Int): Double { + val a = relativeLuminance(argbA) + val b = relativeLuminance(argbB) + return (max(a, b) + 0.05) / (min(a, b) + 0.05) + } + + private fun relativeLuminance(argb: Int): Double { + fun channel(shift: Int): Double { + val raw = (argb shr shift and 0xFF) / 255.0 + return if (raw <= 0.03928) raw / 12.92 else ((raw + 0.055) / 1.055).pow(2.4) + } + return 0.2126 * channel(16) + 0.7152 * channel(8) + 0.0722 * channel(0) + } +}