From d71e6d7911ad1ea81db18d6822484cf9f06cefed Mon Sep 17 00:00:00 2001 From: vito Date: Tue, 28 Jul 2026 12:32:29 +0200 Subject: [PATCH] :sparkles: feat(reader): add a Dark sepia theme (#91) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Light, Dark and Sepia all assume you want either a bright page or a cold high-contrast one. On a phone in a dark room none of them are comfortable, and the workaround reported in #91 — pick Sepia, then turn on Android's system-wide colour inversion — inverts images and covers along with the text. Dark sepia is warm text on a warm-dark background: Calibre's own "sepia dark" values (#F6F3E9 on #39322B), since that is the scheme the request named. The pair is 11.36:1, softer than Dark's ~19.6:1 but well past the WCAG AAA floor; a test asserts it stays above AA so a later colour tweak can't quietly make the theme unreadable. It is built on Readium's DARK appearance rather than SEPIA: the sepia stylesheet blends images with mix-blend-mode: multiply, which assumes a light backdrop and smears them against a dark one. The custom colours ride on top via EpubPreferences.textColor/backgroundColor, which ReadiumCSS applies unconditionally — unlike line height and paragraph indent, they are not gated behind the publisher-styles toggle, so the theme holds either way. Two call sites needed care: - The immersive system bars keyed off `theme != DARK`, which would have painted dark icons onto the new dark background. That is now `!theme.isDark`, a property of the theme rather than an equality test. - Both theme pickers formatted labels with `name.lowercase()`, which renders DARK_SEPIA as "Dark_sepia", and laid four options out in a non-wrapping Row. They now share a displayLabel() helper and stack vertically, matching the font-family picker directly below them. The enum deliberately holds no Readium type: Readium's Theme initialises through android.graphics.Color, so storing one would drag the framework into the class initializer and force every plain JVM test that names a theme onto Robolectric. Verified on an emulator (Pixel 10, API 37): the rendered page samples exactly #39322B / #F6F3E9. Links stay cyan, as they already do in Dark — night mode forces that colour at higher specificity. --- .../ereader/ui/reader/FontSettingsSheet.kt | 8 +- .../ereader/ui/reader/ReaderScreen.kt | 2 +- .../ereader/ui/reader/ReaderThemeLabel.kt | 12 +++ .../ereader/ui/settings/SettingsScreen.kt | 10 ++- .../ereader/ui/reader/ReaderThemeLabelTest.kt | 24 +++++ .../ereader/reader/ReaderPreferences.kt | 47 +++++++++- .../reader/ReaderPreferencesStoreTest.kt | 16 ++++ .../ereader/reader/ReaderThemeTest.kt | 90 +++++++++++++++++++ 8 files changed, 200 insertions(+), 9 deletions(-) create mode 100644 app/src/main/java/io/theficos/ereader/ui/reader/ReaderThemeLabel.kt create mode 100644 app/src/test/java/io/theficos/ereader/ui/reader/ReaderThemeLabelTest.kt create mode 100644 reader/src/test/java/io/theficos/ereader/reader/ReaderThemeTest.kt 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) + } +}