diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/UISharedPreferences.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/UISharedPreferences.kt index 21670d0ce4..7a9de9483e 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/UISharedPreferences.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/preferences/UISharedPreferences.kt @@ -218,13 +218,25 @@ class UiSharedPreferences( if (raw.isBlank()) return emptyList() // Current format: a JSON list of BottomBarEntry (built-ins + favorites). runCatching { return JsonMapper.fromJson>(raw) } - // Legacy format: comma-joined NavBarItem enum names (before favorites/unified entries). - return runCatching { - raw - .split(",") - .mapNotNull { name -> runCatching { NavBarItem.valueOf(name) }.getOrNull() } - .map { BottomBarEntry.BuiltIn(it) } - }.getOrNull() + // Configs written before the stable @SerialName discriminators used the fully-qualified + // class name as the polymorphic "type" value. Rewrite it to the short name and retry, so a + // customized bar survives the upgrade instead of silently resetting to defaults. + runCatching { + val migrated = + raw + .replace(LEGACY_BUILTIN_DISCRIMINATOR, "builtIn") + .replace(LEGACY_FAVORITE_DISCRIMINATOR, "favorite") + return JsonMapper.fromJson>(migrated) + } + // Oldest format: comma-joined NavBarItem enum names (before favorites/unified entries). + val legacy = raw.split(",").mapNotNull { name -> runCatching { NavBarItem.valueOf(name) }.getOrNull() } + if (legacy.isNotEmpty()) return legacy.map { BottomBarEntry.BuiltIn(it) } + // Unrecognizable — fall back to the defaults rather than leaving the bar empty. + return DefaultBottomBarEntries } + + // The pre-@SerialName polymorphic discriminators (fully-qualified class names) for migration. + private const val LEGACY_BUILTIN_DISCRIMINATOR = "com.vitorpamplona.amethyst.ui.navigation.bottombars.BottomBarEntry.BuiltIn" + private const val LEGACY_FAVORITE_DISCRIMINATOR = "com.vitorpamplona.amethyst.ui.navigation.bottombars.BottomBarEntry.Favorite" } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedTabLayer.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedTabLayer.kt index fdc0c652e6..1b58b21d8d 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedTabLayer.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/EmbeddedTabLayer.kt @@ -23,6 +23,8 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.embed import android.os.Build import android.view.ViewGroup import androidx.annotation.RequiresApi +import androidx.compose.foundation.clickable +import androidx.compose.foundation.interaction.MutableInteractionSource import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.absoluteOffset import androidx.compose.foundation.layout.fillMaxSize @@ -126,23 +128,36 @@ fun EmbeddedTabLayer(barFavoriteIds: List) { } // The active tab's top pull-down sheet, drawn AFTER the surfaces so it sits on top of the - // (z-below) surface, anchored to the top of the active tab's reserved bounds. + // (z-below) surface, anchored to the top of the active tab's reserved bounds. Its expanded state + // is owned here (reset per tab) so we can draw a full-area dismiss scrim behind the open sheet — + // while collapsed, only the small grabber is interactive and page taps pass through. val chrome = EmbeddedTabHost.activeChrome if (chrome != null && bounds.width > 0f && bounds.height > 0f) { + var sheetExpanded by remember(activeId) { mutableStateOf(false) } + + if (sheetExpanded) { + Box( + Modifier + .fillMaxSize() + .clickable( + interactionSource = remember { MutableInteractionSource() }, + indication = null, + ) { sheetExpanded = false }, + ) + } + with(density) { - // Key on the active tab so the sheet's expand/collapse state resets per tab instead of - // a previous tab's open sheet bleeding into the next. - key(activeId) { - TopControlSheet( - chrome = chrome, - modifier = - Modifier - .absoluteOffset( - (bounds.left - layerOrigin.x).toDp(), - (bounds.top - layerOrigin.y).toDp(), - ).width(bounds.width.toDp()), - ) - } + TopControlSheet( + chrome = chrome, + expanded = sheetExpanded, + onExpandedChange = { sheetExpanded = it }, + modifier = + Modifier + .absoluteOffset( + (bounds.left - layerOrigin.x).toDp(), + (bounds.top - layerOrigin.y).toDp(), + ).width(bounds.width.toDp()), + ) } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/TopControlSheet.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/TopControlSheet.kt index 5df8e01390..ccc2d26868 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/TopControlSheet.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/embed/TopControlSheet.kt @@ -46,8 +46,6 @@ import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -69,10 +67,10 @@ import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols @Composable fun TopControlSheet( chrome: EmbeddedTabChrome, + expanded: Boolean, + onExpandedChange: (Boolean) -> Unit, modifier: Modifier = Modifier, ) { - var expanded by remember { mutableStateOf(false) } - Column( modifier = modifier.fillMaxWidth(), horizontalAlignment = Alignment.CenterHorizontally, @@ -115,40 +113,41 @@ fun TopControlSheet( ) } SheetItem(MaterialSymbols.Refresh, stringResource(R.string.browser_reload)) { - expanded = false + onExpandedChange(false) chrome.onReload() } chrome.onInfo?.let { info -> SheetItem(MaterialSymbols.Info, stringResource(R.string.favorite_app_access_show)) { - expanded = false + onExpandedChange(false) info() } } SheetItem(MaterialSymbols.OpenInFull, stringResource(R.string.favorite_app_open_window)) { - expanded = false + onExpandedChange(false) chrome.onOpenFull() } } } } - // The grabber: a small rounded bar centered at the top edge. Pull down to open, up to close; - // tapping toggles. Wrapped in padding so the touch target is comfortable despite the thin bar. + // The grabber: a small rounded bar centered at the top edge. It is the ONLY touch target the sheet + // draws — the rest of the top strip stays transparent so page taps pass straight through to the + // surface below. Pull down to open, up to close; tapping toggles. Column( horizontalAlignment = Alignment.CenterHorizontally, modifier = Modifier .clip(RoundedCornerShape(bottomStart = 12.dp, bottomEnd = 12.dp)) .background(MaterialTheme.colorScheme.surface.copy(alpha = 0.6f)) - .clickable { expanded = !expanded } + .clickable { onExpandedChange(!expanded) } .draggable( orientation = Orientation.Vertical, state = rememberDraggableState { delta -> if (delta > 1f) { - expanded = true + onExpandedChange(true) } else if (delta < -1f) { - expanded = false + onExpandedChange(false) } }, ).padding(horizontal = 16.dp, vertical = 7.dp), diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/navigation/BottomBarEntrySerializationTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/navigation/BottomBarEntrySerializationTest.kt new file mode 100644 index 0000000000..e4da67a3ad --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/navigation/BottomBarEntrySerializationTest.kt @@ -0,0 +1,73 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.navigation + +import com.vitorpamplona.amethyst.ui.navigation.bottombars.BottomBarEntry +import com.vitorpamplona.amethyst.ui.navigation.bottombars.NavBarItem +import com.vitorpamplona.quartz.nip01Core.core.JsonMapper +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Locks the persisted bottom-bar format. [BottomBarEntry] uses stable `@SerialName` discriminators + * (`builtIn` / `favorite`) so the saved config survives class renames/moves. Configs written before the + * stable names used the fully-qualified class name as the polymorphic discriminator, so this also pins + * the migration that recovers them — otherwise upgrading silently resets the user's bottom bar. + */ +class BottomBarEntrySerializationTest { + private val sample = + listOf( + BottomBarEntry.BuiltIn(NavBarItem.HOME), + BottomBarEntry.Favorite("url:https://example.com"), + ) + + @Test + fun roundTripsWithStableDiscriminators() { + val json = JsonMapper.toJson(sample) + // Stable short names, NOT the fragile fully-qualified class name. + assertTrue("expected stable discriminators, got: $json", json.contains("\"builtIn\"") && json.contains("\"favorite\"")) + assertEquals(sample, JsonMapper.fromJson>(json)) + } + + @Test + fun legacyFullyQualifiedDiscriminatorMigratesInsteadOfResetting() { + // What an earlier build of this branch persisted: the default polymorphic discriminator is the + // fully-qualified class name. Under the stable @SerialName this no longer decodes directly... + val legacy = + """ + [ + {"type":"com.vitorpamplona.amethyst.ui.navigation.bottombars.BottomBarEntry.BuiltIn","item":"HOME"}, + {"type":"com.vitorpamplona.amethyst.ui.navigation.bottombars.BottomBarEntry.Favorite","favoriteId":"url:https://example.com"} + ] + """.trimIndent() + + runCatching { JsonMapper.fromJson>(legacy) } + .onSuccess { error("legacy discriminator unexpectedly decoded directly: $it") } + + // ...but the migration (rewrite FQN -> short name) recovers the exact same config. + val migrated = + legacy + .replace("com.vitorpamplona.amethyst.ui.navigation.bottombars.BottomBarEntry.BuiltIn", "builtIn") + .replace("com.vitorpamplona.amethyst.ui.navigation.bottombars.BottomBarEntry.Favorite", "favorite") + assertEquals(sample, JsonMapper.fromJson>(migrated)) + } +}