Merge pull request #2591 from vitorpamplona/claude/audit-feedfilterspinner-GHhgH

Refactor FeedFilterSpinner to pass FeedDefinition objects instead of indices
2026-04-26 15:16:33 -04:00
parent 49f769f92a e00da8c52d
commit dd0a6731f6
20 changed files with 106 additions and 157 deletions
@@ -35,6 +35,7 @@ import com.vitorpamplona.quartz.nip38UserStatus.StatusEvent
 import com.vitorpamplona.quartz.nip50Search.SearchRelayListEvent
 import com.vitorpamplona.quartz.nip51Lists.geohashList.GeohashListEvent
 import com.vitorpamplona.quartz.nip51Lists.hashtagList.HashtagListEvent
+import com.vitorpamplona.quartz.nip51Lists.interestSet.InterestSetEvent
 import com.vitorpamplona.quartz.nip51Lists.relayLists.BlockedRelayListEvent
 import com.vitorpamplona.quartz.nip51Lists.relayLists.BroadcastRelayListEvent
 import com.vitorpamplona.quartz.nip51Lists.relayLists.IndexerRelayListEvent
@@ -73,6 +74,7 @@ val AccountInfoAndListsFromKeyKinds2 =
        TrustProviderListEvent.KIND,
        PaymentTargetsEvent.KIND,
        RelayFeedsListEvent.KIND,
+        InterestSetEvent.KIND,
    )

 val AmethystMetadataKinds = listOf(AppSpecificDataEvent.KIND)
@@ -102,7 +104,7 @@ fun filterAccountInfoAndListsFromKey(
                Filter(
                    kinds = AccountInfoAndListsFromKeyKinds2,
                    authors = listOf(pubkey),
-                    limit = 20,
+                    limit = 80,
                    since = since,
                ),
        ),
@@ -31,6 +31,7 @@ import com.vitorpamplona.quartz.nip17Dm.settings.ChatMessageRelayListEvent
 import com.vitorpamplona.quartz.nip50Search.SearchRelayListEvent
 import com.vitorpamplona.quartz.nip51Lists.geohashList.GeohashListEvent
 import com.vitorpamplona.quartz.nip51Lists.hashtagList.HashtagListEvent
+import com.vitorpamplona.quartz.nip51Lists.interestSet.InterestSetEvent
 import com.vitorpamplona.quartz.nip51Lists.relayLists.BlockedRelayListEvent
 import com.vitorpamplona.quartz.nip51Lists.relayLists.BroadcastRelayListEvent
 import com.vitorpamplona.quartz.nip51Lists.relayLists.IndexerRelayListEvent
@@ -65,6 +66,7 @@ val BasicAccountInfoKinds2 =
        GeohashListEvent.KIND,
        TrustProviderListEvent.KIND,
        RelayFeedsListEvent.KIND,
+        InterestSetEvent.KIND,
    )

 fun filterBasicAccountInfoFromKeys(
@@ -34,15 +34,14 @@ import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.layout.width
 import androidx.compose.foundation.lazy.LazyColumn
 import androidx.compose.foundation.shape.RoundedCornerShape
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Surface
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
-import androidx.compose.runtime.Immutable
 import androidx.compose.runtime.LaunchedEffect
-import androidx.compose.runtime.derivedStateOf
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
@@ -69,7 +68,6 @@ import com.vitorpamplona.amethyst.Amethyst
 import com.vitorpamplona.amethyst.R
 import com.vitorpamplona.amethyst.commons.icons.symbols.Icon
 import com.vitorpamplona.amethyst.commons.icons.symbols.MaterialSymbols
-import com.vitorpamplona.amethyst.model.AddressableNote
 import com.vitorpamplona.amethyst.model.TopFilter
 import com.vitorpamplona.amethyst.service.location.LocationState
 import com.vitorpamplona.amethyst.service.relayClient.reqCommand.event.observeNote
@@ -92,10 +90,6 @@ import com.vitorpamplona.amethyst.ui.theme.Font14SP
 import com.vitorpamplona.amethyst.ui.theme.Size20Modifier
 import com.vitorpamplona.amethyst.ui.theme.StdHorzSpacer
 import com.vitorpamplona.amethyst.ui.theme.placeholderText
-import com.vitorpamplona.quartz.nip51Lists.followList.FollowListEvent
-import com.vitorpamplona.quartz.nip51Lists.peopleList.PeopleListEvent
-import com.vitorpamplona.quartz.nip72ModCommunities.definition.CommunityDefinitionEvent
-import com.vitorpamplona.quartz.nip89AppHandlers.definition.AppDefinitionEvent
 import kotlinx.collections.immutable.ImmutableList

@OptIn(ExperimentalPermissionsApi::class)
@@ -104,32 +98,27 @@ fun FeedFilterSpinner(
    placeholderCode: TopFilter,
    explainer: String,
    options: ImmutableList<FeedDefinition>,
-    onSelect: (Int) -> Unit,
+    onSelect: (FeedDefinition) -> Unit,
    modifier: Modifier = Modifier,
    accountViewModel: AccountViewModel,
 ) {
    var optionsShowing by remember { mutableStateOf(false) }

    val context = LocalContext.current
-    val selectAnOption =
-        stringRes(
-            id = R.string.select_an_option,
-        )
+    val selectAnOption = stringRes(id = R.string.select_an_option)

-    var selected by
+    val selected =
        remember(placeholderCode, options) {
-            mutableStateOf(
-                options.firstOrNull { it.code.code == placeholderCode.code },
-            )
-        }
-
-    val currentText by
-        remember(placeholderCode, options) {
-            derivedStateOf {
-                selected?.name?.name(context) ?: selectAnOption
+            // Match by both subclass and code string to avoid collisions between
+            // TopFilter variants that derive `code` from the same Address (e.g.
+            // PeopleList vs MuteList).
+            options.firstOrNull {
+                it.code::class == placeholderCode::class && it.code.code == placeholderCode.code
            }
        }

+    val currentText = selected?.name?.name(context) ?: selectAnOption
+
    val accessibilityDescription =
        if (selected != null) {
            stringRes(R.string.feed_filter_selected, currentText)
@@ -282,10 +271,9 @@ fun FeedFilterSpinner(
            title = explainer,
            options = options,
            onDismiss = { optionsShowing = false },
-            onSelect = {
-                selected = options[it]
+            onSelect = { definition ->
                optionsShowing = false
-                onSelect(it)
+                onSelect(definition)
            },
        ) {
            RenderOption(it.name, accountViewModel)
@@ -298,6 +286,7 @@ fun RenderOption(
    option: Name,
    accountViewModel: AccountViewModel,
 ) {
+    val context = LocalContext.current
    when (option) {
        is GeoHashName -> {
            LoadCityName(option.geoHashTag) {
@@ -305,74 +294,35 @@ fun RenderOption(
            }
        }

-        is HashtagName -> {
-            Text(text = option.name(), fontSize = Font14SP, color = MaterialTheme.colorScheme.onSurface)
-        }
-
-        is ResourceName -> {
-            Text(
-                text = stringRes(id = option.resourceId),
-                fontSize = Font14SP,
-                color = MaterialTheme.colorScheme.onSurface,
-            )
-        }
-
+        // Note-backed names: subscribe to the note so the displayed title updates as
+        // the corresponding event arrives from relays. The displayed string itself is
+        // produced by Name.name(), which already has the right precedence rules.
        is PeopleListName -> {
            val noteState by observeNote(option.note, accountViewModel)
-
-            val noteEvent = noteState.note.event
-            val name =
-                when (noteEvent) {
-                    is PeopleListEvent -> {
-                        noteEvent.titleOrName() ?: option.note.dTag()
-                    }
-
-                    is FollowListEvent -> {
-                        noteEvent.title() ?: option.note.dTag()
-                    }
-
-                    else -> {
-                        option.note.dTag()
-                    }
-                }
-
+            val name = remember(noteState) { option.name(context) }
            Text(text = name, fontSize = Font14SP, color = MaterialTheme.colorScheme.onSurface)
        }

        is CommunityName -> {
-            val it by observeNote(option.note, accountViewModel)
-
-            val addressable = it.note as? AddressableNote
-            val definition = addressable?.event as? CommunityDefinitionEvent
-            val label = definition?.name()?.ifBlank { null } ?: addressable?.dTag() ?: ""
-            Text(text = "/n/$label", fontSize = Font14SP, color = MaterialTheme.colorScheme.onSurface)
-        }
-
-        is RelayName -> {
-            Text(
-                text = option.name(),
-                fontSize = Font14SP,
-                color = MaterialTheme.colorScheme.onSurface,
-            )
+            val noteState by observeNote(option.note, accountViewModel)
+            val name = remember(noteState) { option.name(context) }
+            Text(text = name, fontSize = Font14SP, color = MaterialTheme.colorScheme.onSurface)
        }

        is FavoriteAlgoFeedName -> {
            val noteState by observeNote(option.note, accountViewModel)
-            val name =
-                (noteState.note.event as? AppDefinitionEvent)
-                    ?.appMetaData()
-                    ?.name
-                    ?.takeIf { it.isNotBlank() } ?: option.note.dTag()
-            Text(
-                text = name,
-                fontSize = Font14SP,
-                color = MaterialTheme.colorScheme.onSurface,
-            )
+            val name = remember(noteState) { option.name(context) }
+            Text(text = name, fontSize = Font14SP, color = MaterialTheme.colorScheme.onSurface)
        }

-        is InterestSetName -> {
+        // Pure names: no relay subscription needed.
+        is HashtagName,
+        is ResourceName,
+        is RelayName,
+        is InterestSetName,
+        -> {
            Text(
-                text = option.name(),
+                text = option.name(context),
                fontSize = Font14SP,
                color = MaterialTheme.colorScheme.onSurface,
            )
@@ -380,12 +330,6 @@ fun RenderOption(
    }
 }

-@Immutable
-private data class IndexedFeedDefinition(
-    val originalIndex: Int,
-    val item: FeedDefinition,
-)
-
 private enum class FeedGroup(
    @param:androidx.annotation.StringRes val labelRes: Int,
 ) {
@@ -399,48 +343,51 @@ private enum class FeedGroup(
    RELAYS(R.string.feed_group_relays),
 }

-private fun groupFeedDefinitions(options: ImmutableList<FeedDefinition>): Map<FeedGroup, List<IndexedFeedDefinition>> {
-    val indexed = options.mapIndexed { index, item -> IndexedFeedDefinition(index, item) }
-    return indexed.groupBy { entry ->
-        when (entry.item.name) {
-            is HashtagName -> {
-                FeedGroup.HASHTAGS
-            }
+private fun FeedDefinition.group(): FeedGroup =
+    when (name) {
+        is HashtagName -> {
+            FeedGroup.HASHTAGS
+        }

-            is CommunityName -> {
-                FeedGroup.COMMUNITIES
-            }
+        is CommunityName -> {
+            FeedGroup.COMMUNITIES
+        }

-            is PeopleListName -> {
-                FeedGroup.LISTS
-            }
+        is PeopleListName -> {
+            FeedGroup.LISTS
+        }

-            is RelayName -> {
-                FeedGroup.RELAYS
-            }
+        is RelayName -> {
+            FeedGroup.RELAYS
+        }

-            is GeoHashName -> {
-                FeedGroup.LOCATIONS
-            }
+        is GeoHashName -> {
+            FeedGroup.LOCATIONS
+        }

-            is FavoriteAlgoFeedName -> {
-                FeedGroup.DVMS
-            }
+        is FavoriteAlgoFeedName -> {
+            FeedGroup.DVMS
+        }

-            is InterestSetName -> {
-                FeedGroup.INTEREST_SETS
-            }
+        is InterestSetName -> {
+            FeedGroup.INTEREST_SETS
+        }

-            is ResourceName -> {
-                when (entry.item.code) {
-                    is TopFilter.AroundMe -> FeedGroup.LOCATIONS
-                    is TopFilter.Global -> FeedGroup.RELAYS
-                    is TopFilter.AllFavoriteAlgoFeeds -> FeedGroup.DVMS
-                    else -> FeedGroup.FEEDS
-                }
+        is ResourceName -> {
+            when (code) {
+                is TopFilter.AroundMe -> FeedGroup.LOCATIONS
+                is TopFilter.Global -> FeedGroup.RELAYS
+                is TopFilter.AllFavoriteAlgoFeeds -> FeedGroup.DVMS
+                else -> FeedGroup.FEEDS
            }
        }
    }
+
+private fun groupFeedDefinitions(options: ImmutableList<FeedDefinition>): List<Pair<FeedGroup, List<FeedDefinition>>> {
+    val grouped = options.groupBy { it.group() }
+    return FeedGroup.entries.mapNotNull { group ->
+        grouped[group]?.takeIf { it.isNotEmpty() }?.let { group to it }
+    }
 }

@OptIn(ExperimentalLayoutApi::class)
@@ -448,7 +395,7 @@ private fun groupFeedDefinitions(options: ImmutableList<FeedDefinition>): Map<Fe
 private fun GroupedFeedFilterDialog(
    title: String,
    options: ImmutableList<FeedDefinition>,
-    onSelect: (Int) -> Unit,
+    onSelect: (FeedDefinition) -> Unit,
    onDismiss: () -> Unit,
    onRenderItem: @Composable (FeedDefinition) -> Unit,
 ) {
@@ -472,18 +419,15 @@ private fun GroupedFeedFilterDialog(
                    )
                }

-                FeedGroup.entries.forEach { group ->
-                    val items = grouped[group]
-                    if (!items.isNullOrEmpty()) {
-                        item {
-                            GroupSection(
-                                label = stringRes(group.labelRes),
-                                items = items,
-                                isChipLayout = group == FeedGroup.HASHTAGS,
-                                onSelect = onSelect,
-                                onRenderItem = onRenderItem,
-                            )
-                        }
+                grouped.forEach { (group, items) ->
+                    item(key = group) {
+                        GroupSection(
+                            label = stringRes(group.labelRes),
+                            items = items,
+                            isChipLayout = group == FeedGroup.HASHTAGS,
+                            onSelect = onSelect,
+                            onRenderItem = onRenderItem,
+                        )
                    }
                }
            }
@@ -495,11 +439,12 @@ private fun GroupedFeedFilterDialog(
@Composable
 private fun GroupSection(
    label: String,
-    items: List<IndexedFeedDefinition>,
+    items: List<FeedDefinition>,
    isChipLayout: Boolean,
-    onSelect: (Int) -> Unit,
+    onSelect: (FeedDefinition) -> Unit,
    onRenderItem: @Composable (FeedDefinition) -> Unit,
 ) {
+    val context = LocalContext.current
    Surface(
        modifier = Modifier.padding(horizontal = 12.dp, vertical = 4.dp),
        shape = RoundedCornerShape(16.dp),
@@ -523,13 +468,13 @@ private fun GroupSection(
                ) {
                    items.forEach { entry ->
                        Surface(
-                            modifier = Modifier.clickable { onSelect(entry.originalIndex) },
+                            modifier = Modifier.clickable { onSelect(entry) },
                            shape = RoundedCornerShape(18.dp),
                            border = BorderStroke(1.dp, MaterialTheme.colorScheme.outline),
                            color = Color.Transparent,
                        ) {
                            Text(
-                                text = entry.item.name.name(),
+                                text = entry.name.name(context),
                                fontSize = 13.sp,
                                color = MaterialTheme.colorScheme.onSurface,
                                modifier = Modifier.padding(horizontal = 14.dp, vertical = 7.dp),
@@ -545,15 +490,15 @@ private fun GroupSection(
                        modifier =
                            Modifier
                                .fillMaxWidth()
-                                .clickable { onSelect(entry.originalIndex) }
+                                .clickable { onSelect(entry) }
                                .padding(horizontal = 16.dp, vertical = 6.dp),
                    ) {
                        FeedIcon(
-                            item = entry.item,
+                            item = entry,
                            modifier = Size20Modifier,
                        )
-                        Spacer(modifier = Modifier.padding(start = 12.dp))
-                        Column(modifier = Modifier.weight(1f)) { onRenderItem(entry.item) }
+                        Spacer(modifier = Modifier.width(12.dp))
+                        Column(modifier = Modifier.weight(1f)) { onRenderItem(entry) }
                    }
                }
            }
@@ -64,7 +64,7 @@ private fun ArticlesTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun AudioRoomsTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun BadgesTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun CommunitiesTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun TopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun BrowseEmojiSetsTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun FollowPacksTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -69,7 +69,7 @@ private fun TopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun LiveStreamsTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun LongsTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun TopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun PicturesTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun PollsTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun ProductsTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun PublicChatsTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -64,7 +64,7 @@ private fun ShortsTopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }
@@ -65,7 +65,7 @@ private fun TopNavFilterBar(
        placeholderCode = listName,
        explainer = stringRes(R.string.select_list_to_filter),
        options = allLists,
-        onSelect = { onChange(allLists.getOrNull(it) ?: followListsModel.allFollows) },
+        onSelect = onChange,
        accountViewModel = accountViewModel,
    )
 }