refactor(emoji): review cleanup — bugs, naming, and UX polish

Critical: - EmojiPackScreen delete dialog: swap plus icon for Delete - EmojiGrid: switch emoji AsyncImage from Crop to Fit so non-square artwork and transparent backgrounds render correctly - AddEmojiDialog: wrap Address.parse in runCatching; a malformed user-entered address no longer crashes the app - BrowseEmojiSetsSubAssembler: use the query scope, not the account scope, for the feed-freshness collector so the subscription lives only as long as the query High: - Drop descriptive kdoc from AddEmojiDialog and EmojiPackViewModel - EmojiPackCard takes an optional author; Browse feed always shows it, MyEmojiList shows it only for foreign packs - EmojiPackScreen: helper line 'Long-press to remove' above the grid so the interaction is discoverable - AddEmojiDialog: private toggle is now a Switch (correct control for a boolean form value) instead of a FilterChip Medium: - EmojiPackState: distinct, accurate error messages on the add/ remove-to-selection paths (was copy-paste) - OwnedEmojiPacksState: import NameTag/TitleTag rather than using fully qualified names inline
2026-04-21 12:42:48 +00:00
parent 876ca197de
commit 3488bbfd94
10 changed files with 65 additions and 39 deletions
@@ -43,6 +43,8 @@ import com.vitorpamplona.quartz.nip51Lists.encryption.PrivateTagsInContent
 import com.vitorpamplona.quartz.nip51Lists.remove
 import com.vitorpamplona.quartz.nip51Lists.tags.DescriptionTag
 import com.vitorpamplona.quartz.nip51Lists.tags.ImageTag
+import com.vitorpamplona.quartz.nip51Lists.tags.NameTag
+import com.vitorpamplona.quartz.nip51Lists.tags.TitleTag
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -179,8 +181,8 @@ class OwnedEmojiPacksState(

        val template =
            packEvent.update<EmojiPackEvent> {
-                remove(com.vitorpamplona.quartz.nip51Lists.tags.NameTag.TAG_NAME)
-                remove(com.vitorpamplona.quartz.nip51Lists.tags.TitleTag.TAG_NAME)
+                remove(NameTag.TAG_NAME)
+                remove(TitleTag.TAG_NAME)
                remove(DescriptionTag.TAG_NAME)
                remove(ImageTag.TAG_NAME)
                title(newTitle)
@@ -184,11 +184,13 @@ private fun BrowsedEmojiPackCard(
            event.taggedEmojis().map { it.url }
        }
    val coverImage = remember(event) { event.image() }
+    val author = remember(note) { note.author?.toBestDisplayName() }

    EmojiPackCard(
        title = title,
        emojiUrls = emojiUrls,
        coverImage = coverImage,
+        author = author,
        onClick = onClick,
        modifier = modifier,
    )
@@ -78,7 +78,7 @@ class BrowseEmojiSetsSubAssembler(
                        invalidateFilters()
                    }
                },
-                key.account.scope.launch(Dispatchers.IO) {
+                key.scope.launch(Dispatchers.IO) {
                    key.feedStates.browseEmojiSetsFeed.lastNoteCreatedAtWhenFullyLoaded.sample(5000).collectLatest {
                        invalidateFilters()
                    }
@@ -58,6 +58,7 @@ fun EmojiPackCard(
    title: String,
    emojiUrls: List<String>,
    coverImage: String? = null,
+    author: String? = null,
    onClick: () -> Unit,
    modifier: Modifier = Modifier,
 ) {
@@ -81,6 +82,16 @@ fun EmojiPackCard(
                    maxLines = 1,
                    overflow = TextOverflow.Ellipsis,
                )
+                if (!author.isNullOrBlank()) {
+                    Spacer(Modifier.height(2.dp))
+                    Text(
+                        text = stringRes(R.string.my_emoji_list_by_author, author),
+                        style = MaterialTheme.typography.labelSmall,
+                        color = MaterialTheme.colorScheme.onSurfaceVariant,
+                        maxLines = 1,
+                        overflow = TextOverflow.Ellipsis,
+                    )
+                }
                Spacer(Modifier.height(2.dp))
                Text(
                    text = stringRes(R.string.emoji_pack_count, emojiCount),
@@ -22,18 +22,20 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.emojipacks.display

 import androidx.compose.foundation.layout.Arrangement
 import androidx.compose.foundation.layout.Column
+import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
+import androidx.compose.foundation.layout.width
 import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.filled.Lock
 import androidx.compose.material.icons.filled.LockOpen
 import androidx.compose.material3.AlertDialog
 import androidx.compose.material3.Button
-import androidx.compose.material3.FilterChip
 import androidx.compose.material3.Icon
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.OutlinedTextField
+import androidx.compose.material3.Switch
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.derivedStateOf
@@ -41,6 +43,7 @@ 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
 import androidx.compose.ui.platform.LocalContext
 import androidx.compose.ui.unit.dp
@@ -52,16 +55,6 @@ import com.vitorpamplona.amethyst.ui.theme.DoubleVertSpacer
 import com.vitorpamplona.quartz.nip01Core.core.Address
 import com.vitorpamplona.quartz.nip30CustomEmoji.EmojiUrlTag

-/**
- * Dialog for adding a custom emoji to an owned emoji pack (NIP-30 kind 30030).
- *
- * The [onConfirm] callback receives the new [EmojiUrlTag] alongside an `isPrivate`
- * flag: when `true`, the caller stores the entry in the event's encrypted
- * `.content` (NIP-51 private tags) rather than as a public tag. Private emojis
- * are surfaced to the pack owner end-to-end (autocomplete + reaction menu) via
- * asynchronous decryption — see
- * [com.vitorpamplona.amethyst.commons.model.nip30CustomEmojis.EmojiPackState.mergePackWithPrivate].
- */
@Composable
 fun AddEmojiDialog(
    viewModel: EmojiPackViewModel,
@@ -141,17 +134,25 @@ fun AddEmojiDialog(
                    label = { Text(stringRes(R.string.emoji_pack_address_label)) },
                )
                Spacer(DoubleVertSpacer)
-                FilterChip(
-                    selected = isPrivate,
-                    onClick = { isPrivate = !isPrivate },
-                    label = { Text(stringRes(R.string.emoji_private_toggle)) },
-                    leadingIcon = {
-                        Icon(
-                            imageVector = if (isPrivate) Icons.Default.Lock else Icons.Default.LockOpen,
-                            contentDescription = null,
-                        )
-                    },
-                )
+                Row(
+                    modifier = Modifier.fillMaxWidth(),
+                    verticalAlignment = Alignment.CenterVertically,
+                ) {
+                    Icon(
+                        imageVector = if (isPrivate) Icons.Default.Lock else Icons.Default.LockOpen,
+                        contentDescription = null,
+                        tint = MaterialTheme.colorScheme.onSurfaceVariant,
+                    )
+                    Spacer(Modifier.width(12.dp))
+                    Text(
+                        text = stringRes(R.string.emoji_private_toggle),
+                        modifier = Modifier.weight(1f),
+                    )
+                    Switch(
+                        checked = isPrivate,
+                        onCheckedChange = { isPrivate = it },
+                    )
+                }
                Spacer(DoubleVertSpacer)
                Text(
                    text =
@@ -171,7 +172,10 @@ fun AddEmojiDialog(
            Button(
                enabled = canConfirm,
                onClick = {
-                    val parsedAddress = packAddressText.trim().takeIf { it.isNotEmpty() }?.let { Address.parse(it) }
+                    val parsedAddress =
+                        packAddressText.trim().takeIf { it.isNotEmpty() }?.let {
+                            runCatching { Address.parse(it) }.getOrNull()
+                        }
                    onConfirm(
                        EmojiUrlTag(code = shortcode, url = url.trim(), emojiSet = parsedAddress),
                        isPrivate,
@@ -37,6 +37,7 @@ import androidx.compose.foundation.shape.CircleShape
 import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.filled.Lock
 import androidx.compose.material.icons.outlined.Add
+import androidx.compose.material.icons.outlined.Delete
 import androidx.compose.material3.ExtendedFloatingActionButton
 import androidx.compose.material3.Icon
 import androidx.compose.material3.IconButton
@@ -158,6 +159,14 @@ private fun EmojiPackScreenView(
                    ).consumeWindowInsets(padding),
        ) {
            pack?.let { currentPack ->
+                if (currentPack.publicEmojis.isNotEmpty() || currentPack.privateEmojis.isNotEmpty()) {
+                    Text(
+                        text = stringRes(R.string.emoji_long_press_hint),
+                        style = MaterialTheme.typography.labelSmall,
+                        color = MaterialTheme.colorScheme.onSurfaceVariant,
+                        modifier = Modifier.padding(horizontal = 16.dp, vertical = 8.dp),
+                    )
+                }
                EmojiGrid(
                    pack = currentPack,
                    onLongPress = { emoji, isPrivate -> pendingDelete = EmojiDeleteTarget(emoji, isPrivate) },
@@ -187,7 +196,7 @@ private fun EmojiPackScreenView(
        ) {
            M3ActionSection {
                M3ActionRow(
-                    icon = Icons.Outlined.Add,
+                    icon = Icons.Outlined.Delete,
                    text = stringRes(R.string.quick_action_delete),
                    isDestructive = true,
                ) {
@@ -259,7 +268,7 @@ private fun EmojiCell(
            model = emoji.url,
            contentDescription = if (isPrivate) "${emoji.code} ($privateLabel)" else emoji.code,
            modifier = Size35Modifier,
-            contentScale = ContentScale.Crop,
+            contentScale = ContentScale.Fit,
        )
        if (isPrivate) {
            Box(
@@ -77,14 +77,6 @@ class EmojiPackViewModel(
        account.deleteOwnedEmojiPack(packIdentifier)
    }

-    /**
-     * Uploads an image selected from the gallery to the account's default file
-     * server (NIP-96 or Blossom) and calls [onUploaded] with the resulting URL.
-     *
-     * Mirrors the uploader pattern used by
-     * `BookmarkGroupMetadataViewModel.uploadForPicture` — see that file for the
-     * canonical implementation.
-     */
    fun uploadEmojiImage(
        uri: SelectedMedia,
        context: Context,
@@ -200,12 +200,17 @@ private fun SelectedEmojiPackCard(
        remember(packEvent) {
            packEvent?.taggedEmojis()?.map { it.url }.orEmpty()
        }
+    val author =
+        remember(packNote) {
+            if (accountViewModel.isLoggedUser(packNote.author)) null else packNote.author?.toBestDisplayName()
+        }

    Box(modifier = modifier.fillMaxWidth()) {
        EmojiPackCard(
            title = title,
            emojiUrls = emojiUrls,
            coverImage = image,
+            author = author,
            onClick = onOpen,
        )
        // Remove button is on top-start so it doesn't clash with the cover badge (top-end).
@@ -2398,6 +2398,7 @@
    <string name="add_emoji_fab">Add emoji</string>
    <string name="emoji_add_dialog_title">Add custom emoji</string>
    <string name="emoji_remove_dialog_title">Remove :%1$s:?</string>
+    <string name="emoji_long_press_hint">Long-press an emoji to remove it</string>
    <string name="emoji_pack_is_in_list">\"%1$s\" is in your emoji list</string>
    <string name="emoji_pack_is_not_in_list">\"%1$s\" is not in your emoji list</string>
    <string name="emoji_pack_actions_dialog_title">Emoji pack actions</string>
@@ -126,9 +126,9 @@ class EmojiPackState(

    suspend fun addEmojiPack(emojiPack: Note): EmojiPackSelectionEvent {
        val emojiPackEvent = emojiPack.event
-        if (emojiPackEvent !is EmojiPackEvent) throw IllegalArgumentException("Cannot add an emoji pack to this kind of event.")
+        if (emojiPackEvent !is EmojiPackEvent) throw IllegalArgumentException("Note is not an EmojiPackEvent; cannot add to emoji list.")

-        val eventHint = emojiPack.toEventHint<EmojiPackEvent>() ?: throw IllegalArgumentException("Cannot add an emoji pack to this kind of event.")
+        val eventHint = emojiPack.toEventHint<EmojiPackEvent>() ?: throw IllegalArgumentException("Cannot build event hint for this emoji pack.")

        val usersEmojiList = getEmojiPackSelection()
        return if (usersEmojiList == null) {
@@ -141,7 +141,7 @@ class EmojiPackState(
    }

    suspend fun removeEmojiPack(emojiPack: Note): EmojiPackSelectionEvent? {
-        val usersEmojiList = getEmojiPackSelection() ?: throw IllegalArgumentException("Cannot remove an emoji pack to this kind of event.")
+        val usersEmojiList = getEmojiPackSelection() ?: throw IllegalArgumentException("No emoji pack selection exists to remove from.")

        val emojiPackEvent = emojiPack.event
        if (emojiPackEvent !is EmojiPackEvent) return null