From f3ad364aea7d152f6637cbcbdd6f3b3f43366f54 Mon Sep 17 00:00:00 2001 From: davotoula Date: Sat, 25 Apr 2026 21:11:16 +0200 Subject: [PATCH] Code reviews: fix(broadcast): observe tracked-broadcasts toggle reactively fix(settings): preserve prior broadcast-tracker behavior on upgrade refactor(broadcast): rename setting to match what the toggle does --- .../amethyst/model/UiSettings.kt | 2 +- .../amethyst/model/UiSettingsFlow.kt | 12 +- .../model/preferences/UISharedPreferences.kt | 12 +- .../ui/broadcast/DisplayBroadcastProgress.kt | 7 +- .../amethyst/ui/screen/UiSettingsState.kt | 2 +- .../ui/screen/loggedIn/AccountViewModel.kt | 182 +++++++----------- .../nip23LongForm/LongFormPostViewModel.kt | 2 +- .../loggedIn/home/ShortNotePostViewModel.kt | 2 +- .../loggedIn/settings/AppSettingsScreen.kt | 19 +- amethyst/src/main/res/values/strings.xml | 4 +- 10 files changed, 101 insertions(+), 143 deletions(-) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/UiSettings.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/UiSettings.kt index 768cd7917..154bd40fd 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/UiSettings.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/UiSettings.kt @@ -42,7 +42,7 @@ data class UiSettings( val featureSet: FeatureSetType = FeatureSetType.SIMPLIFIED, val gallerySet: ProfileGalleryType = ProfileGalleryType.CLASSIC, val automaticallyProposeAiImprovements: BooleanType = BooleanType.ALWAYS, - val showBroadcaster: BooleanType = BooleanType.ALWAYS, + val useTrackedBroadcasts: BooleanType = BooleanType.ALWAYS, val bottomBarItems: List = DefaultBottomBarItems, ) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/UiSettingsFlow.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/UiSettingsFlow.kt index 9d75408fc..b7bcb17c3 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/UiSettingsFlow.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/UiSettingsFlow.kt @@ -42,7 +42,7 @@ class UiSettingsFlow( val featureSet: MutableStateFlow = MutableStateFlow(FeatureSetType.SIMPLIFIED), val gallerySet: MutableStateFlow = MutableStateFlow(ProfileGalleryType.CLASSIC), val automaticallyProposeAiImprovements: MutableStateFlow = MutableStateFlow(BooleanType.ALWAYS), - val showBroadcaster: MutableStateFlow = MutableStateFlow(BooleanType.ALWAYS), + val useTrackedBroadcasts: MutableStateFlow = MutableStateFlow(BooleanType.ALWAYS), val bottomBarItems: MutableStateFlow> = MutableStateFlow(DefaultBottomBarItems), ) { val listOfFlows: List> = @@ -60,7 +60,7 @@ class UiSettingsFlow( featureSet, gallerySet, automaticallyProposeAiImprovements, - showBroadcaster, + useTrackedBroadcasts, bottomBarItems, ) @@ -102,7 +102,7 @@ class UiSettingsFlow( featureSet.value, gallerySet.value, automaticallyProposeAiImprovements.value, - showBroadcaster.value, + useTrackedBroadcasts.value, bottomBarItems.value, ) @@ -161,8 +161,8 @@ class UiSettingsFlow( automaticallyProposeAiImprovements.tryEmit(torSettings.automaticallyProposeAiImprovements) any = true } - if (showBroadcaster.value != torSettings.showBroadcaster) { - showBroadcaster.tryEmit(torSettings.showBroadcaster) + if (useTrackedBroadcasts.value != torSettings.useTrackedBroadcasts) { + useTrackedBroadcasts.tryEmit(torSettings.useTrackedBroadcasts) any = true } if (bottomBarItems.value != torSettings.bottomBarItems) { @@ -201,7 +201,7 @@ class UiSettingsFlow( MutableStateFlow(uiSettings.featureSet), MutableStateFlow(uiSettings.gallerySet), MutableStateFlow(uiSettings.automaticallyProposeAiImprovements), - MutableStateFlow(uiSettings.showBroadcaster), + MutableStateFlow(uiSettings.useTrackedBroadcasts), MutableStateFlow(uiSettings.bottomBarItems), ) } 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 53075aa9d..28be3d53e 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 @@ -106,7 +106,7 @@ class UiSharedPreferences( val UI_FEATURE_SET = stringPreferencesKey("ui.feature_set") val UI_GALLERY_SET = stringPreferencesKey("ui.gallery_set") val UI_PROPOSE_AI_IMPROVEMENTS = stringPreferencesKey("ui.propose_ai_improvements") - val UI_SHOW_BROADCASTER = stringPreferencesKey("ui.show_broadcaster") + val UI_USE_TRACKED_BROADCASTS = stringPreferencesKey("ui.use_tracked_broadcasts") val UI_BOTTOM_BAR_ITEMS = stringPreferencesKey("ui.bottom_bar_items") suspend fun uiPreferences(context: Context): UiSettings? = @@ -114,6 +114,8 @@ class UiSharedPreferences( // Get the preference flow and take the first value. val preferences = context.sharedPreferencesDataStore.data.first() + val featureSet = preferences[UI_FEATURE_SET]?.let { FeatureSetType.valueOf(it) } ?: FeatureSetType.SIMPLIFIED + UiSettings( theme = preferences[UI_THEME]?.let { ThemeType.valueOf(it) } ?: ThemeType.SYSTEM, preferredLanguage = preferences[UI_LANGUAGE]?.ifBlank { null }, @@ -125,10 +127,12 @@ class UiSharedPreferences( automaticallyShowProfilePictures = preferences[UI_SHOW_PROFILE_PICTURES]?.let { ConnectivityType.valueOf(it) } ?: ConnectivityType.ALWAYS, dontShowPushNotificationSelector = preferences[UI_DONT_SHOW_PUSH_NOTIFICATION_SELECTOR] ?: false, dontAskForNotificationPermissions = preferences[UI_DONT_ASK_FOR_NOTIFICATION_PERMISSIONS] ?: false, - featureSet = preferences[UI_FEATURE_SET]?.let { FeatureSetType.valueOf(it) } ?: FeatureSetType.SIMPLIFIED, + featureSet = featureSet, gallerySet = preferences[UI_GALLERY_SET]?.let { ProfileGalleryType.valueOf(it) } ?: ProfileGalleryType.CLASSIC, automaticallyProposeAiImprovements = preferences[UI_PROPOSE_AI_IMPROVEMENTS]?.let { BooleanType.valueOf(it) } ?: BooleanType.ALWAYS, - showBroadcaster = preferences[UI_SHOW_BROADCASTER]?.let { BooleanType.valueOf(it) } ?: BooleanType.ALWAYS, + useTrackedBroadcasts = + preferences[UI_USE_TRACKED_BROADCASTS]?.let { BooleanType.valueOf(it) } + ?: if (featureSet == FeatureSetType.COMPLETE) BooleanType.ALWAYS else BooleanType.NEVER, bottomBarItems = preferences[UI_BOTTOM_BAR_ITEMS]?.let { decodeBottomBarItems(it) } ?: DefaultBottomBarItems, ) } catch (e: Exception) { @@ -167,7 +171,7 @@ class UiSharedPreferences( preferences[UI_FEATURE_SET] = sharedSettings.featureSet.name preferences[UI_GALLERY_SET] = sharedSettings.gallerySet.name preferences[UI_PROPOSE_AI_IMPROVEMENTS] = sharedSettings.automaticallyProposeAiImprovements.name - preferences[UI_SHOW_BROADCASTER] = sharedSettings.showBroadcaster.name + preferences[UI_USE_TRACKED_BROADCASTS] = sharedSettings.useTrackedBroadcasts.name preferences[UI_BOTTOM_BAR_ITEMS] = sharedSettings.bottomBarItems.joinToString(",") { it.name } } } catch (e: Exception) { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/broadcast/DisplayBroadcastProgress.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/broadcast/DisplayBroadcastProgress.kt index 1783e74b4..58d8ac4b6 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/broadcast/DisplayBroadcastProgress.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/broadcast/DisplayBroadcastProgress.kt @@ -37,6 +37,7 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.unit.dp import androidx.lifecycle.compose.collectAsStateWithLifecycle +import com.vitorpamplona.amethyst.model.BooleanType import com.vitorpamplona.amethyst.service.broadcast.BroadcastEvent import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl @@ -49,12 +50,14 @@ import kotlinx.coroutines.delay * - CompletedBroadcastIndicator: Shows completed broadcast for tap-to-view (auto-dismisses after 10s) * - BroadcastDetailsSheet: Shows detailed relay status on tap * - * Visibility is controlled by the dedicated "Show Broadcaster" UI setting. + * Hidden when the "Tracked broadcasts" UI setting is off. */ @OptIn(ExperimentalMaterial3Api::class) @Composable fun DisplayBroadcastProgress(accountViewModel: AccountViewModel) { - if (!accountViewModel.settings.showBroadcaster()) return + val useTrackedBroadcasts by accountViewModel.settings.uiSettingsFlow.useTrackedBroadcasts + .collectAsStateWithLifecycle() + if (useTrackedBroadcasts != BooleanType.ALWAYS) return val activeBroadcasts by accountViewModel.broadcastTracker.activeBroadcasts.collectAsStateWithLifecycle() diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/UiSettingsState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/UiSettingsState.kt index a39afa8d6..bd28f91bf 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/UiSettingsState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/UiSettingsState.kt @@ -131,7 +131,7 @@ class UiSettingsState( fun isCompleteUIMode() = uiSettingsFlow.featureSet.value == FeatureSetType.COMPLETE - fun showBroadcaster() = uiSettingsFlow.showBroadcaster.value == BooleanType.ALWAYS + fun useTrackedBroadcasts() = uiSettingsFlow.useTrackedBroadcasts.value == BooleanType.ALWAYS fun isImmersiveScrollingActive() = uiSettingsFlow.automaticallyHideNavigationBars.value == BooleanType.ALWAYS diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt index 410658ee1..c944b071f 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/AccountViewModel.kt @@ -413,7 +413,7 @@ class AccountViewModel( if (currentReactions.isNotEmpty()) { account.delete(currentReactions) } else { - if (settings.showBroadcaster() && note.event !is NIP17Group) { + if (settings.useTrackedBroadcasts() && note.event !is NIP17Group) { // Tracked broadcasting with progress feedback account.createReactionEvent(note, reaction)?.let { (event, relays) -> broadcastTracker.trackBroadcast( @@ -841,24 +841,12 @@ class AccountViewModel( } } - fun boost(note: Note) { - if (settings.showBroadcaster()) { - // Tracked broadcasting with progress feedback - launchSigner { - account.createBoostEvent(note)?.let { (event, relays) -> - broadcastTracker.trackBroadcast( - event = event, - relays = relays, - client = account.client, - ) - account.consumeBoostEvent(event) - } - } - } else { - // Fire-and-forget (original behavior) - launchSigner { account.boost(note) } - } - } + fun boost(note: Note) = + launchTrackedOrDirect( + createTracked = { account.createBoostEvent(note) }, + consumeTracked = account::consumeBoostEvent, + direct = { account.boost(note) }, + ) fun removeEmojiPack(emojiPack: Note) = launchSigner { account.removeEmojiPack(emojiPack) } @@ -886,112 +874,51 @@ class AccountViewModel( fun pinnedNotes(user: User): Note = LocalCache.getOrCreateAddressableNote(PinListEvent.createPinAddress(user.pubkeyHex)) - fun addPin(note: Note) { - if (settings.showBroadcaster()) { - launchSigner { - account.createAddPinEvent(note)?.let { (event, relays) -> - broadcastTracker.trackBroadcast( - event = event, - relays = relays, - client = account.client, - ) - account.consumePinEvent(event) - } - } - } else { - launchSigner { account.addPin(note) } - } - } + fun addPin(note: Note) = + launchTrackedOrDirect( + createTracked = { account.createAddPinEvent(note) }, + consumeTracked = account::consumePinEvent, + direct = { account.addPin(note) }, + ) - fun removePin(note: Note) { - if (settings.showBroadcaster()) { - launchSigner { - account.createRemovePinEvent(note)?.let { (event, relays) -> - broadcastTracker.trackBroadcast( - event = event, - relays = relays, - client = account.client, - ) - account.consumePinEvent(event) - } - } - } else { - launchSigner { account.removePin(note) } - } - } + fun removePin(note: Note) = + launchTrackedOrDirect( + createTracked = { account.createRemovePinEvent(note) }, + consumeTracked = account::consumePinEvent, + direct = { account.removePin(note) }, + ) fun removeDeletedPins(deletedNotes: Set) { launchSigner { account.removeDeletedPins(deletedNotes) } } - fun addPrivateBookmark(note: Note) { - if (settings.showBroadcaster()) { - launchSigner { - account.createAddBookmarkEvent(note, true)?.let { (event, relays) -> - broadcastTracker.trackBroadcast( - event = event, - relays = relays, - client = account.client, - ) - account.consumeBookmarkEvent(event) - } - } - } else { - launchSigner { account.addBookmark(note, true) } - } - } + fun addPrivateBookmark(note: Note) = + launchTrackedOrDirect( + createTracked = { account.createAddBookmarkEvent(note, true) }, + consumeTracked = account::consumeBookmarkEvent, + direct = { account.addBookmark(note, true) }, + ) - fun addPublicBookmark(note: Note) { - if (settings.showBroadcaster()) { - launchSigner { - account.createAddBookmarkEvent(note, false)?.let { (event, relays) -> - broadcastTracker.trackBroadcast( - event = event, - relays = relays, - client = account.client, - ) - account.consumeBookmarkEvent(event) - } - } - } else { - launchSigner { account.addBookmark(note, false) } - } - } + fun addPublicBookmark(note: Note) = + launchTrackedOrDirect( + createTracked = { account.createAddBookmarkEvent(note, false) }, + consumeTracked = account::consumeBookmarkEvent, + direct = { account.addBookmark(note, false) }, + ) - fun removePrivateBookmark(note: Note) { - if (settings.showBroadcaster()) { - launchSigner { - account.createRemoveBookmarkEvent(note, true)?.let { (event, relays) -> - broadcastTracker.trackBroadcast( - event = event, - relays = relays, - client = account.client, - ) + fun removePrivateBookmark(note: Note) = + launchTrackedOrDirect( + createTracked = { account.createRemoveBookmarkEvent(note, true) }, + consumeTracked = account::consumeBookmarkEvent, + direct = { account.removeBookmark(note, true) }, + ) - account.consumeBookmarkEvent(event) - } - } - } else { - launchSigner { account.removeBookmark(note, true) } - } - } - - fun removePublicBookmark(note: Note) { - if (settings.showBroadcaster()) { - launchSigner { - account.createRemoveBookmarkEvent(note, false)?.let { (event, relays) -> - broadcastTracker.trackBroadcast( - event = event, - relays = relays, - client = account.client, - ) - account.consumeBookmarkEvent(event) - } - } - } else { - launchSigner { account.removeBookmark(note, false) } - } - } + fun removePublicBookmark(note: Note) = + launchTrackedOrDirect( + createTracked = { account.createRemoveBookmarkEvent(note, false) }, + consumeTracked = account::consumeBookmarkEvent, + direct = { account.removeBookmark(note, false) }, + ) fun removeDeletedBookmarks( deletedEventIds: Set, @@ -1035,6 +962,29 @@ class AccountViewModel( account.decryptContent(note)?.let { onReady(it) } } + /** + * Runs an action that has both a tracked and a direct broadcast variant, + * picking the path the user selected via the "Tracked broadcasts" setting. + */ + inline fun launchTrackedOrDirect( + crossinline createTracked: suspend () -> Pair>?, + crossinline consumeTracked: (Event) -> Unit, + crossinline direct: suspend () -> Unit, + ) = launchSigner { + if (settings.useTrackedBroadcasts()) { + createTracked()?.let { (event, relays) -> + broadcastTracker.trackBroadcast( + event = event, + relays = relays, + client = account.client, + ) + consumeTracked(event) + } + } else { + direct() + } + } + inline fun launchSigner(crossinline action: suspend () -> Unit) { viewModelScope.launch(Dispatchers.IO) { try { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip23LongForm/LongFormPostViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip23LongForm/LongFormPostViewModel.kt index 6b88f6410..972ed92cc 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip23LongForm/LongFormPostViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/discover/nip23LongForm/LongFormPostViewModel.kt @@ -326,7 +326,7 @@ class LongFormPostViewModel : val version = draftTag.current cancel() - if (accountViewModel.settings.showBroadcaster()) { + if (accountViewModel.settings.useTrackedBroadcasts()) { val (event, relays, extras) = accountViewModel.account.createPostEvent(template, emptyList()) accountViewModel.viewModelScope.launch(Dispatchers.IO) { accountViewModel.broadcastTracker.trackBroadcast( diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt index 41aaf3205..e92b1ac83 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/home/ShortNotePostViewModel.kt @@ -833,7 +833,7 @@ open class ShortNotePostViewModel : if (anonymous) { accountViewModel.account.signAnonymouslyAndBroadcast(template, extraNotesToBroadcast) - } else if (accountViewModel.settings.showBroadcaster()) { + } else if (accountViewModel.settings.useTrackedBroadcasts()) { // Tracked broadcasting with progress feedback (non-blocking) val (event, relays, extras) = accountViewModel.account.createPostEvent(template, extraNotesToBroadcast) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AppSettingsScreen.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AppSettingsScreen.kt index 1871b5bef..b4e4468ea 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AppSettingsScreen.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/settings/AppSettingsScreen.kt @@ -54,6 +54,7 @@ import androidx.compose.ui.unit.dp import androidx.core.os.LocaleListCompat import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.vitorpamplona.amethyst.R +import com.vitorpamplona.amethyst.model.BooleanType import com.vitorpamplona.amethyst.model.ConnectivityType import com.vitorpamplona.amethyst.model.FeatureSetType import com.vitorpamplona.amethyst.model.ProfileGalleryType @@ -133,7 +134,7 @@ fun SettingsScreen( FeatureSetChoice(sharedPrefs) GalleryChoice(sharedPrefs) AiWritingHelpChoice(sharedPrefs) - ShowBroadcasterChoice(sharedPrefs) + TrackedBroadcastsChoice(sharedPrefs) PushNotificationSettingsRow(sharedPrefs) if (accountViewModel != null) { AlwaysOnNotificationServiceChoice(accountViewModel) @@ -423,22 +424,22 @@ fun AiWritingHelpChoice(sharedPrefs: UiSettingsFlow) { } @Composable -fun ShowBroadcasterChoice(sharedPrefs: UiSettingsFlow) { - val showBroadcasterIndex by sharedPrefs.showBroadcaster.collectAsState() +fun TrackedBroadcastsChoice(sharedPrefs: UiSettingsFlow) { + val useTrackedBroadcastsIndex by sharedPrefs.useTrackedBroadcasts.collectAsState() val booleanItems = persistentListOf( - TitleExplainer(stringRes(ConnectivityType.ALWAYS.resourceId)), - TitleExplainer(stringRes(ConnectivityType.NEVER.resourceId)), + TitleExplainer(stringRes(BooleanType.ALWAYS.reourceId)), + TitleExplainer(stringRes(BooleanType.NEVER.reourceId)), ) SettingsRow( - R.string.show_broadcaster_setting_title, - R.string.show_broadcaster_setting_description, + R.string.tracked_broadcasts_setting_title, + R.string.tracked_broadcasts_setting_description, booleanItems, - showBroadcasterIndex.screenCode, + useTrackedBroadcastsIndex.screenCode, ) { - sharedPrefs.showBroadcaster.tryEmit(parseBooleanType(it)) + sharedPrefs.useTrackedBroadcasts.tryEmit(parseBooleanType(it)) } } diff --git a/amethyst/src/main/res/values/strings.xml b/amethyst/src/main/res/values/strings.xml index efd30c697..50dbf4480 100644 --- a/amethyst/src/main/res/values/strings.xml +++ b/amethyst/src/main/res/values/strings.xml @@ -2469,8 +2469,8 @@ AI Writing Help Propose text improvements Uses an on-device AI model to propose text corrections and tone changes. - Show broadcast tracker - Show progress and per-relay status while events are being broadcast. + Tracked broadcasts + Use the tracked broadcaster when sending events. Shows live progress and per-relay status while broadcasting. Use This Dismiss Correct