mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-11 16:57:39 +00:00
Code review:
- fold the pickers onto shared row/expand-state UI
This commit is contained in:
+7
@@ -463,6 +463,7 @@ val DefaultBottomBarEntries: List<BottomBarEntry> = DefaultBottomBarItems.map {
|
||||
*/
|
||||
data class NavBarCategory(
|
||||
val titleRes: Int,
|
||||
val icon: MaterialSymbol,
|
||||
val items: List<NavBarItem>,
|
||||
)
|
||||
|
||||
@@ -475,6 +476,7 @@ val BottomBarCategories: List<NavBarCategory> =
|
||||
listOf(
|
||||
NavBarCategory(
|
||||
R.string.bottom_bar_category_main,
|
||||
MaterialSymbols.Home,
|
||||
listOf(
|
||||
NavBarItem.HOME,
|
||||
NavBarItem.MESSAGES,
|
||||
@@ -485,6 +487,7 @@ val BottomBarCategories: List<NavBarCategory> =
|
||||
),
|
||||
NavBarCategory(
|
||||
R.string.bottom_bar_category_chats,
|
||||
MaterialSymbols.Group,
|
||||
listOf(
|
||||
NavBarItem.PUBLIC_CHATS,
|
||||
NavBarItem.RELAY_GROUPS,
|
||||
@@ -494,6 +497,7 @@ val BottomBarCategories: List<NavBarCategory> =
|
||||
),
|
||||
NavBarCategory(
|
||||
R.string.bottom_bar_category_you,
|
||||
MaterialSymbols.AccountCircle,
|
||||
listOf(
|
||||
NavBarItem.PROFILE,
|
||||
NavBarItem.MY_LISTS,
|
||||
@@ -511,6 +515,7 @@ val BottomBarCategories: List<NavBarCategory> =
|
||||
),
|
||||
NavBarCategory(
|
||||
R.string.bottom_bar_category_feeds,
|
||||
MaterialSymbols.Subscriptions,
|
||||
listOf(
|
||||
NavBarItem.ARTICLES,
|
||||
NavBarItem.LONGS,
|
||||
@@ -537,6 +542,7 @@ val BottomBarCategories: List<NavBarCategory> =
|
||||
),
|
||||
NavBarCategory(
|
||||
R.string.bottom_bar_category_apps,
|
||||
MaterialSymbols.Apps,
|
||||
listOf(
|
||||
NavBarItem.BROWSER,
|
||||
NavBarItem.FAVORITE_APPS,
|
||||
@@ -547,6 +553,7 @@ val BottomBarCategories: List<NavBarCategory> =
|
||||
),
|
||||
NavBarCategory(
|
||||
R.string.bottom_bar_category_other,
|
||||
MaterialSymbols.Settings,
|
||||
listOf(
|
||||
NavBarItem.SETTINGS,
|
||||
),
|
||||
|
||||
+1
-2
@@ -648,8 +648,7 @@ fun CatalogSection(
|
||||
val onBackground = MaterialTheme.colorScheme.onBackground
|
||||
|
||||
val visible = remember(section, hidden) { DrawerItemVisibility.visibleItems(section, hidden) }
|
||||
val hasFixedRows = section.id == DrawerSectionId.CREATE || section.id == DrawerSectionId.SYSTEM
|
||||
if (visible.isEmpty() && !hasFixedRows) return
|
||||
if (visible.isEmpty() && !section.hasFixedRows) return
|
||||
|
||||
CollapsibleSection(title = section.titleRes) {
|
||||
when (section.id) {
|
||||
|
||||
+8
-4
@@ -47,6 +47,12 @@ data class DrawerSection(
|
||||
val titleRes: Int,
|
||||
val icon: MaterialSymbol,
|
||||
val items: List<NavBarItem>,
|
||||
/**
|
||||
* True for a section that renders rows of its own on top of its catalog items (see [CatalogSection]).
|
||||
* Such a section stays in the drawer even with every catalog row switched off, and — since a fixed
|
||||
* row is not a catalog destination — it never appears in the Side Menu settings screen's counts.
|
||||
*/
|
||||
val hasFixedRows: Boolean = false,
|
||||
)
|
||||
|
||||
/**
|
||||
@@ -133,12 +139,10 @@ val DrawerSections: List<DrawerSection> =
|
||||
DrawerSection(DrawerSectionId.YOU, R.string.drawer_section_you, MaterialSymbols.AccountCircle, DrawerYouItems),
|
||||
DrawerSection(DrawerSectionId.NAVIGATE, R.string.drawer_section_navigate, MaterialSymbols.Home, DrawerNavigateItems),
|
||||
DrawerSection(DrawerSectionId.FEEDS, R.string.drawer_section_feeds, MaterialSymbols.Subscriptions, DrawerFeedsItems),
|
||||
DrawerSection(DrawerSectionId.CREATE, R.string.drawer_section_create, MaterialSymbols.Edit, emptyList()),
|
||||
DrawerSection(DrawerSectionId.SYSTEM, R.string.drawer_section_system, MaterialSymbols.Settings, listOf(NavBarItem.SETTINGS)),
|
||||
DrawerSection(DrawerSectionId.CREATE, R.string.drawer_section_create, MaterialSymbols.Edit, emptyList(), hasFixedRows = true),
|
||||
DrawerSection(DrawerSectionId.SYSTEM, R.string.drawer_section_system, MaterialSymbols.Settings, listOf(NavBarItem.SETTINGS), hasFixedRows = true),
|
||||
)
|
||||
|
||||
fun drawerSection(id: DrawerSectionId): DrawerSection = DrawerSections.first { it.id == id }
|
||||
|
||||
/**
|
||||
* Catalog ids deliberately absent from every [DrawerSections] list, with the reason. Only Favorite
|
||||
* Apps qualifies: [DrawerFeedsItems] gates it on API 30+ (its inline tabs need SurfaceControlViewHost),
|
||||
|
||||
+14
-36
@@ -54,7 +54,6 @@ import androidx.compose.runtime.mutableIntStateOf
|
||||
import androidx.compose.runtime.mutableStateMapOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.runtime.snapshots.SnapshotStateMap
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.draw.clip
|
||||
@@ -91,6 +90,7 @@ import com.vitorpamplona.amethyst.ui.navigation.topbars.TopBarWithBackButton
|
||||
import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel
|
||||
import com.vitorpamplona.amethyst.ui.screen.loggedIn.mockAccountViewModel
|
||||
import com.vitorpamplona.amethyst.ui.stringRes
|
||||
import com.vitorpamplona.amethyst.ui.theme.Size22Modifier
|
||||
import com.vitorpamplona.amethyst.ui.theme.ThemeComparisonRow
|
||||
import com.vitorpamplona.quartz.concord.cord02Community.ConcordCommunityListEntry
|
||||
import com.vitorpamplona.quartz.nip51Lists.simpleGroupList.GroupTag
|
||||
@@ -158,8 +158,8 @@ fun BottomBarSettingsContent(accountViewModel: AccountViewModel) {
|
||||
val pinned = state.pinned
|
||||
val pinnedKeys = remember(pinned) { state.pinnedKeys() }
|
||||
|
||||
val expandedCategories = remember { mutableStateMapOf<Int, Boolean>() }
|
||||
val expandedItems = remember { mutableStateMapOf<NavBarItem, Boolean>() }
|
||||
val expandedCategories = rememberExpandedKeys<Int>()
|
||||
val expandedItems = rememberExpandedKeys<NavBarItem>()
|
||||
|
||||
Column(
|
||||
modifier =
|
||||
@@ -183,8 +183,8 @@ fun BottomBarSettingsContent(accountViewModel: AccountViewModel) {
|
||||
CategoryCard(
|
||||
category = category,
|
||||
pinnedKeys = pinnedKeys,
|
||||
expanded = expandedCategories[category.titleRes] ?: false,
|
||||
onToggleExpand = { expandedCategories[category.titleRes] = !(expandedCategories[category.titleRes] ?: false) },
|
||||
expanded = expandedCategories.isExpanded(category.titleRes),
|
||||
onToggleExpand = { expandedCategories.toggle(category.titleRes) },
|
||||
expandedItems = expandedItems,
|
||||
accountViewModel = accountViewModel,
|
||||
onTogglePin = state::togglePin,
|
||||
@@ -437,12 +437,12 @@ private fun CategoryCard(
|
||||
pinnedKeys: Set<String>,
|
||||
expanded: Boolean,
|
||||
onToggleExpand: () -> Unit,
|
||||
expandedItems: SnapshotStateMap<NavBarItem, Boolean>,
|
||||
expandedItems: ExpandedKeys<NavBarItem>,
|
||||
accountViewModel: AccountViewModel,
|
||||
onTogglePin: (BottomBarEntry) -> Unit,
|
||||
) {
|
||||
CatalogCard(
|
||||
icon = categoryIcon(category.titleRes),
|
||||
icon = category.icon,
|
||||
title = stringRes(category.titleRes),
|
||||
expanded = expanded,
|
||||
onToggleExpand = onToggleExpand,
|
||||
@@ -455,9 +455,9 @@ private fun CategoryCard(
|
||||
icon = def.icon,
|
||||
label = stringRes(def.labelRes),
|
||||
pinned = entry.stableKey in pinnedKeys,
|
||||
expanded = expandedItems[item] ?: false,
|
||||
expanded = expandedItems.isExpanded(item),
|
||||
onTogglePin = { onTogglePin(entry) },
|
||||
onToggleExpand = { expandedItems[item] = !(expandedItems[item] ?: false) },
|
||||
onToggleExpand = { expandedItems.toggle(item) },
|
||||
) {
|
||||
PickerChildren(item, pinnedKeys, accountViewModel, onTogglePin)
|
||||
}
|
||||
@@ -717,27 +717,15 @@ private fun ExpandableAvailableRow(
|
||||
onToggleExpand: () -> Unit,
|
||||
children: @Composable () -> Unit,
|
||||
) {
|
||||
Row(
|
||||
modifier =
|
||||
Modifier
|
||||
.fillMaxWidth()
|
||||
.clickable(onClick = onToggleExpand)
|
||||
.padding(start = 13.dp, end = 13.dp, top = 7.dp, bottom = 7.dp),
|
||||
verticalAlignment = Alignment.CenterVertically,
|
||||
horizontalArrangement = Arrangement.spacedBy(12.dp),
|
||||
CatalogRow(
|
||||
leading = { LeadingGlyph(icon) },
|
||||
label = label,
|
||||
onToggle = onToggleExpand,
|
||||
) {
|
||||
LeadingGlyph(icon)
|
||||
Text(
|
||||
text = label,
|
||||
style = MaterialTheme.typography.bodyLarge,
|
||||
maxLines = 1,
|
||||
overflow = TextOverflow.Ellipsis,
|
||||
modifier = Modifier.weight(1f),
|
||||
)
|
||||
Icon(
|
||||
symbol = if (expanded) MaterialSymbols.ExpandLess else MaterialSymbols.ExpandMore,
|
||||
contentDescription = stringRes(R.string.bottom_bar_settings_expand),
|
||||
modifier = Modifier.size(22.dp),
|
||||
modifier = Size22Modifier,
|
||||
tint = MaterialTheme.colorScheme.onSurfaceVariant,
|
||||
)
|
||||
AddPill(added = pinned, onClick = onTogglePin)
|
||||
@@ -777,16 +765,6 @@ private fun FavoriteLeading(app: FavoriteApp) {
|
||||
}
|
||||
}
|
||||
|
||||
private fun categoryIcon(titleRes: Int): MaterialSymbol =
|
||||
when (titleRes) {
|
||||
R.string.bottom_bar_category_main -> MaterialSymbols.Home
|
||||
R.string.bottom_bar_category_chats -> MaterialSymbols.Group
|
||||
R.string.bottom_bar_category_you -> MaterialSymbols.AccountCircle
|
||||
R.string.bottom_bar_category_feeds -> MaterialSymbols.Subscriptions
|
||||
R.string.bottom_bar_category_apps -> MaterialSymbols.Apps
|
||||
else -> MaterialSymbols.Settings
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------------------------------------
|
||||
// Leading/label resolution for a pinned entry (built-in glyph, favorite icon, or group avatar).
|
||||
// Computed once so a group's channel is subscribed at most once per row.
|
||||
|
||||
+14
-21
@@ -38,7 +38,6 @@ import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.runtime.derivedStateOf
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateMapOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.text.font.FontWeight
|
||||
@@ -113,11 +112,9 @@ fun DrawerSettingsContent(accountViewModel: AccountViewModel) {
|
||||
|
||||
// Sections start collapsed: expanded, they are ~50 rows of scrolling. The header's hidden
|
||||
// counter is what tells the user which one to open.
|
||||
val expandedSections = remember { mutableStateMapOf<DrawerSectionId, Boolean>() }
|
||||
val expandedSections = rememberExpandedKeys<DrawerSectionId>()
|
||||
|
||||
// derivedStateOf so a toggle that leaves this total unchanged doesn't re-run the whole screen
|
||||
// body — every SectionCard below reads the same coarse `hidden` state.
|
||||
val totalHidden by remember { derivedStateOf { state.totalHidden() } }
|
||||
val totalHidden = state.totalHidden()
|
||||
|
||||
Column(
|
||||
modifier =
|
||||
@@ -142,8 +139,8 @@ fun DrawerSettingsContent(accountViewModel: AccountViewModel) {
|
||||
SectionCard(
|
||||
section = section,
|
||||
state = state,
|
||||
expanded = expandedSections[section.id] ?: false,
|
||||
onToggleExpand = { expandedSections[section.id] = !(expandedSections[section.id] ?: false) },
|
||||
expanded = expandedSections.isExpanded(section.id),
|
||||
onToggleExpand = { expandedSections.toggle(section.id) },
|
||||
)
|
||||
}
|
||||
|
||||
@@ -241,22 +238,18 @@ private fun VisibilityPill(
|
||||
mandatory: Boolean,
|
||||
onClick: () -> Unit,
|
||||
) {
|
||||
// One branch decides both halves of the pill, so a label can't drift away from its glyph.
|
||||
val (labelRes, icon) =
|
||||
when {
|
||||
mandatory -> R.string.drawer_settings_always_on to MaterialSymbols.Lock
|
||||
visible -> R.string.drawer_settings_visible to MaterialSymbols.Visibility
|
||||
else -> R.string.drawer_settings_hidden to MaterialSymbols.VisibilityOff
|
||||
}
|
||||
|
||||
TogglePill(
|
||||
on = visible,
|
||||
label =
|
||||
stringRes(
|
||||
when {
|
||||
mandatory -> R.string.drawer_settings_always_on
|
||||
visible -> R.string.drawer_settings_visible
|
||||
else -> R.string.drawer_settings_hidden
|
||||
},
|
||||
),
|
||||
icon =
|
||||
when {
|
||||
mandatory -> MaterialSymbols.Lock
|
||||
visible -> MaterialSymbols.Visibility
|
||||
else -> MaterialSymbols.VisibilityOff
|
||||
},
|
||||
label = stringRes(labelRes),
|
||||
icon = icon,
|
||||
enabled = !mandatory,
|
||||
onClick = onClick,
|
||||
)
|
||||
|
||||
+23
-1
@@ -42,6 +42,9 @@ import androidx.compose.material3.Surface
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.material3.TextButton
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.Stable
|
||||
import androidx.compose.runtime.mutableStateMapOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.draw.clip
|
||||
@@ -90,13 +93,32 @@ val SectionCollapse = shrinkVertically(shrinkTowards = Alignment.Top) + fadeOut(
|
||||
* category (a favorite, or a relay/community "server" row), 2 = a room nested under its server (a
|
||||
* NIP-29 group under its relay, or a Concord channel under its community).
|
||||
*/
|
||||
fun indentPadding(level: Int) =
|
||||
private fun indentPadding(level: Int) =
|
||||
when (level) {
|
||||
0 -> Size13dp
|
||||
1 -> Size24dp
|
||||
else -> Size40dp
|
||||
}
|
||||
|
||||
/**
|
||||
* Which collapsible rows of a picker are currently open, keyed by whatever identifies a row (a
|
||||
* section id, a string-resource id, a catalog item). Absent means collapsed, so the initial state
|
||||
* costs nothing and no list has to be seeded.
|
||||
*/
|
||||
@Stable
|
||||
class ExpandedKeys<K> {
|
||||
private val open = mutableStateMapOf<K, Boolean>()
|
||||
|
||||
fun isExpanded(key: K): Boolean = open[key] == true
|
||||
|
||||
fun toggle(key: K) {
|
||||
open[key] = !isExpanded(key)
|
||||
}
|
||||
}
|
||||
|
||||
@Composable
|
||||
fun <K> rememberExpandedKeys(): ExpandedKeys<K> = remember { ExpandedKeys() }
|
||||
|
||||
@Composable
|
||||
fun PickerSectionHeader(title: String) {
|
||||
Text(
|
||||
|
||||
+3
-3
@@ -23,7 +23,7 @@ package com.vitorpamplona.amethyst.navigation
|
||||
import com.vitorpamplona.amethyst.ui.navigation.bottombars.NavBarItem
|
||||
import com.vitorpamplona.amethyst.ui.navigation.drawer.DrawerItemVisibility
|
||||
import com.vitorpamplona.amethyst.ui.navigation.drawer.DrawerSectionId
|
||||
import com.vitorpamplona.amethyst.ui.navigation.drawer.drawerSection
|
||||
import com.vitorpamplona.amethyst.ui.navigation.drawer.DrawerSections
|
||||
import com.vitorpamplona.amethyst.ui.screen.loggedIn.settings.DrawerSettingsState
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
@@ -38,8 +38,8 @@ import org.junit.Test
|
||||
* rows, which no code path may switch off.
|
||||
*/
|
||||
class DrawerItemVisibilityTest {
|
||||
private val you = drawerSection(DrawerSectionId.YOU)
|
||||
private val system = drawerSection(DrawerSectionId.SYSTEM)
|
||||
private val you = DrawerSections.first { it.id == DrawerSectionId.YOU }
|
||||
private val system = DrawerSections.first { it.id == DrawerSectionId.SYSTEM }
|
||||
|
||||
@Test
|
||||
fun nothingHiddenMeansEverythingVisible() {
|
||||
|
||||
@@ -90,6 +90,22 @@ class DrawerSectionsTest {
|
||||
assertEquals("two sections share a DrawerSectionId", ids.size, ids.toSet().size)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun aSectionWithNoCatalogItemsRendersFixedRowsOrNothingAtAll() {
|
||||
// hasFixedRows is declared on the section but consumed by CatalogSection's `when (section.id)`,
|
||||
// in another file — so the flag and the branch that honours it can drift apart with no compile
|
||||
// error. A section that carries neither is unreachable in both directions at once: the settings
|
||||
// screen skips it on items.isEmpty(), and CatalogSection returns before rendering a heading.
|
||||
val unreachable = DrawerSections.filter { it.items.isEmpty() && !it.hasFixedRows }
|
||||
|
||||
assertEquals(
|
||||
"a drawer section has no catalog items and no fixed rows, so it renders nowhere — " +
|
||||
"give it items, set hasFixedRows and a branch in CatalogSection, or delete it",
|
||||
emptyList<Any>(),
|
||||
unreachable.map { it.id },
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun mandatoryItemsAreActuallyRenderedByASection() {
|
||||
// A mandatory item that no section renders would be unhideable *and* invisible — the worst
|
||||
|
||||
Reference in New Issue
Block a user