From f9c0da2e08bd6102938d290befde6de142ac8f6c Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Tue, 26 May 2026 12:11:36 +0200 Subject: [PATCH] Add unit tests and migrate legacy per-row spacing on decode Restores TestHomeRowSamples.kt (sample-data + sanity tests that survived an earlier soft-reset on disk but never got staged). Updates the RecentlyAdded sample to use the new cardSizeMultiplier and spacingMultiplier fields. Adds InterfaceHelpersTest.kt covering: - Int.withinBoundsOrDefault (in-range, below-min, above-max, boundary equality) for both CardSize and Spacing preferences. - resolveCardHeight (newly extracted pure helper underneath the composable resolvedCardHeight): sentinel 0, legacy 172, explicit per-row override, multiplier path. - InterfaceCustomization.cardHeightDp / spacingDp math. - migrateLegacyRowSpacing: absent / default / non-default / snapping / clamping / does-not-overwrite-existing-multiplier. Adds migrateLegacyRowSpacing in HomeSettingsService and wires it into decode(): when a row's raw JSON still carries the legacy "spacing" field (from before this PR deleted the absolute spacing data field), translate the value into the new spacingMultiplier so users keep the same visual gap after upgrade. Extracts resolveCardHeight as a pure helper from the @Composable resolvedCardHeight so the resolution math is testable without the Compose runtime. --- .../wholphin/services/HomeSettingsService.kt | 28 ++- .../wholphin/ui/util/InterfaceHelpers.kt | 22 ++- .../wholphin/test/TestHomeRowSamples.kt | 3 +- .../wholphin/ui/util/InterfaceHelpersTest.kt | 170 ++++++++++++++++++ 4 files changed, 216 insertions(+), 7 deletions(-) create mode 100644 app/src/test/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpersTest.kt diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt index 4288e337..997cb7dd 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt @@ -194,7 +194,14 @@ class HomeSettingsService rowsElement ?.mapNotNull { row -> try { - jsonParser.decodeFromJsonElement(row) + val config = jsonParser.decodeFromJsonElement(row) + val legacySpacing = + row.jsonObject["viewOptions"] + ?.jsonObject + ?.get("spacing") + ?.jsonPrimitive + ?.intOrNull + migrateLegacyRowSpacing(config, legacySpacing) } catch (ex: Exception) { Timber.w(ex, "Unknown row %s", row) // TODO maybe use placeholder instead of null? @@ -1179,6 +1186,25 @@ class UnsupportedHomeSettingsVersionException( val maxSupportedVersion: Int = SUPPORTED_HOME_PAGE_SETTINGS_VERSION, ) : Exception("Unsupported version $unsupportedVersion, max supported is $maxSupportedVersion") +private const val LEGACY_DEFAULT_ROW_SPACING_DP = 16 +private const val MULTIPLIER_MIN = 50 +private const val MULTIPLIER_MAX = 150 +private const val MULTIPLIER_STEP = 5 + +internal fun migrateLegacyRowSpacing( + config: HomeRowConfig, + legacySpacingDp: Int?, +): HomeRowConfig { + if (legacySpacingDp == null || legacySpacingDp == LEGACY_DEFAULT_ROW_SPACING_DP) return config + if (config.viewOptions.spacingMultiplier != 100) return config + val raw = + (legacySpacingDp * 100 / LEGACY_DEFAULT_ROW_SPACING_DP) + .coerceIn(MULTIPLIER_MIN, MULTIPLIER_MAX) + val snapped = (raw / MULTIPLIER_STEP) * MULTIPLIER_STEP + if (snapped == 100) return config + return config.updateViewOptions(config.viewOptions.copy(spacingMultiplier = snapped)) +} + fun getRecentlyAddedTitle(library: Library?): StringProvider = if (library?.isRecordingFolder == true) { ResStringProvider(R.string.recently_recorded) diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpers.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpers.kt index 062237dc..cf44117a 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpers.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpers.kt @@ -8,11 +8,23 @@ import com.github.damontecres.wholphin.preferences.AppSliderPreference import com.github.damontecres.wholphin.ui.Cards @Composable -internal fun HomeRowViewOptions.resolvedCardHeight(): Dp { - val global = LocalInterfaceCustomization.current.cardHeightDp - val isOverride = heightDp > 0 && heightDp != Cards.HEIGHT_2X3_DP - val base = if (isOverride) heightDp else global - return (base * cardSizeMultiplier / 100).dp +internal fun HomeRowViewOptions.resolvedCardHeight(): Dp = + resolveCardHeight( + heightDp = heightDp, + cardSizeMultiplier = cardSizeMultiplier, + globalCardHeightDp = LocalInterfaceCustomization.current.cardHeightDp, + baseCardHeightDp = Cards.HEIGHT_2X3_DP, + ).dp + +internal fun resolveCardHeight( + heightDp: Int, + cardSizeMultiplier: Int, + globalCardHeightDp: Int, + baseCardHeightDp: Int, +): Int { + val isOverride = heightDp > 0 && heightDp != baseCardHeightDp + val base = if (isOverride) heightDp else globalCardHeightDp + return base * cardSizeMultiplier / 100 } internal fun Int.withinBoundsOrDefault(preference: AppSliderPreference<*>): Int { diff --git a/app/src/test/java/com/github/damontecres/wholphin/test/TestHomeRowSamples.kt b/app/src/test/java/com/github/damontecres/wholphin/test/TestHomeRowSamples.kt index e8c2fed1..61677414 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/test/TestHomeRowSamples.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/test/TestHomeRowSamples.kt @@ -27,7 +27,8 @@ class TestHomeRowSamples { viewOptions = HomeRowViewOptions( heightDp = 100, - spacing = 8, + cardSizeMultiplier = 110, + spacingMultiplier = 50, contentScale = PrefContentScale.CROP, aspectRatio = AspectRatio.FOUR_THREE, imageType = ViewOptionImageType.THUMB, diff --git a/app/src/test/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpersTest.kt b/app/src/test/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpersTest.kt new file mode 100644 index 00000000..0163509a --- /dev/null +++ b/app/src/test/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpersTest.kt @@ -0,0 +1,170 @@ +package com.github.damontecres.wholphin.ui.util + +import com.github.damontecres.wholphin.data.model.HomeRowConfig +import com.github.damontecres.wholphin.data.model.HomeRowViewOptions +import com.github.damontecres.wholphin.preferences.AppPreference +import com.github.damontecres.wholphin.preferences.AppPreferences +import com.github.damontecres.wholphin.services.migrateLegacyRowSpacing +import com.github.damontecres.wholphin.ui.Cards +import org.jellyfin.sdk.model.UUID +import org.junit.Assert.assertEquals +import org.junit.Test + +class InterfaceHelpersTest { + @Test + fun `withinBoundsOrDefault returns value when in range`() { + assertEquals(100, 100.withinBoundsOrDefault(AppPreference.CardSize)) + assertEquals(75, 75.withinBoundsOrDefault(AppPreference.CardSize)) + assertEquals(150, 150.withinBoundsOrDefault(AppPreference.CardSize)) + } + + @Test + fun `withinBoundsOrDefault returns default when below min`() { + // CardSize is 50..150 default 100; Spacing is 25..175 default 100. + assertEquals(100, 49.withinBoundsOrDefault(AppPreference.CardSize)) + assertEquals(100, 0.withinBoundsOrDefault(AppPreference.Spacing)) + } + + @Test + fun `withinBoundsOrDefault returns default when above max`() { + assertEquals(100, 151.withinBoundsOrDefault(AppPreference.CardSize)) + assertEquals(100, 200.withinBoundsOrDefault(AppPreference.Spacing)) + } + + @Test + fun `withinBoundsOrDefault includes both bounds inclusively`() { + // Sanity: min and max themselves are accepted, NOT replaced with default. + assertEquals(50, 50.withinBoundsOrDefault(AppPreference.CardSize)) + assertEquals(150, 150.withinBoundsOrDefault(AppPreference.CardSize)) + assertEquals(25, 25.withinBoundsOrDefault(AppPreference.Spacing)) + assertEquals(175, 175.withinBoundsOrDefault(AppPreference.Spacing)) + } + + @Test + fun `resolveCardHeight uses global when heightDp is sentinel 0`() { + assertEquals(172, resolveCardHeight(0, 100, 172, Cards.HEIGHT_2X3_DP)) + assertEquals(258, resolveCardHeight(0, 100, 258, Cards.HEIGHT_2X3_DP)) // global at 150% + } + + @Test + fun `resolveCardHeight uses global when heightDp equals legacy default 172`() { + // A1 fix: legacy stored heightDp=172 (the pre-PR default) is treated as "no override". + assertEquals(172, resolveCardHeight(Cards.HEIGHT_2X3_DP, 100, 172, Cards.HEIGHT_2X3_DP)) + assertEquals(258, resolveCardHeight(Cards.HEIGHT_2X3_DP, 100, 258, Cards.HEIGHT_2X3_DP)) + } + + @Test + fun `resolveCardHeight honours non-default per-row override`() { + // Display Presets set absolute per-row heights like 128 or 148. + assertEquals(128, resolveCardHeight(128, 100, 172, Cards.HEIGHT_2X3_DP)) + assertEquals(148, resolveCardHeight(148, 100, 172, Cards.HEIGHT_2X3_DP)) + } + + @Test + fun `resolveCardHeight applies cardSizeMultiplier on top of global`() { + // Global card height 172, multiplier 120 -> 206 dp. + assertEquals(206, resolveCardHeight(0, 120, 172, Cards.HEIGHT_2X3_DP)) + // Global 172, multiplier 75 -> 129. + assertEquals(129, resolveCardHeight(0, 75, 172, Cards.HEIGHT_2X3_DP)) + } + + @Test + fun `resolveCardHeight applies cardSizeMultiplier on top of per-row override`() { + // Preset row at 148 with multiplier 120 -> 177. + assertEquals(177, resolveCardHeight(148, 120, 172, Cards.HEIGHT_2X3_DP)) + // Episode preset at 128 with multiplier 50 -> 64. + assertEquals(64, resolveCardHeight(128, 50, 172, Cards.HEIGHT_2X3_DP)) + } + + @Test + fun `InterfaceCustomization cardHeightDp computes global from slider percent`() { + val customization = InterfaceCustomization(prefs = AppPreferences.getDefaultInstance()) + // Default cardSizePercent is 100, so cardHeightDp == HEIGHT_2X3_DP. + assertEquals(Cards.HEIGHT_2X3_DP, customization.cardHeightDp) + } + + @Test + fun `InterfaceCustomization cardHeightDp scales linearly`() { + val customization = + InterfaceCustomization( + enabledDisplayToggles = + java.util.EnumSet.noneOf( + com.github.damontecres.wholphin.preferences.DisplayToggle::class.java, + ), + cardSizePercent = 150, + ) + // 172 * 150 / 100 = 258. + assertEquals(258, customization.cardHeightDp) + } + + @Test + fun `InterfaceCustomization spacingDp scales linearly`() { + val customization = + InterfaceCustomization( + enabledDisplayToggles = + java.util.EnumSet.noneOf( + com.github.damontecres.wholphin.preferences.DisplayToggle::class.java, + ), + spacingPercent = 175, + ) + // BASE_SPACING_DP = 16. 16 * 175 / 100 = 28. + assertEquals(28, customization.spacingDp) + } + + @Test + fun `migrateLegacyRowSpacing preserves row when legacy field is absent`() { + val row = sampleRow(spacingMultiplier = 100) + val result = migrateLegacyRowSpacing(row, legacySpacingDp = null) + assertEquals(100, result.viewOptions.spacingMultiplier) + } + + @Test + fun `migrateLegacyRowSpacing preserves row when legacy value matches the default`() { + val row = sampleRow(spacingMultiplier = 100) + val result = migrateLegacyRowSpacing(row, legacySpacingDp = 16) + assertEquals(100, result.viewOptions.spacingMultiplier) + } + + @Test + fun `migrateLegacyRowSpacing translates legacy spacing into spacingMultiplier`() { + val row = sampleRow(spacingMultiplier = 100) + // legacy 20 dp -> 20 / 16 * 100 = 125 -> snaps to 125. + val result = migrateLegacyRowSpacing(row, legacySpacingDp = 20) + assertEquals(125, result.viewOptions.spacingMultiplier) + } + + @Test + fun `migrateLegacyRowSpacing snaps to nearest step of 5`() { + val row = sampleRow(spacingMultiplier = 100) + // legacy 18 dp -> 18 / 16 * 100 = 112 -> floor-snap to 110. + val result = migrateLegacyRowSpacing(row, legacySpacingDp = 18) + assertEquals(110, result.viewOptions.spacingMultiplier) + } + + @Test + fun `migrateLegacyRowSpacing clamps extreme values into 50-150 range`() { + val rowLow = sampleRow(spacingMultiplier = 100) + // legacy 4 dp -> 25 -> clamped to 50. + val resultLow = migrateLegacyRowSpacing(rowLow, legacySpacingDp = 4) + assertEquals(50, resultLow.viewOptions.spacingMultiplier) + + val rowHigh = sampleRow(spacingMultiplier = 100) + // legacy 40 dp -> 250 -> clamped to 150. + val resultHigh = migrateLegacyRowSpacing(rowHigh, legacySpacingDp = 40) + assertEquals(150, resultHigh.viewOptions.spacingMultiplier) + } + + @Test + fun `migrateLegacyRowSpacing does not overwrite a non-default spacingMultiplier`() { + // If the user has already set a new-model multiplier, the legacy value must not clobber it. + val row = sampleRow(spacingMultiplier = 120) + val result = migrateLegacyRowSpacing(row, legacySpacingDp = 24) + assertEquals(120, result.viewOptions.spacingMultiplier) + } + + private fun sampleRow(spacingMultiplier: Int): HomeRowConfig = + HomeRowConfig.RecentlyAdded( + parentId = UUID.randomUUID(), + viewOptions = HomeRowViewOptions(spacingMultiplier = spacingMultiplier), + ) +}