From 065fa1b7ddd733f313ab90f9d8dcce1f174721d6 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 21 Sep 2026 15:59:55 -0400 Subject: [PATCH 01/14] feat(reporting): let people report a user, a group, or a message Three entry points, one sheet. A report row sits on the user profile above Block and on the group profile above Leave Chat, and Report joins the message selection bar for anything another participant sent. Each opens ReportSheet, which lists the six shared reasons and, for Something Else, asks for a description before it will submit. The capability rule is that anything a participant sent can be reported and anything the server wrote, or that no longer exists, cannot. Your own messages are excluded: reporting one is not a thing anyone does, and a row that is always present is a row people stop reading. Report carries no time window, unlike edit and delete, because nothing in the contract limits how old a reportable message may be. Report is ordered last in the selection bar, so it is the first to collapse into the overflow when the bar runs out of room. --- .../com/flipcash/app/core/chat/ChatStep.kt | 14 ++ .../flipcash/app/core/chat/ReportSubject.kt | 32 +++++ .../core/src/main/res/values/strings.xml | 15 ++ .../features/messenger/build.gradle.kts | 1 + .../flipcash/app/messenger/ChatFlowScreen.kt | 47 ++++++- .../app/messenger/internal/ChatViewModel.kt | 131 ++++++++++++++---- .../internal/screens/ReportReasonLabels.kt | 23 +++ .../messenger/internal/screens/ReportSheet.kt | 131 ++++++++++++++++++ .../internal/screens/components/ChatTopBar.kt | 24 +++- .../screens/profile/ChatProfileScreen.kt | 12 +- .../screens/profile/GroupProfileMenuItems.kt | 12 ++ .../screens/profile/GroupProfileScreen.kt | 17 ++- .../screens/profile/ProfileMenuItems.kt | 12 ++ .../internal/ChatSelectionBarTest.kt | 68 +++++++++ .../internal/screens/ReportSheetTest.kt | 97 +++++++++++++ .../flipcash/shared/chat/MessageCapability.kt | 25 +++- .../shared/chat/MessageCapabilityTest.kt | 55 +++++++- 17 files changed, 667 insertions(+), 49 deletions(-) create mode 100644 apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ReportSubject.kt create mode 100644 apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportReasonLabels.kt create mode 100644 apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt create mode 100644 apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ChatStep.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ChatStep.kt index 0efc8db555..798f3726d9 100644 --- a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ChatStep.kt +++ b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ChatStep.kt @@ -67,6 +67,20 @@ sealed interface ChatStep : FlowStep, Parcelable { @Serializable data object MuteChat : ChatStep, Sheet, WrapContentSheet + /** + * Why this is being reported. + * + * A [WrapContentSheet] for the same reasons [MuteChat] is: a short list of choices, opened from + * somewhere you stay. Unlike [MuteChat] it carries its [subject], because the same sheet is + * reached from a person's profile, a group's profile and a message in the transcript, and only + * the caller knows which. The chosen reason travels back as a `ChatViewModel` event rather than + * a nav result, matching muting: filing a report is a request the view model makes, not an + * answer the sheet returns. + */ + @Parcelize + @Serializable + data class Report(val subject: ReportSubject) : ChatStep, Sheet, WrapContentSheet + @Parcelize @Serializable data class Profile(val contact: ChatParticipant): ChatStep diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ReportSubject.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ReportSubject.kt new file mode 100644 index 0000000000..bcf95d3568 --- /dev/null +++ b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ReportSubject.kt @@ -0,0 +1,32 @@ +package com.flipcash.app.core.chat + +import android.os.Parcelable +import kotlinx.parcelize.Parcelize +import kotlinx.serialization.Serializable + +/** + * What a report opened from the chat flow is about. + * + * Not [com.flipcash.services.models.ReportTarget], which is what finally goes on the wire: that one + * names a chat by id, and the id is something the flow's view model holds rather than something a + * navigation step should carry a second copy of. This says which *surface* asked, and + * `ChatViewModel` turns it into a target with the ids it already has. + * + * The user case carries its participant for the same reason [ChatStep.Profile] does — a member's + * profile is opened from a bubble, so the person is not the one the flow was opened on. + */ +@Serializable +sealed interface ReportSubject : Parcelable { + @Parcelize + @Serializable + data class User(val participant: ChatParticipant) : ReportSubject + + /** The conversation the flow is already open on. */ + @Parcelize + @Serializable + data object Chat : ReportSubject + + @Parcelize + @Serializable + data class Message(val messageId: Long) : ReportSubject +} diff --git a/apps/flipcash/core/src/main/res/values/strings.xml b/apps/flipcash/core/src/main/res/values/strings.xml index 92cb2f7e00..b4e53fe4b6 100644 --- a/apps/flipcash/core/src/main/res/values/strings.xml +++ b/apps/flipcash/core/src/main/res/values/strings.xml @@ -1165,6 +1165,21 @@ Something Went Wrong We were unable to unblock the user. Please try again + Report + Tell us what’s wrong. Your report is private — we won’t tell them who reported + Spam + Scam or Fraud + Harassment or Bullying + Sexual Content + Violence or Threats + Something Else + Tell us more + Submit Report + Report Sent + Thanks. We’ll take a look + Something Went Wrong + We were unable to send your report. Please try again + Send Your First Tip diff --git a/apps/flipcash/features/messenger/build.gradle.kts b/apps/flipcash/features/messenger/build.gradle.kts index 606acb52d5..20760fb700 100644 --- a/apps/flipcash/features/messenger/build.gradle.kts +++ b/apps/flipcash/features/messenger/build.gradle.kts @@ -24,6 +24,7 @@ dependencies { implementation(project(":apps:flipcash:shared:userflags")) implementation(project(":libs:vibrator:bindings")) implementation(project(":libs:messaging")) + implementation(project(":libs:reporting")) implementation(project(":services:flipcash")) implementation(project(":services:opencode")) implementation(project(":libs:datetime")) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ChatFlowScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ChatFlowScreen.kt index 046c1158cc..003521da8c 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ChatFlowScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ChatFlowScreen.kt @@ -17,12 +17,14 @@ import com.flipcash.app.core.chat.ChatIdentifier import com.flipcash.app.core.chat.ChatParticipant import com.flipcash.app.core.chat.ChatSendResult import com.flipcash.app.core.chat.ChatStep +import com.flipcash.app.core.chat.ReportSubject import com.flipcash.app.core.extensions.openAsSheet import com.flipcash.app.messenger.internal.ChatSubject import com.flipcash.app.messenger.internal.ChatViewModel import com.flipcash.app.messenger.internal.screens.GroupInviteSheet import com.flipcash.app.messenger.internal.screens.MessengerScreen import com.flipcash.app.messenger.internal.screens.MuteChatSheet +import com.flipcash.app.messenger.internal.screens.ReportSheet import com.flipcash.app.messenger.internal.screens.cash.ChatAmountEntryContent import com.flipcash.app.messenger.internal.screens.cash.ChatInitPaymentSheet import com.flipcash.app.messenger.internal.screens.profile.ChatProfileScreen @@ -64,8 +66,8 @@ fun ChatFlowScreen( // Popping with the IME still up drags the screen behind it out from under the keyboard. onExit = { _, _ -> keyboard.hideIfVisible { navigator.pop() } }, entryProvider = chatEntryProvider(route.identifier, route.openKeyboard), - // ChatStep.AmountEntry, ChatStep.InitPayment, ChatStep.InviteToGroup and ChatStep.MuteChat - // are Sheets, so the + // ChatStep.AmountEntry, ChatStep.InitPayment, ChatStep.InviteToGroup, ChatStep.MuteChat + // and ChatStep.Report are Sheets, so the // flow needs the sheet strategy to draw them as such; without it the step would fall // through to SinglePane and cover the thread. Amount entry // returns its result inside the flow (resultBackNavigator), so the strategy's own @@ -98,6 +100,10 @@ private fun chatEntryProvider( annotatedEntry { FlowMuteChatSheet() } + + annotatedEntry { step -> + FlowReportSheet(step.subject) + } annotatedEntry { step -> FlowChatProfileScreen(step.contact) } @@ -177,6 +183,21 @@ private fun FlowConversationScreen(identifier: ChatIdentifier, openKeyboard: Boo } } + // The selection bar reaches the report sheet from here rather than navigating itself: the bar + // is rebuilt from the selection and the sheet has to outlive clearing it. The two profile rows + // call `navigateTo` directly because they are already inside the flow and have nothing to + // outlive. + val flowNavigator = rememberFlowNavigator() + LaunchedEffect(viewModel) { + viewModel.eventFlow + .filterIsInstance() + .collect { (subject) -> + keyboard.hideIfVisible { + flowNavigator.navigateTo(ChatStep.Report(subject)) + } + } + } + MessengerScreen(viewModel) } @@ -274,6 +295,28 @@ private fun FlowMuteChatSheet() { ) } +@Composable +private fun FlowReportSheet(subject: ReportSubject) { + val viewModel = flowSharedViewModel() + // Same dismissal rule as the other sheets in this flow: exit through the sheet so it animates + // down rather than having its scene deleted mid-frame. + val dismissSheet = LocalBottomSheetDismissDispatcher.current + + ReportSheet( + // Dismissed on submit rather than on the result: the request is fire-and-forget from the + // sheet's point of view, and the outcome arrives as a bottom bar over whatever is + // underneath. Holding the sheet open for a round trip would make a two-tap action feel + // like a form. + onSubmit = { reason, details -> + viewModel.dispatchEvent( + ChatViewModel.Event.ReportSubmitted(subject, reason, details) + ) + dismissSheet() + }, + onDismiss = dismissSheet, + ) +} + @Composable private fun FlowChatProfileScreen(participant: ChatParticipant) { val viewModel = flowSharedViewModel() diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt index 23502ab942..71cab1557b 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt @@ -20,42 +20,30 @@ import com.flipcash.app.contacts.ContactCoordinator import com.flipcash.app.core.AppRoute import com.flipcash.app.core.chat.ChatIdentifier import com.flipcash.app.core.chat.ChatParticipant +import com.flipcash.app.core.chat.ReportSubject import com.flipcash.app.core.contacts.DeviceContact import com.flipcash.app.core.extensions.setText import com.flipcash.app.core.tokens.brandedName import com.flipcash.app.core.tokens.isReserve import com.flipcash.app.core.ui.ConfirmationStyle import com.flipcash.app.core.util.Linkify -import com.flipcash.shared.chat.MessageCapability -import com.flipcash.shared.chat.MessagePolicy -import com.flipcash.shared.chat.withinWindows -import com.flipcash.shared.chat.applying -import com.flipcash.shared.chat.resolveCapabilities -import com.flipcash.shared.chat.models.ChatListItem -import com.flipcash.shared.chat.models.SenderIdentity -import com.flipcash.shared.chat.models.ChatQuote -import com.flipcash.shared.chat.models.ChatQuoteSnippet -import com.flipcash.shared.chat.models.LinkCard -import com.flipcash.shared.chat.models.LinkCardResolution -import com.flipcash.shared.chat.models.ReceiptStatus -import com.flipcash.shared.chat.models.SeparatorConfig -import com.flipcash.shared.chat.ui.detectUrls -import com.flipcash.shared.chat.ui.linkableText import com.flipcash.app.funding.PurchaseMethodController -import com.flipcash.app.session.CashLinkClaims -import com.flipcash.app.session.SettledClaim import com.flipcash.app.messenger.internal.link.ClaimReplyTargets import com.flipcash.app.messenger.internal.link.LinkCardClassifier import com.flipcash.app.messenger.internal.link.LinkCardResolver +import com.flipcash.app.session.CashLinkClaims +import com.flipcash.app.session.SettledClaim import com.flipcash.app.tokens.TokenCoordinator import com.flipcash.app.userflags.UserFlagsCoordinator import com.flipcash.features.messenger.R +import com.flipcash.reporting.ReportDescription +import com.flipcash.reporting.ReportReason +import com.flipcash.services.controllers.ReportingController import com.flipcash.services.models.JoinChatError +import com.flipcash.services.models.ReportTarget import com.flipcash.services.models.TipAction import com.flipcash.services.models.TipOrigin import com.flipcash.services.models.UserProfile -import com.flipcash.shared.chat.GroupAccess -import com.flipcash.shared.chat.groupAccess import com.flipcash.services.models.chat.ChatId import com.flipcash.services.models.chat.ChatMessage import com.flipcash.services.models.chat.ChatType @@ -73,36 +61,58 @@ import com.flipcash.shared.chat.ActiveTypist import com.flipcash.shared.chat.ChatCoordinator import com.flipcash.shared.chat.ChatDraftSnapshot import com.flipcash.shared.chat.ChatDraftStore -import com.flipcash.shared.chat.chatDraftOf import com.flipcash.shared.chat.ChatMembership +import com.flipcash.shared.chat.GroupAccess +import com.flipcash.shared.chat.MessageCapability +import com.flipcash.shared.chat.MessagePolicy +import com.flipcash.shared.chat.applying +import com.flipcash.shared.chat.chatDraftOf +import com.flipcash.shared.chat.groupAccess +import com.flipcash.shared.chat.models.ChatListItem +import com.flipcash.shared.chat.models.ChatQuote +import com.flipcash.shared.chat.models.ChatQuoteSnippet +import com.flipcash.shared.chat.models.LinkCard +import com.flipcash.shared.chat.models.LinkCardResolution +import com.flipcash.shared.chat.models.ReceiptStatus +import com.flipcash.shared.chat.models.SenderIdentity +import com.flipcash.shared.chat.models.SeparatorConfig +import com.flipcash.shared.chat.resolveCapabilities +import com.flipcash.shared.chat.ui.detectUrls +import com.flipcash.shared.chat.ui.linkableText +import com.flipcash.shared.chat.withinWindows import com.flipcash.shared.payments.ContactPaymentDelegate import com.flipcash.shared.payments.TipPaymentDelegate -import com.getcode.solana.keys.Mint -import com.getcode.utils.hexEncodedString -import com.getcode.opencode.model.core.ID import com.getcode.manager.BottomBarAction import com.getcode.manager.BottomBarManager import com.getcode.opencode.controllers.TransactionController import com.getcode.opencode.exchange.Exchange import com.getcode.opencode.exchange.VerifiedFiatCalculator +import com.getcode.opencode.model.core.ID import com.getcode.opencode.model.core.errors.ComputeVerifiedFiatError import com.getcode.opencode.model.financial.Fiat import com.getcode.opencode.model.financial.Limits import com.getcode.opencode.model.financial.SendLimit import com.getcode.opencode.model.financial.Token +import com.getcode.solana.keys.Mint import com.getcode.ui.utils.generateComplementaryColorPalette import com.getcode.util.resources.ResourceHelper +import com.getcode.utils.hexEncodedString import com.getcode.utils.trace import com.getcode.view.BaseViewModel import com.getcode.view.LoadingSuccessState import dagger.hilt.android.lifecycle.HiltViewModel +import javax.inject.Inject +import kotlin.math.min +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.seconds import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.debounce @@ -113,6 +123,7 @@ import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapNotNull @@ -120,15 +131,9 @@ import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.transformLatest import kotlinx.coroutines.flow.update -import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import javax.inject.Inject -import kotlin.math.min -import kotlin.time.Duration -import kotlin.time.Duration.Companion.milliseconds -import kotlin.time.Duration.Companion.seconds data class TypingConstraints( val enabled: Boolean = false, @@ -160,6 +165,7 @@ internal class ChatViewModel @Inject constructor( private val linkCardResolver: LinkCardResolver, private val cashLinkClaims: CashLinkClaims, private val chatDraftStore: ChatDraftStore, + private val reporting: ReportingController, ) : BaseViewModel( initialState = State(), updateStateForEvent = updateStateForEvent, @@ -395,6 +401,26 @@ internal class ChatViewModel @Inject constructor( /** The leave went through, so whatever is showing the group's profile should close. */ data object LeftChat : Event + /** + * A surface asked for the report sheet. Raised by the selection bar, which dispatches + * events and holds no navigator; the conversation screen collects it and navigates, the + * same way it handles [OpenScreen]. + */ + data class OpenReportSheet(val subject: ReportSubject) : Event + + /** + * A report the user has finished composing in [ChatStep.Report]. + * + * Carries the subject back rather than reading it off state, because the sheet may outlive + * the selection that opened it: a message report taken from the selection bar is still + * about that message after the bar has cleared. + */ + data class ReportSubmitted( + val subject: ReportSubject, + val reason: ReportReason, + val details: String?, + ) : Event + /** This chat's viewer state moved, from the stream or from the viewer's own request. */ data class OnViewerStateResolved(val viewerState: ViewerState?) : Event @@ -1495,6 +1521,49 @@ internal class ChatViewModel @Inject constructor( } .launchIn(viewModelScope) + eventFlow.filterIsInstance() + .onEach { event -> + val chatId = stateFlow.value.chatId + val target = when (val subject = event.subject) { + // The user, not the DM. A tip DM's id is derived on the client, so naming the + // chat here would name something the server has never been told about; and a + // report made from a group member's profile is about the person either way. + is ReportSubject.User -> (subject.participant as? ChatParticipant.TipUser) + ?.let { ReportTarget.User(it.userId) } + ReportSubject.Chat -> chatId?.let { ReportTarget.Chat(it) } + is ReportSubject.Message -> chatId?.let { + ReportTarget.Message(chatId = it, messageId = subject.messageId) + } + } ?: return@onEach BottomBarManager.showError( + title = resources.getString(R.string.error_title_failedToReport), + message = resources.getString(R.string.error_description_failedToReport), + ) + + reporting.report(target, ReportDescription.build(event.reason, event.details)) + .onSuccess { + // The contract makes a duplicate report a no-op that answers OK, so this + // says the same thing whether or not the report was the first one. That is + // the intended reading: telling someone they had already reported this + // would be answering a question they did not ask. + BottomBarManager.showMessage( + title = resources.getString(R.string.prompt_title_reportSubmitted), + message = resources.getString( + R.string.prompt_description_reportSubmitted + ), + ) + } + .onFailure { + trace("failed to report - ${it.localizedMessage}") + BottomBarManager.showError( + title = resources.getString(R.string.error_title_failedToReport), + message = resources.getString( + R.string.error_description_failedToReport + ), + ) + } + } + .launchIn(viewModelScope) + eventFlow.filterIsInstance() .mapNotNull { stateFlow.value.subject as? ChatSubject.Group } .onEach { group -> @@ -2075,6 +2144,8 @@ internal class ChatViewModel @Inject constructor( Event.LeaveConfirmed, Event.LeftChat, is Event.MuteChat, + is Event.OpenReportSheet, + is Event.ReportSubmitted, Event.UnmuteChat -> { state -> state } is Event.OnTipUserResolved -> { state -> // A device contact, once matched, wins over the server profile (it carries the diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportReasonLabels.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportReasonLabels.kt new file mode 100644 index 0000000000..ea727c0ec9 --- /dev/null +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportReasonLabels.kt @@ -0,0 +1,23 @@ +package com.flipcash.app.messenger.internal.screens + +import androidx.annotation.StringRes +import com.flipcash.features.messenger.R +import com.flipcash.reporting.ReportReason + +/** + * The label for each reason. + * + * An exhaustive `when` rather than a field on the enum: [ReportReason] is shared with iOS and + * compiled for both, and an Android string resource cannot cross that boundary. Exhaustive so a + * reason added upstream fails the build here instead of shipping a row with no words on it. + */ +@get:StringRes +internal val ReportReason.labelRes: Int + get() = when (this) { + ReportReason.Spam -> R.string.action_reportReason_spam + ReportReason.ScamOrFraud -> R.string.action_reportReason_scamOrFraud + ReportReason.Harassment -> R.string.action_reportReason_harassment + ReportReason.SexualContent -> R.string.action_reportReason_sexualContent + ReportReason.Violence -> R.string.action_reportReason_violence + ReportReason.Other -> R.string.action_reportReason_other + } diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt new file mode 100644 index 0000000000..afc0438c5f --- /dev/null +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt @@ -0,0 +1,131 @@ +package com.flipcash.app.messenger.internal.screens + +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.imePadding +import androidx.compose.foundation.layout.navigationBarsPadding +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.text.input.InputTransformation +import androidx.compose.foundation.text.input.rememberTextFieldState +import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.outlined.Flag +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 +import androidx.compose.ui.graphics.vector.rememberVectorPainter +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.text.style.TextAlign +import com.flipcash.features.messenger.R +import com.flipcash.reporting.ReportDescription +import com.flipcash.reporting.ReportReason +import com.getcode.theme.CodeTheme +import com.getcode.ui.components.AppBarDefaults +import com.getcode.ui.components.AppBarWithTitle +import com.getcode.ui.components.ChoiceRow +import com.getcode.ui.components.TextInput +import com.getcode.ui.theme.CodeButton + +/** + * Why this is being reported. + * + * One row per [ReportReason], drawn as [MuteChatSheet] draws its rows. Every row but the last is + * the whole interaction: the tap is the answer, so a report is two taps from anywhere it can be + * started. [ReportReason.Other] is the exception, and it is why this sheet has a second state at + * all — a list that cannot say anything outside its own six words is a list people work around by + * picking the nearest wrong one, which is worse than no taxonomy. + * + * The details field does not appear beside the rows, because a field standing open next to six + * one-tap choices reads as required. It replaces them, after the choice that needs it. + * + * What the sheet hands back is the reason and the raw details; assembling the two into the + * contract's single `description` string is [ReportDescription]'s job, in shared code, so that + * both platforms send the same bytes. Do not format anything here. + * + * @param onSubmit called once, with the chosen reason and the details the user typed, or `null` + * where the reason needed none. + */ +@Composable +internal fun ReportSheet( + onSubmit: (ReportReason, String?) -> Unit, + onDismiss: () -> Unit, +) { + var describing by remember { mutableStateOf(false) } + val details = rememberTextFieldState() + + Column( + modifier = Modifier + .fillMaxWidth() + .navigationBarsPadding() + .imePadding(), + ) { + AppBarWithTitle( + title = stringResource(R.string.title_report), + titleAlignment = Alignment.CenterHorizontally, + endContent = { AppBarDefaults.Close(onClick = onDismiss) }, + ) + + Text( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = CodeTheme.dimens.inset), + text = stringResource(R.string.subtitle_report), + style = CodeTheme.typography.textSmall, + color = CodeTheme.colors.textSecondary, + textAlign = TextAlign.Center, + ) + + Column( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = CodeTheme.dimens.inset) + .padding(top = CodeTheme.dimens.grid.x4, bottom = CodeTheme.dimens.grid.x4), + verticalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x3), + ) { + if (describing) { + val typed = details.text.toString() + TextInput( + modifier = Modifier + .fillMaxWidth() + .testTag("input_report_details"), + state = details, + placeholder = stringResource(R.string.hint_reportDetails), + // Clipped rather than rejected: the cap is the contract's, not a rule the user + // broke, and a field that stops accepting characters is clearer than an error + // about a limit nobody was told about. + inputTransformation = InputTransformation { + if (length > ReportDescription.MAX_DETAILS_LENGTH) revertAllChanges() + }, + ) + CodeButton( + modifier = Modifier + .fillMaxWidth() + .testTag("action_submit_report"), + text = stringResource(R.string.action_submitReport), + // Picking "Something Else" and then saying nothing leaves the report with less + // information than any other row would have carried, so the button waits. + enabled = typed.isNotBlank(), + onClick = { onSubmit(ReportReason.Other, typed.trim()) }, + ) + } else { + for (reason in ReportReason.entries) { + ChoiceRow( + modifier = Modifier.testTag("action_report_reason_${reason.name}"), + label = stringResource(reason.labelRes), + icon = rememberVectorPainter(Icons.Outlined.Flag), + onClick = { + if (reason == ReportReason.Other) describing = true + else onSubmit(reason, null) + }, + ) + } + } + } + } +} diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt index 07c061449a..5f8fdefa72 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt @@ -23,6 +23,7 @@ import androidx.compose.material.icons.automirrored.outlined.Reply import androidx.compose.material.icons.outlined.ContentCopy import androidx.compose.material.icons.outlined.Delete import androidx.compose.material.icons.outlined.Edit +import androidx.compose.material.icons.outlined.Flag import androidx.compose.material3.DropdownMenu import androidx.compose.material3.DropdownMenuItem import androidx.compose.material3.Icon @@ -43,8 +44,10 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.DpOffset import androidx.compose.ui.unit.dp +import com.flipcash.app.core.chat.ReportSubject import com.flipcash.app.messenger.internal.ChatSubject import com.flipcash.app.messenger.internal.ChatViewModel +import com.flipcash.app.messenger.internal.screens.components.ChatTopEdge.topFade import com.flipcash.features.messenger.R import com.flipcash.shared.chat.MessageCapability import com.flipcash.shared.chat.models.ChatAction @@ -57,7 +60,6 @@ import com.getcode.theme.extraLarge import com.getcode.ui.components.AppBarDefaults import com.getcode.ui.components.AppBarWithTitle import com.getcode.ui.components.CircularIconButton -import com.flipcash.app.messenger.internal.screens.components.ChatTopEdge.topFade import com.getcode.ui.core.measured import com.getcode.ui.core.unboundedClickable import com.getcode.ui.utils.KeyboardController @@ -232,7 +234,9 @@ private fun MessageSelectionBar( // Order is priority: the first actions keep their icons when the bar runs out of room. Reply // leads because it is the most common action and the only one a cash bubble offers — burying it // is the one choice that would leave that bubble's bar empty. Delete follows: putting the one - // action with a confirmation behind a menu makes it a three-tap job. + // action with a confirmation behind a menu makes it a three-tap job. Report goes last: it is + // the rarest of the five and the only one that is never about your own message, so it is the + // right one to collapse into the overflow on a narrow screen. val actions = buildList { if (MessageCapability.Reply in capabilities) { add( @@ -281,6 +285,22 @@ private fun MessageSelectionBar( ) ) } + if (MessageCapability.Report in capabilities) { + add( + MessageAction( + label = stringResource(R.string.title_report), + icon = Icons.Outlined.Flag, + testTag = "action_report_message", + onClick = { + dispatch( + ChatViewModel.Event.OpenReportSheet( + ReportSubject.Message(selection.messageId) + ) + ) + }, + ) + ) + } } AppBarWithTitle( diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ChatProfileScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ChatProfileScreen.kt index fc93d31e67..3c9f45f93d 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ChatProfileScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ChatProfileScreen.kt @@ -21,6 +21,7 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.flipcash.app.core.chat.ChatParticipant import com.flipcash.app.core.chat.ChatStep +import com.flipcash.app.core.chat.ReportSubject import com.flipcash.app.menu.MenuItem import com.flipcash.app.menu.MenuList import com.flipcash.app.messenger.internal.ChatMuteStatusChip @@ -72,12 +73,14 @@ internal fun ChatProfileScreen( modifier = Modifier .fillMaxSize() .padding(innerPadding), - // Mute first, block last: the one that is reversible and routine sits above the one - // that ends the conversation, the same order the group's profile puts leaving in. + // Mute first, then report, block last: the reversible and routine sits above the one + // that asks someone else to look, which sits above the one that ends the conversation. + // Same shape as the group's profile, where leaving holds the last place. items = buildList> { if (chatState.chatType == ChatType.TIP_DM) { add(MuteDm) } + add(ReportUser) add(BlockUser) }, header = { @@ -105,6 +108,11 @@ internal fun ChatProfileScreen( // Both muting and unmuting go through the picker, which is why this row // navigates either way rather than acting on one of them here. ChatProfileAction.Mute -> flowNavigator.navigateTo(ChatStep.MuteChat) + ChatProfileAction.Report -> state.participant?.let { participant -> + flowNavigator.navigateTo( + ChatStep.Report(ReportSubject.User(participant)) + ) + } } }, endSlot = { item -> diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileMenuItems.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileMenuItems.kt index c0c0e2eb53..9c4c0fc4b4 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileMenuItems.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileMenuItems.kt @@ -2,6 +2,7 @@ package com.flipcash.app.messenger.internal.screens.profile import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.outlined.Logout +import androidx.compose.material.icons.outlined.Flag import androidx.compose.material.icons.outlined.PersonAdd import androidx.compose.runtime.Composable import androidx.compose.ui.graphics.painter.Painter @@ -22,6 +23,7 @@ internal sealed interface GroupProfileAction { data object Invite : GroupProfileAction data object Mute : GroupProfileAction data object Leave : GroupProfileAction + data object Report : GroupProfileAction } /** @@ -44,6 +46,16 @@ internal data object InviteToGroup : FullMenuItem() { override val action: GroupProfileAction = GroupProfileAction.Invite } +internal data object ReportGroup : FullMenuItem() { + override val icon: Painter + @Composable get() = rememberVectorPainter(Icons.Outlined.Flag) + + override val name: String + @Composable get() = stringResource(R.string.title_report) + + override val action: GroupProfileAction = GroupProfileAction.Report +} + internal data object LeaveChat : FullMenuItem() { override val icon: Painter @Composable get() = rememberVectorPainter(Icons.AutoMirrored.Outlined.Logout) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileScreen.kt index d42bd5ebd3..2a9c912440 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileScreen.kt @@ -19,6 +19,7 @@ import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.text.style.TextOverflow import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.flipcash.app.core.chat.ChatStep +import com.flipcash.app.core.chat.ReportSubject import com.flipcash.app.menu.MenuItem import com.flipcash.app.menu.MenuList import com.flipcash.app.messenger.internal.ChatMuteStatusChip @@ -67,16 +68,18 @@ internal fun GroupProfileScreen(viewModel: ChatViewModel) { modifier = Modifier .fillMaxSize() .padding(innerPadding), - // Every row is for members only: a non-member has no link to hand out, nothing to - // leave, and no viewer state on a chat they are not in. Membership comes from the + // Every other row is for members only: a non-member has no link to hand out, nothing + // to leave, and no viewer state on a chat they are not in. Membership comes from the // roster rather than from the gate, so the rows go away the moment the leave itself // lands, not when the balance rule next re-decides. + // + // Reporting is the exception, and deliberately so: a group you have already left is + // the one you are most likely to report. items = buildList> { if (state.groupInviteUrl != null) add(InviteToGroup) - if (group?.isMember == true) { - add(MuteChat) - add(LeaveChat) - } + if (group?.isMember == true) add(MuteChat) + add(ReportGroup) + if (group?.isMember == true) add(LeaveChat) }, header = { GroupProfileHeader( @@ -105,6 +108,8 @@ internal fun GroupProfileScreen(viewModel: ChatViewModel) { GroupProfileAction.Mute -> flowNavigator.navigateTo(ChatStep.MuteChat) GroupProfileAction.Leave -> viewModel.dispatchEvent(ChatViewModel.Event.LeaveChat) + GroupProfileAction.Report -> + flowNavigator.navigateTo(ChatStep.Report(ReportSubject.Chat)) } }, ) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ProfileMenuItems.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ProfileMenuItems.kt index 569cac053d..cb9ffce220 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ProfileMenuItems.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ProfileMenuItems.kt @@ -2,6 +2,7 @@ package com.flipcash.app.messenger.internal.screens.profile import androidx.compose.material.icons.Icons import androidx.compose.material.icons.outlined.Block +import androidx.compose.material.icons.outlined.Flag import androidx.compose.runtime.Composable import androidx.compose.ui.graphics.painter.Painter import androidx.compose.ui.graphics.vector.rememberVectorPainter @@ -21,6 +22,17 @@ import com.flipcash.features.messenger.R internal sealed interface ChatProfileAction { data object Block : ChatProfileAction data object Mute : ChatProfileAction + data object Report : ChatProfileAction +} + +internal data object ReportUser : FullMenuItem() { + override val icon: Painter + @Composable get() = rememberVectorPainter(Icons.Outlined.Flag) + + override val name: String + @Composable get() = stringResource(R.string.title_report) + + override val action: ChatProfileAction = ChatProfileAction.Report } internal data object BlockUser : FullMenuItem() { diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt index 0dbd97cb83..9eb976cc89 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt @@ -72,7 +72,75 @@ class ChatSelectionBarTest { "action_delete_message", "action_copy_message", "action_edit_message", + "action_report_message", "action_message_overflow", ).forEach { composeTestRule.onAllNodesWithTag(it).assertCountEquals(0) } } + + // Wider than the class default on purpose. Which actions stay as icons and which collapse into + // the overflow falls out of the width, and these two tests are about whether report is offered + // at all — given room for every action, the icon is there or it is not. + @Test + @Config(qualifiers = "w800dp-h800dp-xhdpi") + fun `a message from someone else can be reported`() { + // The capability set is what `resolveCapabilities` returns for another participant's text; + // that mapping has its own tests in `:shared:chat`, so this pins only the bar's reading. + select( + text( + isFromSelf = false, + capabilities = setOf( + MessageCapability.Copy, + MessageCapability.Reply, + MessageCapability.Report, + ), + ) + ) + + composeTestRule.onNodeWithTag("action_report_message").assertIsDisplayed() + } + + @Test + @Config(qualifiers = "w800dp-h800dp-xhdpi") + fun `your own message cannot be reported`() { + select( + text( + isFromSelf = true, + capabilities = setOf( + MessageCapability.Copy, + MessageCapability.Reply, + MessageCapability.Edit, + MessageCapability.Delete, + ), + ) + ) + + composeTestRule.onAllNodesWithTag("action_report_message").assertCountEquals(0) + } + + private fun text(isFromSelf: Boolean, capabilities: Set) = + ChatListItem.ContentBubble( + messageId = 2, + contentIndex = 0, + content = MessageContent.Text("hello"), + isFromSelf = isFromSelf, + timestamp = Instant.fromEpochSeconds(1_000), + capabilities = capabilities, + ) + + private fun select(bubble: ChatListItem.ContentBubble) { + composeTestRule.setContent { + FlipcashPreview { + ChatTopBar( + navigator = mockk(relaxed = true), + state = ChatViewModel.State( + chatType = ChatType.CONTACT_DM, + selection = bubble, + ), + onBarHeightChange = {}, + chatActionHandler = {}, + dispatch = {}, + ) + } + } + } } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt new file mode 100644 index 0000000000..696638df40 --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt @@ -0,0 +1,97 @@ +package com.flipcash.app.messenger.internal.screens + +import androidx.activity.ComponentActivity +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.assertIsNotEnabled +import androidx.compose.ui.test.hasSetTextAction +import androidx.compose.ui.test.junit4.AndroidComposeTestRule +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performTextInput +import com.flipcash.app.theme.FlipcashPreview +import com.flipcash.reporting.ReportReason +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** + * Every row but one is the whole interaction: the tap is the answer. [ReportReason.Other] is the + * exception, and the three cases below are what that exception costs — it reveals instead of + * submitting, it carries what was typed, and it will not submit nothing. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], qualifiers = "w400dp-h800dp-xhdpi") +class ReportSheetTest { + + @get:Rule + val composeTestRule = createAndroidComposeRule() + + private var submitted: Pair? = null + + /** + * The editable node itself, reached by its set-text action rather than by tag: `TextInput` + * puts the caller's modifier on the box it draws around the field, and hands the field a + * modifier of its own making, so the focus and text semantics are on a node no tag can name. + * The sheet has one field, so the action identifies it. + */ + private fun AndroidComposeTestRule<*, *>.detailsField() = onNode(hasSetTextAction()) + + private fun showSheet() { + composeTestRule.setContent { + FlipcashPreview { + ReportSheet( + onSubmit = { reason, details -> submitted = reason to details }, + onDismiss = {}, + ) + } + } + } + + @Test + fun `a plain reason submits on the tap, with no details`() { + showSheet() + + composeTestRule.onNodeWithTag("action_report_reason_Spam").performClick() + + assertEquals(ReportReason.Spam, submitted?.first) + assertNull(submitted?.second) + } + + @Test + fun `Other reveals the field instead of submitting`() { + showSheet() + + composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() + + assertNull(submitted) + composeTestRule.detailsField().assertIsDisplayed() + } + + @Test + fun `Other submits the typed details`() { + showSheet() + + composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() + composeTestRule.detailsField().performTextInput("they keep doing the thing") + composeTestRule.onNodeWithTag("action_submit_report").performClick() + + assertEquals(ReportReason.Other, submitted?.first) + assertEquals("they keep doing the thing", submitted?.second) + } + + @Test + fun `Other will not submit an empty field`() { + showSheet() + + composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() + composeTestRule.onNodeWithTag("action_submit_report").assertIsNotEnabled() + composeTestRule.onNodeWithTag("action_submit_report").performClick() + + assertNull(submitted) + } +} diff --git a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/MessageCapability.kt b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/MessageCapability.kt index 34d91cd236..0b139562c4 100644 --- a/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/MessageCapability.kt +++ b/apps/flipcash/shared/chat/src/main/kotlin/com/flipcash/shared/chat/MessageCapability.kt @@ -26,6 +26,7 @@ enum class MessageCapability { Reply, Edit, Delete, + Report, } /** @@ -97,9 +98,15 @@ data class MessagePolicy( * | Own text, confirmed, past the edit window | Copy, Reply, Delete | * | Own text, confirmed, past both windows | Copy, Reply | * | Own text, unconfirmed (`eventSequence == 0`) | none | - * | Another participant's text | Copy, Reply | - * | Any cash or tip message | Reply | + * | Another participant's text | Copy, Reply, Report | + * | Own cash or tip message | Reply | + * | Another participant's cash or tip message | Reply, Report | * | A tombstone | none | + * | A system notice | none | + * + * Report follows one rule: anything a participant sent can be reported, and anything the server + * wrote, or that no longer exists, cannot. Your own messages are left out because reporting one is + * not a thing anyone does, and a row that is always present is a row people stop reading. */ fun resolveCapabilities( message: ChatMessage, @@ -120,7 +127,12 @@ fun resolveCapabilities( // Cash is never editable: `EditMessageRequest.content` accepts Text, Reply, and Media, never // Cash. It is deliberately not deletable either, so a payment cannot be hidden from the // transcript that records it. - if (contents.any { it is MessageContent.Cash }) return setOf(MessageCapability.Reply) + if (contents.any { it is MessageContent.Cash }) { + return buildSet { + add(MessageCapability.Reply) + if (!message.isFromSelf) add(MessageCapability.Report) + } + } // Server-authored notices, not a participant's message. if (contents.all { it is MessageContent.System }) return emptySet() @@ -135,6 +147,8 @@ fun resolveCapabilities( if (message.isFromSelf) { if (hasText) add(MessageCapability.Edit) add(MessageCapability.Delete) + } else { + add(MessageCapability.Report) } }.withinWindows(message.timestamp, policy, now) } @@ -156,7 +170,10 @@ fun Set.withinWindows( when (capability) { MessageCapability.Edit -> policy.editWindow.stillOpen(sentAt, now) MessageCapability.Delete -> policy.deleteWindow.stillOpen(sentAt, now) - MessageCapability.Copy, MessageCapability.Reply -> true + // Reporting has no window on purpose: the edit and delete windows exist because the + // server enforces them, and nothing in the contract limits how old a reportable message + // may be. + MessageCapability.Copy, MessageCapability.Reply, MessageCapability.Report -> true } } diff --git a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessageCapabilityTest.kt b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessageCapabilityTest.kt index f8e80af6d6..8d33e91d97 100644 --- a/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessageCapabilityTest.kt +++ b/apps/flipcash/shared/chat/src/test/kotlin/com/flipcash/shared/chat/MessageCapabilityTest.kt @@ -70,9 +70,9 @@ class MessageCapabilityTest { } @Test - fun `someone else's text message is copyable but not editable or deletable`() { + fun `someone else's text message is copyable and reportable, not editable or deletable`() { assertEquals( - setOf(MessageCapability.Copy, MessageCapability.Reply), + setOf(MessageCapability.Copy, MessageCapability.Reply, MessageCapability.Report), resolveCapabilities(text(isFromSelf = false)), ) } @@ -80,7 +80,56 @@ class MessageCapabilityTest { @Test fun `cash is never editable or deletable, by either party`() { assertEquals(setOf(MessageCapability.Reply), resolveCapabilities(cash())) - assertEquals(setOf(MessageCapability.Reply), resolveCapabilities(cash(isFromSelf = false))) + // Reportable though: a payment is something a participant sent, and it is one of the + // things most worth reporting. + assertEquals( + setOf(MessageCapability.Reply, MessageCapability.Report), + resolveCapabilities(cash(isFromSelf = false)), + ) + } + + /** + * Reporting your own message is left out because it is not a thing anyone does, and a row that + * is always present is a row people stop reading. This is the first capability whose answer + * turns on `isFromSelf` for a reason that is not about authorship privileges, so it is worth + * pinning at both ends. + */ + @Test + fun `your own messages are never reportable`() { + assertEquals( + emptySet(), + resolveCapabilities(text(), now = sentAt).filterTo(mutableSetOf()) { + it == MessageCapability.Report + }, + ) + assertEquals(emptySet(), resolveCapabilities(cash()) - MessageCapability.Reply) + } + + @Test + fun `nothing the server wrote, or that is already gone, can be reported`() { + val theirTombstone = message( + listOf(MessageContent.Deleted(sentAt, otherId)), + isFromSelf = false, + ) + assertEquals(emptySet(), resolveCapabilities(theirTombstone)) + + val theirSystemNotice = message( + listOf(MessageContent.System("Anna joined")), + isFromSelf = false, + ) + assertEquals(emptySet(), resolveCapabilities(theirSystemNotice)) + } + + @Test + fun `reporting has no window, so an old message stays reportable`() { + assertEquals( + setOf(MessageCapability.Copy, MessageCapability.Reply, MessageCapability.Report), + resolveCapabilities( + text(isFromSelf = false), + policy = MessagePolicy.Default, + now = sentAt + 365.days, + ), + ) } @Test From f9f0e7c2f07902997d49127986a2860855131e9c Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 21 Sep 2026 19:02:06 -0400 Subject: [PATCH 02/14] fix(reporting): make the details step reachable, and give it room to write Tapping "Something Else" hung the app. The reason list and the details field were swapped with an AnimatedContent, and this sheet sizes itself to its content: it writes the height it measured into its own state, and that write recomputes the drag detents that are read again while the content is placed. Animating the height therefore re-enters measurement on every pass, and the loop runs off the layout pass rather than the frame clock, so it starves the clock that would have finished the animation. The main thread spun at 100% until Android offered to close the app; the ANR trace names the cycle (updateContentHeight -> invalidateDetents -> updateAnchors). Swapping on a plain `if` changes the measured height once, and the sheet animates its own offset between detents, so the two steps still read as one surface resizing. On top of that, the details step now earns the trip: - A leading back arrow returns to the reason list and keeps what was typed. The system back gesture unwinds the same steps in the same order. - The field opens three lines tall, grows with what is typed, and scrolls past eight. - An expand control, always visible, takes it to the height of the sheet. That height is a fraction of the screen rather than the space left over after layout, because measuring for it is what deadlocked above. - Content padding, so the text no longer touches the border. Six tests cover the step machinery: the arrow appears only in the second step, returning preserves the draft, and back collapses an expanded field before it leaves the step. --- .../core/src/main/res/values/strings.xml | 2 + .../messenger/internal/screens/ReportSheet.kt | 106 +++++++++++++++++- .../internal/screens/ReportSheetTest.kt | 99 +++++++++++++++- 3 files changed, 203 insertions(+), 4 deletions(-) diff --git a/apps/flipcash/core/src/main/res/values/strings.xml b/apps/flipcash/core/src/main/res/values/strings.xml index b4e53fe4b6..6a6a740e8f 100644 --- a/apps/flipcash/core/src/main/res/values/strings.xml +++ b/apps/flipcash/core/src/main/res/values/strings.xml @@ -1175,6 +1175,8 @@ Something Else Tell us more Submit Report + Expand + Collapse Report Sent Thanks. We’ll take a look Something Went Wrong diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt index afc0438c5f..ab2cf1d59c 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt @@ -1,15 +1,23 @@ package com.flipcash.app.messenger.internal.screens +import androidx.activity.compose.BackHandler import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.imePadding import androidx.compose.foundation.layout.navigationBarsPadding import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size import androidx.compose.foundation.text.input.InputTransformation import androidx.compose.foundation.text.input.rememberTextFieldState import androidx.compose.material.icons.Icons +import androidx.compose.material.icons.outlined.CloseFullscreen import androidx.compose.material.icons.outlined.Flag +import androidx.compose.material.icons.outlined.OpenInFull +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue @@ -22,6 +30,7 @@ import androidx.compose.ui.graphics.vector.rememberVectorPainter import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.unit.dp import com.flipcash.features.messenger.R import com.flipcash.reporting.ReportDescription import com.flipcash.reporting.ReportReason @@ -44,6 +53,13 @@ import com.getcode.ui.theme.CodeButton * The details field does not appear beside the rows, because a field standing open next to six * one-tap choices reads as required. It replaces them, after the choice that needs it. * + * Because it replaces them, the two states are one sheet that changes shape rather than two + * sheets: the bar and the subtitle are fixed, and only the region under them animates, so the + * height change reads as the same surface resizing. The leading back arrow and the rightward + * slide are the same fact told twice — "Something Else" is a step forward, and it is reversible. + * Going back keeps what was typed, since [details] is remembered above the swap; a breadcrumb + * that discarded the draft would be a trapdoor. + * * What the sheet hands back is the reason and the raw details; assembling the two into the * contract's single `description` string is [ReportDescription]'s job, in shared code, so that * both platforms send the same bytes. Do not format anything here. @@ -57,8 +73,27 @@ internal fun ReportSheet( onDismiss: () -> Unit, ) { var describing by remember { mutableStateOf(false) } + var expanded by remember { mutableStateOf(false) } val details = rememberTextFieldState() + // How tall the field gets when it is expanded, taken from the window rather than from the + // space left over after laying the sheet out. Measuring for it is what the obvious version of + // this does, and it deadlocks: a wrap-content sheet derives its drag anchors from the height + // its content reports, and the anchors are read again while that content is being placed, so + // a field whose height came from the measurement changes the number it was computed from. The + // loop never settles and the main thread spins until Android kills the app. A fraction of the + // screen is a constant across the whole layout pass, so it cannot feed back. The sheet's own + // detent caps the result, which is what makes this "as tall as the sheet allows" rather than + // an exact height this file would otherwise have to keep in step with the bar and the button. + val expandedFieldHeight = CodeTheme.dimens.screenHeight * 0.65f + + // Both of these are steps, so the system gesture has to agree with the controls on screen, and + // unwind them in the order they were entered. Without this, back would dismiss the whole sheet + // from a state the user can see a way out of. + BackHandler(enabled = describing) { + if (expanded) expanded = false else describing = false + } + Column( modifier = Modifier .fillMaxWidth() @@ -66,9 +101,26 @@ internal fun ReportSheet( .imePadding(), ) { AppBarWithTitle( - title = stringResource(R.string.title_report), + title = { + AppBarDefaults.Title( + text = stringResource(R.string.title_report), + style = CodeTheme.typography.screenTitle, + ) + }, titleAlignment = Alignment.CenterHorizontally, - endContent = { AppBarDefaults.Close(onClick = onDismiss) }, + // Built by hand rather than through the `onBackIconClicked` overload, which resolves + // back-vs-close from the ambient navigator. Here the answer is the sheet's own step, + // not its depth in whatever stack opened it: ✕ always closes the report, and ← only + // exists in the second step. + leftIcon = { + if (describing) { + AppBarDefaults.UpNavigation( + modifier = Modifier.testTag("action_report_back"), + onClick = { describing = false; expanded = false }, + ) + } + }, + rightContents = { AppBarDefaults.Close(onClick = onDismiss) }, ) Text( @@ -81,6 +133,18 @@ internal fun ReportSheet( textAlign = TextAlign.Center, ) + // The swap is instantaneous, and it has to be. This sheet sizes itself to its content: + // it writes the height it measured into its own state, and that write recomputes the drag + // detents. Content whose height is being animated therefore re-enters measurement on every + // pass, and because the loop runs off the layout pass rather than the frame clock, it + // starves the clock that would have finished the animation — the app spins at 100% and + // Android kills it. Any cross-fade, slide or size transition over these two steps brings + // that back, so the swap stays a plain `if`. + // + // The morph is still there; it is just the sheet's own. Swapping the content changes the + // measured height once, to a value that does not move, and the sheet animates its offset + // from the old detent to the new one. What the user sees is one surface changing size, + // which is what the two steps being one sheet was supposed to convey. Column( modifier = Modifier .fillMaxWidth() @@ -96,6 +160,43 @@ internal fun ReportSheet( .testTag("input_report_details"), state = details, placeholder = stringResource(R.string.hint_reportDetails), + // Opens at the size of the answer it wants. One line invites one line, + // and "Something Else" is the row that exists because one line was not + // enough. It then grows with what is typed, up to a point past which a + // sheet this tall is worse than a scrollbar — that point is what the + // expand control is for. + minLines = 3, + maxLines = if (expanded) Int.MAX_VALUE else 8, + // Given to the box around the field rather than as `minHeight`, which + // sets a floor on the whole decorated row. The row centres that box in + // itself, so a floor there leaves the text and the placeholder floating in + // the middle of a screen-tall border; sizing the box makes the row wrap to + // it and the text start where the border does. + textModifier = if (expanded) Modifier.height(expandedFieldHeight) + else Modifier, + // Top-aligned so the placeholder sits where the first typed character + // will, rather than centred in a box three lines tall — and so the + // expand control rides the top edge instead of the vertical middle. + textFieldAlignment = Alignment.TopStart, + contentAlignment = Alignment.Top, + contentPadding = PaddingValues(CodeTheme.dimens.grid.x2), + trailingIcon = { + IconButton( + modifier = Modifier.testTag("action_report_expand"), + onClick = { expanded = !expanded }, + ) { + Icon( + imageVector = if (expanded) Icons.Outlined.CloseFullscreen + else Icons.Outlined.OpenInFull, + contentDescription = stringResource( + if (expanded) R.string.action_collapseReportDetails + else R.string.action_expandReportDetails + ), + tint = CodeTheme.colors.textSecondary, + modifier = Modifier.size(CodeTheme.dimens.staticGrid.x3), + ) + } + }, // Clipped rather than rejected: the cap is the contract's, not a rule the user // broke, and a field that stops accepting characters is clearer than an error // about a limit nobody was told about. @@ -127,5 +228,6 @@ internal fun ReportSheet( } } } + } } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt index 696638df40..400f4f2190 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt @@ -3,13 +3,18 @@ package com.flipcash.app.messenger.internal.screens import androidx.activity.ComponentActivity import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.assertIsNotEnabled +import androidx.compose.ui.test.assertCountEquals import androidx.compose.ui.test.hasSetTextAction import androidx.compose.ui.test.junit4.AndroidComposeTestRule import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithTag +import androidx.compose.ui.test.onFirst +import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithTag import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performTextInput import com.flipcash.app.theme.FlipcashPreview +import com.flipcash.features.messenger.R import com.flipcash.reporting.ReportReason import org.junit.Rule import org.junit.Test @@ -21,8 +26,9 @@ import kotlin.test.assertNull /** * Every row but one is the whole interaction: the tap is the answer. [ReportReason.Other] is the - * exception, and the three cases below are what that exception costs — it reveals instead of - * submitting, it carries what was typed, and it will not submit nothing. + * exception, and the cases below are what that exception costs — it reveals instead of + * submitting, it carries what was typed, it will not submit nothing, and, because it is the only + * row that goes somewhere, it is the only one that has to offer a way back. */ @RunWith(RobolectricTestRunner::class) @Config(sdk = [34], qualifiers = "w400dp-h800dp-xhdpi") @@ -41,6 +47,14 @@ class ReportSheetTest { */ private fun AndroidComposeTestRule<*, *>.detailsField() = onNode(hasSetTextAction()) + /** + * The back arrow, of which the semantics tree holds two: [TopAppBarBase] subcomposes its + * leading slot a second time as a `leftIcon_probe`, purely to measure whether a real control + * is there, and that copy is never placed. The placed one is subcomposed first. + */ + private fun AndroidComposeTestRule<*, *>.backArrow() = + onAllNodesWithTag("action_report_back").onFirst() + private fun showSheet() { composeTestRule.setContent { FlipcashPreview { @@ -94,4 +108,85 @@ class ReportSheetTest { assertNull(submitted) } + + @Test + fun `the reason list has nothing to go back to`() { + showSheet() + + composeTestRule.onAllNodesWithTag("action_report_back").assertCountEquals(0) + } + + @Test + fun `the details step goes back to the reasons`() { + showSheet() + + composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() + composeTestRule.backArrow().performClick() + + composeTestRule.onNodeWithTag("action_report_reason_Spam").assertIsDisplayed() + assertNull(submitted) + } + + @Test + fun `going back keeps what was typed`() { + showSheet() + + composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() + composeTestRule.detailsField().performTextInput("half a thought") + composeTestRule.backArrow().performClick() + composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() + + composeTestRule.onNodeWithTag("action_submit_report").performClick() + assertEquals("half a thought", submitted?.second) + } + + private val expandLabel: String + get() = composeTestRule.activity.getString(R.string.action_expandReportDetails) + + private val collapseLabel: String + get() = composeTestRule.activity.getString(R.string.action_collapseReportDetails) + + private fun pressBack() = composeTestRule.runOnUiThread { + composeTestRule.activity.onBackPressedDispatcher.onBackPressed() + } + + @Test + fun `the field offers a way to go full screen`() { + showSheet() + + composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() + + composeTestRule.onNodeWithContentDescription(expandLabel).assertIsDisplayed() + } + + @Test + fun `one control both expands and collapses`() { + showSheet() + + composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() + composeTestRule.onNodeWithTag("action_report_expand").performClick() + composeTestRule.onNodeWithContentDescription(collapseLabel).assertIsDisplayed() + + composeTestRule.onNodeWithTag("action_report_expand").performClick() + composeTestRule.onNodeWithContentDescription(expandLabel).assertIsDisplayed() + } + + /** + * Expanding is a step of its own, so back unwinds it before the step that contains it. A back + * that jumped straight to the reasons would throw away a draft the user was mid-sentence in. + */ + @Test + fun `back leaves full screen before it leaves the details step`() { + showSheet() + + composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() + composeTestRule.detailsField().performTextInput("still typing") + composeTestRule.onNodeWithTag("action_report_expand").performClick() + + pressBack() + composeTestRule.onNodeWithContentDescription(expandLabel).assertIsDisplayed() + + pressBack() + composeTestRule.onNodeWithTag("action_report_reason_Spam").assertIsDisplayed() + } } From d4034c29c53203ee967d245829b63707af703cdd Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 21 Sep 2026 22:42:09 -0400 Subject: [PATCH 03/14] feat(nav): let a route present as a fullscreen sheet Sheets rest at 92.5% of the container, and the 7.5% left showing is what tells you a sheet is a layer over something. A route people work through rather than glance at gains nothing from that sliver and loses the room it costs. FullscreenSheet takes the height back. The sheet rests at full container height, keeps the display's own square top corners instead of carving two notches out of them, and insets its content past the status bar the way a full-screen route's app bar does. Declare it alongside Sheet, as with HalfSheet. --- .../com/getcode/navigation/NavMetadata.kt | 2 + .../kotlin/com/getcode/navigation/Types.kt | 13 +++++ .../scenes/ModalBottomSheetSceneStrategy.kt | 48 +++++++++++++++++-- 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/ui/navigation/src/main/kotlin/com/getcode/navigation/NavMetadata.kt b/ui/navigation/src/main/kotlin/com/getcode/navigation/NavMetadata.kt index 3ff4996d24..85845cb761 100644 --- a/ui/navigation/src/main/kotlin/com/getcode/navigation/NavMetadata.kt +++ b/ui/navigation/src/main/kotlin/com/getcode/navigation/NavMetadata.kt @@ -18,6 +18,7 @@ enum class NavMetadataKeys(val key: String, ) { IsSheet("sheet"), IsWrapContentSheet("sheet_wrap_content"), IsHalfSheet("sheet_half"), + IsFullscreenSheet("sheet_fullscreen"), IsSolitarySheet("sheet_solitary"), NavResultKey("navresult_key"), } @@ -70,6 +71,7 @@ fun KClass<*>.metadata(): Map { NavMetadataKeys.IsSheet.key to Sheet::class.java.isAssignableFrom(this.java), NavMetadataKeys.IsWrapContentSheet.key to WrapContentSheet::class.java.isAssignableFrom(this.java), NavMetadataKeys.IsHalfSheet.key to HalfSheet::class.java.isAssignableFrom(this.java), + NavMetadataKeys.IsFullscreenSheet.key to FullscreenSheet::class.java.isAssignableFrom(this.java), NavMetadataKeys.IsSolitarySheet.key to SolitarySheet::class.java.isAssignableFrom(this.java), NavMetadataKeys.IsNonDismissable.key to NonDismissableRoute::class.java.isAssignableFrom(this.java), NavMetadataKeys.IsNonDraggable.key to NonDraggableRoute::class.java.isAssignableFrom(this.java), diff --git a/ui/navigation/src/main/kotlin/com/getcode/navigation/Types.kt b/ui/navigation/src/main/kotlin/com/getcode/navigation/Types.kt index 2e7de506b7..ee8d0f0a6d 100644 --- a/ui/navigation/src/main/kotlin/com/getcode/navigation/Types.kt +++ b/ui/navigation/src/main/kotlin/com/getcode/navigation/Types.kt @@ -14,6 +14,19 @@ interface WrapContentSheet: NavKey * [com.getcode.ui.utils.AllowSheetExpansionWhenScrollable]. */ interface HalfSheet: NavKey +/** + * Rests at the full height of the screen, status bar included. + * + * Every other sheet stops at 92.5%, which leaves the screen behind it visible as a reminder that + * the sheet is a layer over something. A route that is a screen in its own right — one someone + * works through rather than glances at — has nothing to gain from that sliver and loses the room + * it costs. [com.getcode.navigation.scenes.ModalBottomSheetSceneStrategy] squares off the top + * corners and insets the content past the status bar for these, so the result reads as a screen + * that arrived from the bottom rather than a card wedged against the top of the display. + * + * Declare it alongside [Sheet]; on its own it does nothing, exactly as [HalfSheet] does not. + */ +interface FullscreenSheet: NavKey interface NonDismissableRoute: NavKey interface NonDraggableRoute: NavKey interface SolitarySheet: NavKey diff --git a/ui/navigation/src/main/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneStrategy.kt b/ui/navigation/src/main/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneStrategy.kt index a2968bbae2..03665b4042 100644 --- a/ui/navigation/src/main/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneStrategy.kt +++ b/ui/navigation/src/main/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneStrategy.kt @@ -9,6 +9,7 @@ import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.fillMaxHeight import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.statusBarsPadding import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider @@ -90,6 +91,17 @@ private val Half = SheetDetent("half") { containerHeight, _ -> containerHeight * 0.5f } +/** + * Resting detent for a [com.getcode.navigation.FullscreenSheet]: the whole container, status bar + * included. + * + * [Expanded]'s 7.5% gap is what tells you a sheet is a layer over something. A route that is a + * screen in its own right does not need telling, so it takes the height back. + */ +private val Fullscreen = SheetDetent("fullscreen") { containerHeight, _ -> + containerHeight +} + /** An [OverlayScene] that renders an [entry] within an [UnstyledBottomSheet]. */ internal class ModalBottomSheetScene constructor( override val key: T, @@ -147,13 +159,17 @@ internal class ModalBottomSheetScene constructor( val isWrapContent = metadata[NavMetadataKeys.IsWrapContentSheet.key] as? Boolean ?: false - // A half sheet gains an extra resting detent and opens there; a wrap-content sheet rests - // at its own content height; every other sheet keeps the two-detent (hidden/expanded) - // behaviour. Half wins the overlap: those sheets are sized by the fraction, and - // wrap-content only strips their `fillMaxHeight()` further down. + // A half sheet gains an extra resting detent and opens there; a fullscreen sheet takes + // the whole container; a wrap-content sheet rests at its own content height; every other + // sheet keeps the two-detent (hidden/expanded) behaviour. Half wins the overlap: those + // sheets are sized by the fraction, and wrap-content only strips their `fillMaxHeight()` + // further down. val isHalfSheet = metadata[NavMetadataKeys.IsHalfSheet.key] as? Boolean ?: false + val isFullscreen = + metadata[NavMetadataKeys.IsFullscreenSheet.key] as? Boolean ?: false val restingDetent = when { isHalfSheet -> Half + isFullscreen -> Fullscreen isWrapContent -> WrapContent else -> Expanded } @@ -317,7 +333,19 @@ internal class ModalBottomSheetScene constructor( Sheet( modifier = Modifier .fillMaxWidth() - .clip(RoundedCornerShape(topStart = 28.dp, topEnd = 28.dp)) + // Rounded corners read as a card lifted over the screen behind it. + // A fullscreen sheet has no screen behind it to be lifted over, and + // the curves would only carve two notches out of the display's own + // square corners. + .then( + if (isFullscreen) { + Modifier + } else { + Modifier.clip( + RoundedCornerShape(topStart = 28.dp, topEnd = 28.dp) + ) + } + ) .background(CodeTheme.colors.surface), ) { Box( @@ -327,6 +355,16 @@ internal class ModalBottomSheetScene constructor( if (!isWrapContent) { Modifier.fillMaxHeight() } else Modifier + ) + // Every other sheet rests below the status bar and so never had + // to think about it. This one reaches it: the surface is drawn + // behind the bar (no scrim sliver at the top) and the content is + // inset past it, the way a full-screen route's own app bar + // insets itself. + .then( + if (isFullscreen) { + Modifier.statusBarsPadding() + } else Modifier ), ) { SharedTransitionLayout { From 0a3f428d80cf262247d7db8bbc96707f3267899f Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 21 Sep 2026 22:42:13 -0400 Subject: [PATCH 04/14] docs(nav): say what FlowNavigator.back() already does at the root back() pops the current step, and at the root it calls navigateBack() on a navigator whose onRootReached exits the flow. By the time it returns false the host has already been told. The KDoc said the flow "should be exited by the host", which reads as a request and invites `if (!back()) exitCanceled()`. That exits twice, costing a host that pops on exit one entry more than it opened. --- .../kotlin/com/getcode/navigation/flow/FlowNavigator.kt | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/ui/navigation/src/main/kotlin/com/getcode/navigation/flow/FlowNavigator.kt b/ui/navigation/src/main/kotlin/com/getcode/navigation/flow/FlowNavigator.kt index 4a38171e70..0f493b9796 100644 --- a/ui/navigation/src/main/kotlin/com/getcode/navigation/flow/FlowNavigator.kt +++ b/ui/navigation/src/main/kotlin/com/getcode/navigation/flow/FlowNavigator.kt @@ -30,8 +30,12 @@ interface FlowNavigator { fun replaceStack(steps: List) /** - * Pop the current step. Returns true if a step was popped, or false if this was the root of - * the flow (in which case the flow itself should be exited by the host). + * Pop the current step, or exit the flow if this was its root. + * + * The return value reports which of those happened — true if a step was popped, false if the + * flow was exited. It is not a request for the caller to exit: the host has already been told. + * Following a false return with [exitCanceled] exits a second time, which costs a host that + * pops on exit one entry more than it opened. */ fun back(): Boolean From 4517b74f8672e71de6d463b73b9ae6bc9ff06cdd Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 21 Sep 2026 22:42:45 -0400 Subject: [PATCH 05/14] refactor(reporting): give report its own stepped, fullscreen route Report was a wrap-content sheet inside the chat flow, so its state lived in ChatViewModel and its height came from whatever the content measured. Picking "Something Else" then had to grow that sheet to fit a text field, and resizing a wrap-content sheet spins the main thread: it takes its drag anchors from the content's reported height, then reads them again while placing that content. It ANR'd. Report is now AppRoute.Messaging.Report, a top-level FlowRoute presented as a FullscreenSheet with its own ReportViewModel, stepped as ReasonSelection then Details. Height comes from the container, so the details step has room without anything resizing. The reason list is a radio group carrying a description per reason. Its button reads "Next" for Something Else and "Submit Report" for the rest, so the details step is only reached when it has something to collect. Leaving the first step is a close rather than a back arrow, and it lands you on the profile or chat you opened it from. --- .../ui/navigation/AppScreenContent.kt | 4 + apps/flipcash/core/build.gradle.kts | 1 + .../kotlin/com/flipcash/app/core/AppRoute.kt | 29 +++ .../com/flipcash/app/core/chat/ChatStep.kt | 14 - .../flipcash/app/core/chat/ReportSubject.kt | 27 +- .../flipcash/app/core/reporting/ReportStep.kt | 34 +++ .../core/src/main/res/values/strings.xml | 8 + .../flipcash/app/messenger/ChatFlowScreen.kt | 45 +--- .../app/messenger/ReportFlowScreen.kt | 110 ++++++++ .../app/messenger/internal/ChatViewModel.kt | 70 ----- .../app/messenger/internal/ReportViewModel.kt | 69 +++++ .../internal/screens/ReasonSelectionScreen.kt | 194 ++++++++++++++ .../internal/screens/ReportDetailsScreen.kt | 199 +++++++++++++++ .../internal/screens/ReportReasonLabels.kt | 21 ++ .../messenger/internal/screens/ReportSheet.kt | 233 ----------------- .../internal/screens/components/ChatTopBar.kt | 28 +- .../screens/profile/ChatProfileScreen.kt | 19 +- .../screens/profile/GroupProfileScreen.kt | 12 +- .../internal/screens/ReportFlowTest.kt | 240 ++++++++++++++++++ .../internal/screens/ReportSheetTest.kt | 192 -------------- 20 files changed, 971 insertions(+), 578 deletions(-) create mode 100644 apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/reporting/ReportStep.kt create mode 100644 apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt create mode 100644 apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt create mode 100644 apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt create mode 100644 apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt delete mode 100644 apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt create mode 100644 apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt delete mode 100644 apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/AppScreenContent.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/AppScreenContent.kt index af1ac8cbc1..7fa93a3982 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/AppScreenContent.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/AppScreenContent.kt @@ -36,6 +36,7 @@ import com.flipcash.app.currency.RegionSelectionScreen import com.flipcash.app.deposit.DepositFlowScreen import com.flipcash.app.invite.InviteContactScreen import com.flipcash.app.messenger.ChatFlowScreen +import com.flipcash.app.messenger.ReportFlowScreen import com.flipcash.app.discovery.TokenDiscoveryScreen import com.flipcash.app.internal.ui.navigation.decorators.rememberNavMessagingEntryDecorator import com.flipcash.app.lab.LabsScreen @@ -119,6 +120,9 @@ fun appEntryProvider( annotatedEntry { key -> NewGroupFlowScreen(route = key, resultStateRegistry = resultStateRegistry) } + annotatedEntry { key -> + ReportFlowScreen(route = key, resultStateRegistry = resultStateRegistry) + } // Tokens annotatedEntry(testTag = "token_info_screen") { key -> diff --git a/apps/flipcash/core/build.gradle.kts b/apps/flipcash/core/build.gradle.kts index 71896ec05e..6092a52a90 100644 --- a/apps/flipcash/core/build.gradle.kts +++ b/apps/flipcash/core/build.gradle.kts @@ -29,6 +29,7 @@ dependencies { api(project(":services:flipcash-compose")) + api(project(":libs:reporting")) implementation(project(":libs:messaging")) api(project(":libs:permissions:public")) implementation(project(":libs:vibrator:public")) diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/AppRoute.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/AppRoute.kt index 2f2f3f1219..a9488c8ad9 100644 --- a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/AppRoute.kt +++ b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/AppRoute.kt @@ -4,6 +4,8 @@ import android.os.Parcelable import androidx.navigation3.runtime.NavKey import com.flipcash.app.core.chat.ChatIdentifier import com.flipcash.app.core.chat.ChatStep +import com.flipcash.app.core.chat.ReportSubject +import com.flipcash.app.core.reporting.ReportStep import com.flipcash.app.core.chat.NewGroupStep import com.flipcash.app.core.deposit.DepositResult import com.flipcash.app.core.deposit.DepositStep @@ -360,6 +362,33 @@ sealed interface AppRoute : NavKey, Parcelable { override val initialStack: List get() = listOf(NewGroupStep.Form) } + + /** + * Reporting a person, a group, or a single message. + * + * A top-level route rather than a step of [Chat], although every way in is a chat surface. + * Reporting a person is about the person, not the conversation it was opened from, and the + * flow outlives the surface: it is reachable from a profile, a group profile, and the + * selection bar, and sharing one route means those three cannot drift apart. It also keeps + * a half-written report out of the chat's view model. + * + * A [com.getcode.navigation.FullscreenSheet] without `WrapContentSheet`: it rests at a + * fixed height and stays there while the steps change. The wrap-content version of this + * deadlocked — such a sheet derives its drag anchors from the height its content reports + * and reads them again while placing that content, so content that resizes itself never + * settles. Fullscreen rather than the usual expanded detent because both steps are worked + * through rather than glanced at, and the second is a text field with the keyboard up. + */ + @Serializable + @Parcelize + data class Report(val subject: ReportSubject) : + Messaging, + FlowRoute, + com.getcode.navigation.Sheet, + com.getcode.navigation.FullscreenSheet { + override val initialStack: List + get() = listOf(ReportStep.ReasonSelection) + } } @Serializable diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ChatStep.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ChatStep.kt index 798f3726d9..0efc8db555 100644 --- a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ChatStep.kt +++ b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ChatStep.kt @@ -67,20 +67,6 @@ sealed interface ChatStep : FlowStep, Parcelable { @Serializable data object MuteChat : ChatStep, Sheet, WrapContentSheet - /** - * Why this is being reported. - * - * A [WrapContentSheet] for the same reasons [MuteChat] is: a short list of choices, opened from - * somewhere you stay. Unlike [MuteChat] it carries its [subject], because the same sheet is - * reached from a person's profile, a group's profile and a message in the transcript, and only - * the caller knows which. The chosen reason travels back as a `ChatViewModel` event rather than - * a nav result, matching muting: filing a report is a request the view model makes, not an - * answer the sheet returns. - */ - @Parcelize - @Serializable - data class Report(val subject: ReportSubject) : ChatStep, Sheet, WrapContentSheet - @Parcelize @Serializable data class Profile(val contact: ChatParticipant): ChatStep diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ReportSubject.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ReportSubject.kt index bcf95d3568..3a11b14a5d 100644 --- a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ReportSubject.kt +++ b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/chat/ReportSubject.kt @@ -1,32 +1,37 @@ package com.flipcash.app.core.chat import android.os.Parcelable +import com.flipcash.services.models.chat.ChatId +import com.getcode.opencode.model.core.ID import kotlinx.parcelize.Parcelize import kotlinx.serialization.Serializable /** - * What a report opened from the chat flow is about. + * What a report is about, already resolved to the ids the request will be keyed by. * - * Not [com.flipcash.services.models.ReportTarget], which is what finally goes on the wire: that one - * names a chat by id, and the id is something the flow's view model holds rather than something a - * navigation step should carry a second copy of. This says which *surface* asked, and - * `ChatViewModel` turns it into a target with the ids it already has. + * Mirrors [com.flipcash.services.models.ReportTarget] rather than reusing it: that one belongs to + * the service layer and is not a navigation type, and this has to survive process death on the + * back stack. * - * The user case carries its participant for the same reason [ChatStep.Profile] does — a member's - * profile is opened from a bubble, so the person is not the one the flow was opened on. + * It carries ids rather than naming a surface ("the chat this was opened on") because reporting is + * its own flow now — a top-level route — so there is no shared chat view model behind it to ask. + * Each surface resolves the ids it already holds when it opens the flow. + * + * The user case names the person, not the DM: a tip DM's id is derived on the client, so naming + * the chat would name something the server has never been told about, and a report made from a + * group member's profile is about the person either way. */ @Serializable sealed interface ReportSubject : Parcelable { @Parcelize @Serializable - data class User(val participant: ChatParticipant) : ReportSubject + data class User(val userId: ID) : ReportSubject - /** The conversation the flow is already open on. */ @Parcelize @Serializable - data object Chat : ReportSubject + data class Chat(val chatId: ChatId) : ReportSubject @Parcelize @Serializable - data class Message(val messageId: Long) : ReportSubject + data class Message(val chatId: ChatId, val messageId: Long) : ReportSubject } diff --git a/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/reporting/ReportStep.kt b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/reporting/ReportStep.kt new file mode 100644 index 0000000000..3e09d6d291 --- /dev/null +++ b/apps/flipcash/core/src/main/kotlin/com/flipcash/app/core/reporting/ReportStep.kt @@ -0,0 +1,34 @@ +package com.flipcash.app.core.reporting + +import android.os.Parcelable +import com.flipcash.reporting.ReportReason +import com.getcode.navigation.NonDraggableRoute +import com.getcode.navigation.flow.FlowStep +import kotlinx.parcelize.Parcelize +import kotlinx.serialization.Serializable + +/** + * Picking a reason, and — for [ReportReason.Other] alone — saying what happened. + * + * [NonDraggableRoute] at the interface level, as `CurrencyCreatorStep` does: the enclosing route is + * a sheet, and a sheet that can be dragged away is the wrong affordance over a half-written report. + * Leaving is offered by the app bar, which [com.getcode.navigation.flow.FlowHost] turns from a + * close into a back arrow once there is a step to go back to. + * + * Steps rather than one screen that swaps its own contents. The earlier version did the latter + * inside a wrap-content sheet and deadlocked the main thread: such a sheet takes its drag anchors + * from the height its content reports and reads those anchors again while placing that content, so + * content that resizes itself keeps changing the number it was measured against. A step boundary + * makes the height change once, between two fixed layouts. + */ +@Serializable +sealed interface ReportStep : FlowStep, Parcelable, NonDraggableRoute { + @Parcelize + @Serializable + data object ReasonSelection : ReportStep + + /** Reached only from [ReportReason.Other]; every other reason submits from the list. */ + @Parcelize + @Serializable + data class Details(val reason: ReportReason) : ReportStep +} diff --git a/apps/flipcash/core/src/main/res/values/strings.xml b/apps/flipcash/core/src/main/res/values/strings.xml index 6a6a740e8f..a3cd5afd1e 100644 --- a/apps/flipcash/core/src/main/res/values/strings.xml +++ b/apps/flipcash/core/src/main/res/values/strings.xml @@ -1173,6 +1173,14 @@ Sexual Content Violence or Threats Something Else + Repeated unwanted messages, promotions, or links + Attempts to take money or account details by deception + Abuse, insults, or repeated unwanted contact + Explicit content, or unwanted sexual messages + Threats of harm, or content showing violence + None of these fit — tell us in your own words + Tell us what’s wrong + Your report is private — we won’t tell them who reported Tell us more Submit Report Expand diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ChatFlowScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ChatFlowScreen.kt index 003521da8c..fb6b7c5f21 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ChatFlowScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ChatFlowScreen.kt @@ -17,14 +17,12 @@ import com.flipcash.app.core.chat.ChatIdentifier import com.flipcash.app.core.chat.ChatParticipant import com.flipcash.app.core.chat.ChatSendResult import com.flipcash.app.core.chat.ChatStep -import com.flipcash.app.core.chat.ReportSubject import com.flipcash.app.core.extensions.openAsSheet import com.flipcash.app.messenger.internal.ChatSubject import com.flipcash.app.messenger.internal.ChatViewModel import com.flipcash.app.messenger.internal.screens.GroupInviteSheet import com.flipcash.app.messenger.internal.screens.MessengerScreen import com.flipcash.app.messenger.internal.screens.MuteChatSheet -import com.flipcash.app.messenger.internal.screens.ReportSheet import com.flipcash.app.messenger.internal.screens.cash.ChatAmountEntryContent import com.flipcash.app.messenger.internal.screens.cash.ChatInitPaymentSheet import com.flipcash.app.messenger.internal.screens.profile.ChatProfileScreen @@ -66,8 +64,8 @@ fun ChatFlowScreen( // Popping with the IME still up drags the screen behind it out from under the keyboard. onExit = { _, _ -> keyboard.hideIfVisible { navigator.pop() } }, entryProvider = chatEntryProvider(route.identifier, route.openKeyboard), - // ChatStep.AmountEntry, ChatStep.InitPayment, ChatStep.InviteToGroup, ChatStep.MuteChat - // and ChatStep.Report are Sheets, so the + // ChatStep.AmountEntry, ChatStep.InitPayment, ChatStep.InviteToGroup and + // ChatStep.MuteChat are Sheets, so the // flow needs the sheet strategy to draw them as such; without it the step would fall // through to SinglePane and cover the thread. Amount entry // returns its result inside the flow (resultBackNavigator), so the strategy's own @@ -101,9 +99,6 @@ private fun chatEntryProvider( FlowMuteChatSheet() } - annotatedEntry { step -> - FlowReportSheet(step.subject) - } annotatedEntry { step -> FlowChatProfileScreen(step.contact) } @@ -183,21 +178,6 @@ private fun FlowConversationScreen(identifier: ChatIdentifier, openKeyboard: Boo } } - // The selection bar reaches the report sheet from here rather than navigating itself: the bar - // is rebuilt from the selection and the sheet has to outlive clearing it. The two profile rows - // call `navigateTo` directly because they are already inside the flow and have nothing to - // outlive. - val flowNavigator = rememberFlowNavigator() - LaunchedEffect(viewModel) { - viewModel.eventFlow - .filterIsInstance() - .collect { (subject) -> - keyboard.hideIfVisible { - flowNavigator.navigateTo(ChatStep.Report(subject)) - } - } - } - MessengerScreen(viewModel) } @@ -295,27 +275,6 @@ private fun FlowMuteChatSheet() { ) } -@Composable -private fun FlowReportSheet(subject: ReportSubject) { - val viewModel = flowSharedViewModel() - // Same dismissal rule as the other sheets in this flow: exit through the sheet so it animates - // down rather than having its scene deleted mid-frame. - val dismissSheet = LocalBottomSheetDismissDispatcher.current - - ReportSheet( - // Dismissed on submit rather than on the result: the request is fire-and-forget from the - // sheet's point of view, and the outcome arrives as a bottom bar over whatever is - // underneath. Holding the sheet open for a round trip would make a two-tap action feel - // like a form. - onSubmit = { reason, details -> - viewModel.dispatchEvent( - ChatViewModel.Event.ReportSubmitted(subject, reason, details) - ) - dismissSheet() - }, - onDismiss = dismissSheet, - ) -} @Composable private fun FlowChatProfileScreen(participant: ChatParticipant) { diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt new file mode 100644 index 0000000000..ce5d5f6b9d --- /dev/null +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt @@ -0,0 +1,110 @@ +package com.flipcash.app.messenger + +import android.os.Parcelable +import androidx.compose.foundation.text.input.rememberTextFieldState +import androidx.compose.runtime.Composable +import androidx.navigation3.runtime.NavEntry +import androidx.navigation3.runtime.NavKey +import androidx.navigation3.runtime.entryProvider +import androidx.navigation3.scene.SinglePaneSceneStrategy +import com.flipcash.app.core.AppRoute +import com.flipcash.app.core.chat.ReportSubject +import com.flipcash.app.core.reporting.ReportStep +import com.flipcash.app.messenger.internal.ReportViewModel +import com.flipcash.app.messenger.internal.screens.ReasonSelectionContent +import com.flipcash.app.messenger.internal.screens.ReportDetailsContent +import com.flipcash.reporting.ReportReason +import com.getcode.navigation.annotatedEntry +import com.getcode.navigation.core.LocalCodeNavigator +import com.getcode.navigation.flow.FlowHost +import com.getcode.navigation.flow.flowSharedViewModel +import com.getcode.navigation.flow.rememberFlowNavigator +import com.getcode.navigation.flow.rememberInitialStack +import com.getcode.navigation.results.NavResultStateRegistry +import com.getcode.navigation.scenes.ModalBottomSheetSceneStrategy +import com.getcode.ui.utils.rememberKeyboardController + +/** + * Reporting a person, a group, or a message. + * + * Two steps, and most reports only see the first: picking a reason files it. Only + * [ReportReason.Other] has a second screen, because only it has nothing to go on without one. + * + * Steps rather than one screen that grows: the earlier version swapped its own contents inside a + * wrap-content sheet and deadlocked the main thread. Such a sheet takes its drag anchors from the + * height its content reports and reads those anchors again while placing that content, so content + * that resizes itself keeps changing the number it was measured against, and the loop outruns the + * frame clock that would have ended it. Here the route is a fixed-height sheet and each step is a + * fixed layout, so the height changes once, between them. + */ +@Composable +fun ReportFlowScreen( + route: AppRoute.Messaging.Report, + resultStateRegistry: NavResultStateRegistry, +) { + val navigator = LocalCodeNavigator.current + val keyboard = rememberKeyboardController() + + FlowHost( + initialStack = route.rememberInitialStack(), + resultStateRegistry = resultStateRegistry, + // The details step owns the keyboard, so put it away before the chat behind this is + // uncovered — the same rule the chat and new-group flows follow. + onExit = { _, _ -> keyboard.hideIfVisible { navigator.pop() } }, + entryProvider = reportEntryProvider(route.subject), + sceneStrategies = listOf( + ModalBottomSheetSceneStrategy(navigator.resultStore) { null }, + SinglePaneSceneStrategy(), + ), + ) +} + +@Composable +private fun reportEntryProvider( + subject: ReportSubject, +): (NavKey) -> NavEntry = entryProvider { + annotatedEntry { FlowReasonSelection(subject) } + annotatedEntry { step -> FlowReportDetails(subject, step.reason) } +} + +@Composable +private fun FlowReasonSelection(subject: ReportSubject) { + val viewModel = flowSharedViewModel() + val flowNavigator = rememberFlowNavigator() + + ReasonSelectionContent( + onChoose = { reason -> + if (reason == ReportReason.Other) { + flowNavigator.navigateTo(ReportStep.Details(reason)) + } else { + // Every other reason is the whole report, so the pick is the submit. Asking for + // more after someone has already said what is wrong would be asking twice. + viewModel.submit(subject, reason, details = null) + flowNavigator.exitCanceled() + } + }, + // `back` on its own: at the flow's root it exits the flow itself, so pairing it with an + // `exitCanceled()` for the false return would leave twice and pop the chat with us. + onNavigateUp = { flowNavigator.back() }, + ) +} + +@Composable +private fun FlowReportDetails(subject: ReportSubject, reason: ReportReason) { + val viewModel = flowSharedViewModel() + val flowNavigator = rememberFlowNavigator() + // Remembered against the step, so going back to the reasons and returning keeps what was + // typed — leaving by the back arrow is usually a second thought about the reason, not about + // the words. + val details = rememberTextFieldState() + + ReportDetailsContent( + state = details, + onSubmit = { typed -> + viewModel.submit(subject, reason, typed) + flowNavigator.exitCanceled() + }, + // Not the flow's root, so this steps back to the reasons rather than leaving. + onNavigateUp = { flowNavigator.back() }, + ) +} diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt index 71cab1557b..0b58b4229a 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt @@ -20,7 +20,6 @@ import com.flipcash.app.contacts.ContactCoordinator import com.flipcash.app.core.AppRoute import com.flipcash.app.core.chat.ChatIdentifier import com.flipcash.app.core.chat.ChatParticipant -import com.flipcash.app.core.chat.ReportSubject import com.flipcash.app.core.contacts.DeviceContact import com.flipcash.app.core.extensions.setText import com.flipcash.app.core.tokens.brandedName @@ -36,11 +35,7 @@ import com.flipcash.app.session.SettledClaim import com.flipcash.app.tokens.TokenCoordinator import com.flipcash.app.userflags.UserFlagsCoordinator import com.flipcash.features.messenger.R -import com.flipcash.reporting.ReportDescription -import com.flipcash.reporting.ReportReason -import com.flipcash.services.controllers.ReportingController import com.flipcash.services.models.JoinChatError -import com.flipcash.services.models.ReportTarget import com.flipcash.services.models.TipAction import com.flipcash.services.models.TipOrigin import com.flipcash.services.models.UserProfile @@ -165,7 +160,6 @@ internal class ChatViewModel @Inject constructor( private val linkCardResolver: LinkCardResolver, private val cashLinkClaims: CashLinkClaims, private val chatDraftStore: ChatDraftStore, - private val reporting: ReportingController, ) : BaseViewModel( initialState = State(), updateStateForEvent = updateStateForEvent, @@ -401,26 +395,6 @@ internal class ChatViewModel @Inject constructor( /** The leave went through, so whatever is showing the group's profile should close. */ data object LeftChat : Event - /** - * A surface asked for the report sheet. Raised by the selection bar, which dispatches - * events and holds no navigator; the conversation screen collects it and navigates, the - * same way it handles [OpenScreen]. - */ - data class OpenReportSheet(val subject: ReportSubject) : Event - - /** - * A report the user has finished composing in [ChatStep.Report]. - * - * Carries the subject back rather than reading it off state, because the sheet may outlive - * the selection that opened it: a message report taken from the selection bar is still - * about that message after the bar has cleared. - */ - data class ReportSubmitted( - val subject: ReportSubject, - val reason: ReportReason, - val details: String?, - ) : Event - /** This chat's viewer state moved, from the stream or from the viewer's own request. */ data class OnViewerStateResolved(val viewerState: ViewerState?) : Event @@ -1521,48 +1495,6 @@ internal class ChatViewModel @Inject constructor( } .launchIn(viewModelScope) - eventFlow.filterIsInstance() - .onEach { event -> - val chatId = stateFlow.value.chatId - val target = when (val subject = event.subject) { - // The user, not the DM. A tip DM's id is derived on the client, so naming the - // chat here would name something the server has never been told about; and a - // report made from a group member's profile is about the person either way. - is ReportSubject.User -> (subject.participant as? ChatParticipant.TipUser) - ?.let { ReportTarget.User(it.userId) } - ReportSubject.Chat -> chatId?.let { ReportTarget.Chat(it) } - is ReportSubject.Message -> chatId?.let { - ReportTarget.Message(chatId = it, messageId = subject.messageId) - } - } ?: return@onEach BottomBarManager.showError( - title = resources.getString(R.string.error_title_failedToReport), - message = resources.getString(R.string.error_description_failedToReport), - ) - - reporting.report(target, ReportDescription.build(event.reason, event.details)) - .onSuccess { - // The contract makes a duplicate report a no-op that answers OK, so this - // says the same thing whether or not the report was the first one. That is - // the intended reading: telling someone they had already reported this - // would be answering a question they did not ask. - BottomBarManager.showMessage( - title = resources.getString(R.string.prompt_title_reportSubmitted), - message = resources.getString( - R.string.prompt_description_reportSubmitted - ), - ) - } - .onFailure { - trace("failed to report - ${it.localizedMessage}") - BottomBarManager.showError( - title = resources.getString(R.string.error_title_failedToReport), - message = resources.getString( - R.string.error_description_failedToReport - ), - ) - } - } - .launchIn(viewModelScope) eventFlow.filterIsInstance() .mapNotNull { stateFlow.value.subject as? ChatSubject.Group } @@ -2144,8 +2076,6 @@ internal class ChatViewModel @Inject constructor( Event.LeaveConfirmed, Event.LeftChat, is Event.MuteChat, - is Event.OpenReportSheet, - is Event.ReportSubmitted, Event.UnmuteChat -> { state -> state } is Event.OnTipUserResolved -> { state -> // A device contact, once matched, wins over the server profile (it carries the diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt new file mode 100644 index 0000000000..a17fb9a959 --- /dev/null +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt @@ -0,0 +1,69 @@ +package com.flipcash.app.messenger.internal + +import androidx.lifecycle.ViewModel +import androidx.lifecycle.viewModelScope +import com.flipcash.app.core.chat.ReportSubject +import com.flipcash.features.messenger.R +import com.flipcash.reporting.ReportDescription +import com.flipcash.reporting.ReportReason +import com.flipcash.services.controllers.ReportingController +import com.flipcash.services.models.ReportTarget +import com.getcode.manager.BottomBarManager +import com.getcode.util.resources.ResourceHelper +import com.getcode.utils.trace +import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.launch +import javax.inject.Inject + +/** + * Filing the report the flow has collected. + * + * Scoped to the flow rather than to either step, because the reason is chosen on one screen and — + * for [ReportReason.Other] — described on the next, and the submit can outlive the step that asked + * for it. Previously this lived in the chat's view model, which tied a report to the conversation + * it happened to be opened from and kept a half-written one in chat state. + * + * No state of its own: both screens own their input, and the outcome is a bottom bar prompt rather + * than something the flow renders, so there is nothing to hold across the exit. + */ +@HiltViewModel +internal class ReportViewModel @Inject constructor( + private val reporting: ReportingController, + private val resources: ResourceHelper, +) : ViewModel() { + + /** [details] is the reporter's own words, and only [ReportReason.Other] collects them. */ + fun submit(subject: ReportSubject, reason: ReportReason, details: String?) { + val target = when (subject) { + is ReportSubject.User -> ReportTarget.User(subject.userId) + is ReportSubject.Chat -> ReportTarget.Chat(subject.chatId) + is ReportSubject.Message -> ReportTarget.Message( + chatId = subject.chatId, + messageId = subject.messageId, + ) + } + + // On the flow's scope rather than the caller's: the screen is being left in the same frame + // this is called, so a scope tied to it would cancel the request that leaving is for. + viewModelScope.launch { + reporting.report(target, ReportDescription.build(reason, details)) + .onSuccess { + // The contract makes a duplicate report a no-op that answers OK, so this says + // the same thing whether or not the report was the first one. That is the + // intended reading: telling someone they had already reported this would be + // answering a question they did not ask. + BottomBarManager.showMessage( + title = resources.getString(R.string.prompt_title_reportSubmitted), + message = resources.getString(R.string.prompt_description_reportSubmitted), + ) + } + .onFailure { + trace("failed to report - ${it.localizedMessage}") + BottomBarManager.showError( + title = resources.getString(R.string.error_title_failedToReport), + message = resources.getString(R.string.error_description_failedToReport), + ) + } + } + } +} diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt new file mode 100644 index 0000000000..ab44e5e396 --- /dev/null +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt @@ -0,0 +1,194 @@ +package com.flipcash.app.messenger.internal.screens + +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.navigationBarsPadding +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.selection.selectable +import androidx.compose.foundation.selection.selectableGroup +import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.saveable.rememberSaveable +import androidx.compose.runtime.setValue +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.clip +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role +import androidx.compose.ui.text.style.TextAlign +import androidx.compose.ui.unit.sp +import com.flipcash.features.messenger.R +import com.flipcash.reporting.ReportReason +import com.getcode.theme.CodeTheme +import com.getcode.theme.White05 +import com.getcode.theme.extraSmall +import com.getcode.ui.components.AppBarWithTitle +import com.getcode.ui.core.verticalScrollStateGradient +import com.getcode.ui.theme.CodeButton +import com.getcode.ui.theme.CodeRadioButton +import com.getcode.ui.theme.CodeScaffold + +/** + * Why this is being reported: one row per [ReportReason], picked before it is sent. + * + * A radio group rather than six buttons that each file on touch. Filing a report is not undoable + * from here, and the labels are close enough to one another that a mis-tap is a plausible way to + * reach the wrong one; a selection that a second, deliberate press confirms costs one tap and + * removes that whole class of mistake. It also gives each row room for a line of scope underneath + * its label — see [descriptionRes] — which six tap-to-send rows have nowhere to put. + * + * The button says what it will do, because that differs by row: [ReportReason.Other] opens a + * second step and everything else submits outright. What the flow then does with the answer is + * still the flow's business — this screen reports a choice and nothing more, which is what keeps + * the rows uniform. + * + * The nav control is a ✕ in the leading slot, asked for rather than inferred. [AppBarWithTitle] + * can resolve ✕ against ← on its own, but only from the depth of the stack the screen sits in, and + * report is pushed over the chat it was opened from — so at this step, which is the flow's first + * and leaves it outright, that reading comes out a ←. Leading rather than trailing because the + * route is a fullscreen modal; the trailing ✕ belongs to the bottom sheets. + */ +@Composable +internal fun ReasonSelectionContent( + onChoose: (ReportReason) -> Unit, + onNavigateUp: () -> Unit, +) { + // Saved, so stepping forward to the details screen and back again doesn't lose the pick that + // got you there. + var selected by rememberSaveable { mutableStateOf(null) } + + CodeScaffold( + topBar = { + AppBarWithTitle( + title = stringResource(R.string.title_report), + titleAlignment = Alignment.CenterHorizontally, + onBackIconClicked = onNavigateUp, + leadingDismiss = true, + ) + }, + bottomBar = { + CodeButton( + modifier = Modifier + .fillMaxWidth() + .testTag("action_report_reason_continue") + .padding(horizontal = CodeTheme.dimens.inset) + .navigationBarsPadding() + .padding(bottom = CodeTheme.dimens.grid.x3), + text = if (selected == ReportReason.Other) { + stringResource(com.flipcash.core.R.string.action_next) + } else { + stringResource(R.string.action_submitReport) + }, + enabled = selected != null, + onClick = { selected?.let(onChoose) }, + ) + }, + ) { padding -> + val scrollState = rememberScrollState() + + Column( + modifier = Modifier + .padding(padding) + .verticalScroll(scrollState) + .verticalScrollStateGradient( + scrollState = scrollState, + color = CodeTheme.colors.background, + isLongGradient = true, + ) + .padding(horizontal = CodeTheme.dimens.inset) + .padding(top = CodeTheme.dimens.grid.x4), + ) { + Text( + modifier = Modifier.fillMaxWidth(), + text = stringResource(R.string.subtitle_report), + style = CodeTheme.typography.textSmall, + color = CodeTheme.colors.textSecondary, + textAlign = TextAlign.Center, + ) + + Column( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = CodeTheme.dimens.grid.x4) + .selectableGroup(), + verticalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x2), + ) { + for (reason in ReportReason.entries) { + ReportReasonRow( + modifier = Modifier.testTag("action_report_reason_${reason.name}"), + label = stringResource(reason.labelRes), + description = stringResource(reason.descriptionRes), + selected = selected == reason, + onSelect = { selected = reason }, + ) + } + } + } + } +} + +/** + * One choice: a radio, its label, and the line of scope under it. + * + * Built here rather than on [com.getcode.ui.components.ChoiceRow], which is a card with an icon + * and a single line and no notion of being selected. The card shape and [White05] fill are taken + * from it so the two read as the same family. + * + * The whole row is the target — [selectable] on the row, `null` on the radio — so the radio is a + * state indicator rather than a second, smaller thing to hit. + */ +@Composable +private fun ReportReasonRow( + label: String, + description: String, + selected: Boolean, + onSelect: () -> Unit, + modifier: Modifier = Modifier, +) { + Row( + modifier = modifier + .fillMaxWidth() + .clip(CodeTheme.shapes.extraSmall) + .background(White05) + .selectable( + selected = selected, + role = Role.RadioButton, + onClick = onSelect, + ) + .padding(CodeTheme.dimens.grid.x3), + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x2), + ) { + CodeRadioButton( + selected = selected, + // The row owns the click; a second target inside it would only be a smaller one. + onClick = null, + ) + + Column( + modifier = Modifier.weight(1f), + verticalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x1), + ) { + Text( + text = label, + // 17sp Demi, matching ChoiceRow and the settings rows rather than `textMedium`. + style = CodeTheme.typography.textMedium.copy(fontSize = 17.sp, lineHeight = 22.sp), + color = CodeTheme.colors.textMain, + ) + + Text( + text = description, + style = CodeTheme.typography.textSmall, + color = CodeTheme.colors.textSecondary, + ) + } + } +} diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt new file mode 100644 index 0000000000..fc3d4d49d3 --- /dev/null +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt @@ -0,0 +1,199 @@ +package com.flipcash.app.messenger.internal.screens + +import androidx.compose.animation.AnimatedVisibility +import androidx.compose.animation.animateColorAsState +import androidx.compose.animation.fadeIn +import androidx.compose.animation.fadeOut +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.imePadding +import androidx.compose.foundation.layout.navigationBarsPadding +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.text.KeyboardOptions +import androidx.compose.foundation.text.input.TextFieldState +import androidx.compose.foundation.verticalScroll +import androidx.compose.material3.Text +import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.focus.FocusRequester +import androidx.compose.ui.focus.focusRequester +import androidx.compose.ui.platform.testTag +import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.text.input.ImeAction +import androidx.compose.ui.text.input.KeyboardCapitalization +import androidx.compose.ui.text.input.KeyboardType +import androidx.compose.ui.unit.dp +import com.flipcash.app.core.ui.DisplayTextInput +import com.flipcash.features.messenger.R +import com.flipcash.reporting.ReportDescription +import com.getcode.theme.CodeTheme +import com.getcode.ui.components.AppBarWithTitle +import com.getcode.ui.components.text.AnimatedNumberText +import com.getcode.ui.core.verticalScrollStateGradient +import com.getcode.ui.theme.CodeButton +import com.getcode.ui.theme.CodeScaffold +import com.getcode.ui.utils.rememberKeyboardController + +/** + * The free-text step, reached only by picking [ReportReason.Other]. + * + * This is a step of its own rather than a second act inside the reason sheet, and the reason is + * load-bearing: a sheet that sizes itself to its content derives its drag anchors from the height + * that content reports, then reads those anchors again while placing it. Growing the content to + * hold a text field therefore changes the number the sheet was measured from, and the two chase + * each other off the frame clock until Android kills the app for not responding. A step rests at a + * detent decided before the content is measured, so nothing it holds can feed back into its size — + * which is also why the field can simply take the room left over and the keyboard inset can be + * honoured honestly, instead of both being approximated from a fraction of the screen. + * + * The heading is two blocks, not one sentence: the ask ("tell us what's wrong") is what the + * screen wants, and the privacy line is what it offers in return. Run together at one weight they + * compete, and the part that reassures is the part that gets skipped. + * + * The counter runs down rather than up, and the row carries the remaining count as its + * content description: [AnimatedNumberText] animates per digit, so a screen reader walking the + * children would otherwise read a number that is mid-flight and briefly wrong. + */ +@Composable +internal fun ReportDetailsContent( + state: TextFieldState, + onSubmit: (String) -> Unit, + onNavigateUp: () -> Unit, +) { + val keyboard = rememberKeyboardController() + val typed = state.text.toString() + val remaining = ReportDescription.MAX_DETAILS_LENGTH - typed.length + + CodeScaffold( + topBar = { + AppBarWithTitle( + title = stringResource(R.string.title_report), + titleAlignment = Alignment.CenterHorizontally, + onBackIconClicked = onNavigateUp, + ) + }, + bottomBar = { + Column( + modifier = Modifier + .fillMaxWidth() + .padding(horizontal = CodeTheme.dimens.inset) + .padding(top = CodeTheme.dimens.grid.x1), + verticalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x3), + ) { + val isOverLimit = remaining < 0 + val textColor by animateColorAsState( + if (isOverLimit) CodeTheme.colors.errorText else CodeTheme.colors.textSecondary + ) + + Row( + modifier = Modifier + .testTag("report_details_counter") + .semantics(mergeDescendants = true) { + contentDescription = remaining.toString() + }, + horizontalArrangement = Arrangement.spacedBy(4.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + AnimatedNumberText( + value = remaining.toString(), + style = CodeTheme.typography.textSmall, + color = textColor, + ) + + Text( + text = stringResource(com.flipcash.core.R.string.label_characters), + style = CodeTheme.typography.textSmall, + color = textColor, + ) + + AnimatedVisibility( + visible = typed.isNotEmpty(), + enter = fadeIn(), + exit = fadeOut(), + ) { + Text( + text = stringResource(com.flipcash.core.R.string.label_remaining), + style = CodeTheme.typography.textSmall, + color = textColor, + ) + } + } + + CodeButton( + modifier = Modifier + .fillMaxWidth() + .testTag("action_report_submit") + .navigationBarsPadding() + .padding(bottom = CodeTheme.dimens.grid.x3) + .imePadding(), + text = stringResource(R.string.action_submitReport), + enabled = typed.isNotBlank() && !isOverLimit, + onClick = { keyboard.hideIfVisible { onSubmit(typed.trim()) } }, + ) + } + }, + ) { padding -> + val focusRequester = remember { FocusRequester() } + val scrollState = rememberScrollState() + + Column( + modifier = Modifier + .padding(padding) + .verticalScroll(scrollState) + .verticalScrollStateGradient( + scrollState = scrollState, + color = CodeTheme.colors.background, + isLongGradient = true, + ) + .padding(horizontal = CodeTheme.dimens.inset) + .padding(top = CodeTheme.dimens.grid.x4), + verticalArrangement = Arrangement.spacedBy(CodeTheme.dimens.inset), + ) { + Column(verticalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x2)) { + Text( + modifier = Modifier.fillMaxWidth(0.80f), + text = stringResource(R.string.title_reportDetails), + style = CodeTheme.typography.textLarge, + color = CodeTheme.colors.textMain, + ) + + Text( + modifier = Modifier.fillMaxWidth(0.80f), + text = stringResource(R.string.subtitle_reportDetails), + style = CodeTheme.typography.textSmall, + color = CodeTheme.colors.textSecondary, + ) + } + + DisplayTextInput( + state = state, + placeholder = stringResource(R.string.hint_reportDetails), + modifier = Modifier + .fillMaxWidth() + .testTag("input_report_details") + .focusRequester(focusRequester), + keyboardOptions = KeyboardOptions( + capitalization = KeyboardCapitalization.Sentences, + keyboardType = KeyboardType.Text, + imeAction = ImeAction.Done, + ), + style = CodeTheme.typography.textMedium, + placeholderStyle = CodeTheme.typography.textMedium.copy( + color = CodeTheme.colors.textTertiary, + ), + maxLines = Int.MAX_VALUE, + ) + } + + LaunchedEffect(Unit) { focusRequester.requestFocus() } + } +} diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportReasonLabels.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportReasonLabels.kt index ea727c0ec9..0cfc572842 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportReasonLabels.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportReasonLabels.kt @@ -21,3 +21,24 @@ internal val ReportReason.labelRes: Int ReportReason.Violence -> R.string.action_reportReason_violence ReportReason.Other -> R.string.action_reportReason_other } + +/** + * What each reason covers, in one line under its label. + * + * Six words of label leave people guessing which bucket their situation belongs in, and a guess + * resolves as the nearest wrong one — which costs both the reporter and whoever reads it. The + * descriptions are scope, not policy: they say what the row collects, and promise nothing about + * what happens next. + * + * Exhaustive for the same reason [labelRes] is. + */ +@get:StringRes +internal val ReportReason.descriptionRes: Int + get() = when (this) { + ReportReason.Spam -> R.string.description_reportReason_spam + ReportReason.ScamOrFraud -> R.string.description_reportReason_scamOrFraud + ReportReason.Harassment -> R.string.description_reportReason_harassment + ReportReason.SexualContent -> R.string.description_reportReason_sexualContent + ReportReason.Violence -> R.string.description_reportReason_violence + ReportReason.Other -> R.string.description_reportReason_other + } diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt deleted file mode 100644 index ab2cf1d59c..0000000000 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheet.kt +++ /dev/null @@ -1,233 +0,0 @@ -package com.flipcash.app.messenger.internal.screens - -import androidx.activity.compose.BackHandler -import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.PaddingValues -import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height -import androidx.compose.foundation.layout.imePadding -import androidx.compose.foundation.layout.navigationBarsPadding -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.size -import androidx.compose.foundation.text.input.InputTransformation -import androidx.compose.foundation.text.input.rememberTextFieldState -import androidx.compose.material.icons.Icons -import androidx.compose.material.icons.outlined.CloseFullscreen -import androidx.compose.material.icons.outlined.Flag -import androidx.compose.material.icons.outlined.OpenInFull -import androidx.compose.material3.Icon -import androidx.compose.material3.IconButton -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 -import androidx.compose.ui.graphics.vector.rememberVectorPainter -import androidx.compose.ui.platform.testTag -import androidx.compose.ui.res.stringResource -import androidx.compose.ui.text.style.TextAlign -import androidx.compose.ui.unit.dp -import com.flipcash.features.messenger.R -import com.flipcash.reporting.ReportDescription -import com.flipcash.reporting.ReportReason -import com.getcode.theme.CodeTheme -import com.getcode.ui.components.AppBarDefaults -import com.getcode.ui.components.AppBarWithTitle -import com.getcode.ui.components.ChoiceRow -import com.getcode.ui.components.TextInput -import com.getcode.ui.theme.CodeButton - -/** - * Why this is being reported. - * - * One row per [ReportReason], drawn as [MuteChatSheet] draws its rows. Every row but the last is - * the whole interaction: the tap is the answer, so a report is two taps from anywhere it can be - * started. [ReportReason.Other] is the exception, and it is why this sheet has a second state at - * all — a list that cannot say anything outside its own six words is a list people work around by - * picking the nearest wrong one, which is worse than no taxonomy. - * - * The details field does not appear beside the rows, because a field standing open next to six - * one-tap choices reads as required. It replaces them, after the choice that needs it. - * - * Because it replaces them, the two states are one sheet that changes shape rather than two - * sheets: the bar and the subtitle are fixed, and only the region under them animates, so the - * height change reads as the same surface resizing. The leading back arrow and the rightward - * slide are the same fact told twice — "Something Else" is a step forward, and it is reversible. - * Going back keeps what was typed, since [details] is remembered above the swap; a breadcrumb - * that discarded the draft would be a trapdoor. - * - * What the sheet hands back is the reason and the raw details; assembling the two into the - * contract's single `description` string is [ReportDescription]'s job, in shared code, so that - * both platforms send the same bytes. Do not format anything here. - * - * @param onSubmit called once, with the chosen reason and the details the user typed, or `null` - * where the reason needed none. - */ -@Composable -internal fun ReportSheet( - onSubmit: (ReportReason, String?) -> Unit, - onDismiss: () -> Unit, -) { - var describing by remember { mutableStateOf(false) } - var expanded by remember { mutableStateOf(false) } - val details = rememberTextFieldState() - - // How tall the field gets when it is expanded, taken from the window rather than from the - // space left over after laying the sheet out. Measuring for it is what the obvious version of - // this does, and it deadlocks: a wrap-content sheet derives its drag anchors from the height - // its content reports, and the anchors are read again while that content is being placed, so - // a field whose height came from the measurement changes the number it was computed from. The - // loop never settles and the main thread spins until Android kills the app. A fraction of the - // screen is a constant across the whole layout pass, so it cannot feed back. The sheet's own - // detent caps the result, which is what makes this "as tall as the sheet allows" rather than - // an exact height this file would otherwise have to keep in step with the bar and the button. - val expandedFieldHeight = CodeTheme.dimens.screenHeight * 0.65f - - // Both of these are steps, so the system gesture has to agree with the controls on screen, and - // unwind them in the order they were entered. Without this, back would dismiss the whole sheet - // from a state the user can see a way out of. - BackHandler(enabled = describing) { - if (expanded) expanded = false else describing = false - } - - Column( - modifier = Modifier - .fillMaxWidth() - .navigationBarsPadding() - .imePadding(), - ) { - AppBarWithTitle( - title = { - AppBarDefaults.Title( - text = stringResource(R.string.title_report), - style = CodeTheme.typography.screenTitle, - ) - }, - titleAlignment = Alignment.CenterHorizontally, - // Built by hand rather than through the `onBackIconClicked` overload, which resolves - // back-vs-close from the ambient navigator. Here the answer is the sheet's own step, - // not its depth in whatever stack opened it: ✕ always closes the report, and ← only - // exists in the second step. - leftIcon = { - if (describing) { - AppBarDefaults.UpNavigation( - modifier = Modifier.testTag("action_report_back"), - onClick = { describing = false; expanded = false }, - ) - } - }, - rightContents = { AppBarDefaults.Close(onClick = onDismiss) }, - ) - - Text( - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = CodeTheme.dimens.inset), - text = stringResource(R.string.subtitle_report), - style = CodeTheme.typography.textSmall, - color = CodeTheme.colors.textSecondary, - textAlign = TextAlign.Center, - ) - - // The swap is instantaneous, and it has to be. This sheet sizes itself to its content: - // it writes the height it measured into its own state, and that write recomputes the drag - // detents. Content whose height is being animated therefore re-enters measurement on every - // pass, and because the loop runs off the layout pass rather than the frame clock, it - // starves the clock that would have finished the animation — the app spins at 100% and - // Android kills it. Any cross-fade, slide or size transition over these two steps brings - // that back, so the swap stays a plain `if`. - // - // The morph is still there; it is just the sheet's own. Swapping the content changes the - // measured height once, to a value that does not move, and the sheet animates its offset - // from the old detent to the new one. What the user sees is one surface changing size, - // which is what the two steps being one sheet was supposed to convey. - Column( - modifier = Modifier - .fillMaxWidth() - .padding(horizontal = CodeTheme.dimens.inset) - .padding(top = CodeTheme.dimens.grid.x4, bottom = CodeTheme.dimens.grid.x4), - verticalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x3), - ) { - if (describing) { - val typed = details.text.toString() - TextInput( - modifier = Modifier - .fillMaxWidth() - .testTag("input_report_details"), - state = details, - placeholder = stringResource(R.string.hint_reportDetails), - // Opens at the size of the answer it wants. One line invites one line, - // and "Something Else" is the row that exists because one line was not - // enough. It then grows with what is typed, up to a point past which a - // sheet this tall is worse than a scrollbar — that point is what the - // expand control is for. - minLines = 3, - maxLines = if (expanded) Int.MAX_VALUE else 8, - // Given to the box around the field rather than as `minHeight`, which - // sets a floor on the whole decorated row. The row centres that box in - // itself, so a floor there leaves the text and the placeholder floating in - // the middle of a screen-tall border; sizing the box makes the row wrap to - // it and the text start where the border does. - textModifier = if (expanded) Modifier.height(expandedFieldHeight) - else Modifier, - // Top-aligned so the placeholder sits where the first typed character - // will, rather than centred in a box three lines tall — and so the - // expand control rides the top edge instead of the vertical middle. - textFieldAlignment = Alignment.TopStart, - contentAlignment = Alignment.Top, - contentPadding = PaddingValues(CodeTheme.dimens.grid.x2), - trailingIcon = { - IconButton( - modifier = Modifier.testTag("action_report_expand"), - onClick = { expanded = !expanded }, - ) { - Icon( - imageVector = if (expanded) Icons.Outlined.CloseFullscreen - else Icons.Outlined.OpenInFull, - contentDescription = stringResource( - if (expanded) R.string.action_collapseReportDetails - else R.string.action_expandReportDetails - ), - tint = CodeTheme.colors.textSecondary, - modifier = Modifier.size(CodeTheme.dimens.staticGrid.x3), - ) - } - }, - // Clipped rather than rejected: the cap is the contract's, not a rule the user - // broke, and a field that stops accepting characters is clearer than an error - // about a limit nobody was told about. - inputTransformation = InputTransformation { - if (length > ReportDescription.MAX_DETAILS_LENGTH) revertAllChanges() - }, - ) - CodeButton( - modifier = Modifier - .fillMaxWidth() - .testTag("action_submit_report"), - text = stringResource(R.string.action_submitReport), - // Picking "Something Else" and then saying nothing leaves the report with less - // information than any other row would have carried, so the button waits. - enabled = typed.isNotBlank(), - onClick = { onSubmit(ReportReason.Other, typed.trim()) }, - ) - } else { - for (reason in ReportReason.entries) { - ChoiceRow( - modifier = Modifier.testTag("action_report_reason_${reason.name}"), - label = stringResource(reason.labelRes), - icon = rememberVectorPainter(Icons.Outlined.Flag), - onClick = { - if (reason == ReportReason.Other) describing = true - else onSubmit(reason, null) - }, - ) - } - } - } - - } -} diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt index 5f8fdefa72..3623de2d78 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt @@ -44,6 +44,7 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.DpOffset import androidx.compose.ui.unit.dp +import com.flipcash.app.core.AppRoute import com.flipcash.app.core.chat.ReportSubject import com.flipcash.app.messenger.internal.ChatSubject import com.flipcash.app.messenger.internal.ChatViewModel @@ -101,7 +102,23 @@ internal fun ChatTopBar( when (target) { TopBarMode.Conversation -> ConversationTitleBar(navigator, state, chatActionHandler) TopBarMode.Editing -> EditingBar(dispatch) - is TopBarMode.Selecting -> MessageSelectionBar(target.selection, keyboard, dispatch) + is TopBarMode.Selecting -> MessageSelectionBar( + selection = target.selection, + keyboard = keyboard, + dispatch = dispatch, + // Resolved here rather than in the bar: reporting is its own top-level + // flow now, so it needs the chat's id, and this is the nearest scope holding + // both that and the navigator. + onReport = { + state.chatId?.let { chatId -> + navigator.push( + AppRoute.Messaging.Report( + ReportSubject.Message(chatId, target.selection.messageId) + ) + ) + } + }, + ) } } } @@ -227,6 +244,7 @@ private fun MessageSelectionBar( selection: ChatListItem.ContentBubble, keyboard: KeyboardController, dispatch: (ChatViewModel.Event) -> Unit, + onReport: () -> Unit, ) { val capabilities = selection.capabilities val body = selection.plainText @@ -291,13 +309,7 @@ private fun MessageSelectionBar( label = stringResource(R.string.title_report), icon = Icons.Outlined.Flag, testTag = "action_report_message", - onClick = { - dispatch( - ChatViewModel.Event.OpenReportSheet( - ReportSubject.Message(selection.messageId) - ) - ) - }, + onClick = onReport, ) ) } diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ChatProfileScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ChatProfileScreen.kt index 3c9f45f93d..59bcf44b0a 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ChatProfileScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/ChatProfileScreen.kt @@ -22,6 +22,8 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.flipcash.app.core.chat.ChatParticipant import com.flipcash.app.core.chat.ChatStep import com.flipcash.app.core.chat.ReportSubject +import com.flipcash.app.core.AppRoute +import com.getcode.navigation.core.LocalCodeNavigator import com.flipcash.app.menu.MenuItem import com.flipcash.app.menu.MenuList import com.flipcash.app.messenger.internal.ChatMuteStatusChip @@ -61,6 +63,7 @@ internal fun ChatProfileScreen( chatViewModel: ChatViewModel, ) { val flowNavigator = rememberFlowNavigator() + val navigator = LocalCodeNavigator.current val state by viewModel.stateFlow.collectAsStateWithLifecycle() val chatState by chatViewModel.stateFlow.collectAsStateWithLifecycle() @@ -108,11 +111,17 @@ internal fun ChatProfileScreen( // Both muting and unmuting go through the picker, which is why this row // navigates either way rather than acting on one of them here. ChatProfileAction.Mute -> flowNavigator.navigateTo(ChatStep.MuteChat) - ChatProfileAction.Report -> state.participant?.let { participant -> - flowNavigator.navigateTo( - ChatStep.Report(ReportSubject.User(participant)) - ) - } + // Not flowNavigator: Report is a top-level route rather than a step of + // this flow, and LocalCodeNavigator hands a non-FlowStep route up to its + // parent. So it opens over the chat rather than inside it. + ChatProfileAction.Report -> + (state.participant as? ChatParticipant.TipUser)?.let { participant -> + navigator.push( + AppRoute.Messaging.Report( + ReportSubject.User(participant.userId) + ) + ) + } } }, endSlot = { item -> diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileScreen.kt index 2a9c912440..2f99654c8c 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/profile/GroupProfileScreen.kt @@ -20,6 +20,8 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.lifecycle.compose.collectAsStateWithLifecycle import com.flipcash.app.core.chat.ChatStep import com.flipcash.app.core.chat.ReportSubject +import com.flipcash.app.core.AppRoute +import com.getcode.navigation.core.LocalCodeNavigator import com.flipcash.app.menu.MenuItem import com.flipcash.app.menu.MenuList import com.flipcash.app.messenger.internal.ChatMuteStatusChip @@ -45,6 +47,7 @@ import kotlinx.coroutines.flow.filterIsInstance @Composable internal fun GroupProfileScreen(viewModel: ChatViewModel) { val flowNavigator = rememberFlowNavigator() + val navigator = LocalCodeNavigator.current val state by viewModel.stateFlow.collectAsStateWithLifecycle() val group = state.subject as? ChatSubject.Group @@ -108,8 +111,13 @@ internal fun GroupProfileScreen(viewModel: ChatViewModel) { GroupProfileAction.Mute -> flowNavigator.navigateTo(ChatStep.MuteChat) GroupProfileAction.Leave -> viewModel.dispatchEvent(ChatViewModel.Event.LeaveChat) - GroupProfileAction.Report -> - flowNavigator.navigateTo(ChatStep.Report(ReportSubject.Chat)) + // The outer navigator, not this flow's: reporting is its own top-level + // route, so it opens over the chat rather than inside it. + GroupProfileAction.Report -> state.chatId?.let { chatId -> + navigator.push( + AppRoute.Messaging.Report(ReportSubject.Chat(chatId)) + ) + } } }, ) diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt new file mode 100644 index 0000000000..30685ed1be --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt @@ -0,0 +1,240 @@ +package com.flipcash.app.messenger.internal.screens + +import androidx.activity.ComponentActivity +import androidx.compose.foundation.text.input.TextFieldState +import androidx.compose.ui.test.SemanticsMatcher +import androidx.compose.ui.test.assertContentDescriptionEquals +import androidx.compose.ui.test.assertCountEquals +import androidx.compose.ui.test.assertIsEnabled +import androidx.compose.ui.test.assertIsNotEnabled +import androidx.compose.ui.test.assertTextEquals +import androidx.compose.ui.test.filter +import androidx.compose.ui.test.filterToOne +import androidx.compose.ui.test.hasSetTextAction +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithTag +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performTextInput +import com.flipcash.features.messenger.R +import com.flipcash.reporting.ReportDescription +import com.flipcash.reporting.ReportReason +import com.flipcash.app.theme.FlipcashPreview +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Assert.assertNull +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], qualifiers = "w400dp-h800dp-xhdpi") +class ReportFlowTest { + + @get:Rule + val composeTestRule = createAndroidComposeRule() + + private var chosen: ReportReason? = null + private var submitted: String? = null + private var dismissed = false + + private fun showReasons() { + composeTestRule.setContent { + FlipcashPreview { + ReasonSelectionContent( + onChoose = { chosen = it }, + onNavigateUp = { dismissed = true }, + ) + } + } + } + + private fun showDetails(state: TextFieldState = TextFieldState()) { + composeTestRule.setContent { + FlipcashPreview { + ReportDetailsContent( + state = state, + onSubmit = { submitted = it }, + onNavigateUp = { dismissed = true }, + ) + } + } + } + + // `TopAppBarBase` subcomposes the leading slot more than once: once to measure and place it, + // once as a probe to tell a real control from an empty slot, and — for a centred title — a + // phantom back arrow that only reserves width. Every copy carries the icon's test tag, so the + // tag alone says nothing about what is on screen. Only the real control is ever placed. + private val placed = SemanticsMatcher("is placed") { it.layoutInfo.isPlaced } + + private fun leadingControl(tag: String) = + composeTestRule.onAllNodesWithTag(tag).filterToOne(placed) + + private fun assertNoLeadingControl(tag: String) = + composeTestRule.onAllNodesWithTag(tag).filter(placed).assertCountEquals(0) + + @Test + fun `offers every reason in the shared vocabulary`() { + showReasons() + + for (reason in ReportReason.entries) { + composeTestRule.onNodeWithTag("action_report_reason_${reason.name}").assertExists() + } + } + + @Test + fun `says what each reason covers`() { + showReasons() + + for (reason in ReportReason.entries) { + composeTestRule + .onNodeWithText(composeTestRule.activity.getString(reason.descriptionRes)) + .assertExists() + } + } + + @Test + fun `picking a row is not yet an answer`() { + showReasons() + + composeTestRule.onNodeWithTag("action_report_reason_${ReportReason.Harassment.name}") + .performClick() + + assertNull(chosen) + } + + @Test + fun `will not continue before a reason is picked`() { + showReasons() + + composeTestRule.onNodeWithTag("action_report_reason_continue").assertIsNotEnabled() + } + + @Test + fun `answers with the reason that was picked`() { + showReasons() + + composeTestRule.onNodeWithTag("action_report_reason_${ReportReason.Harassment.name}") + .performClick() + composeTestRule.onNodeWithTag("action_report_reason_continue").performClick() + + assertEquals(ReportReason.Harassment, chosen) + } + + @Test + fun `answers with the last row picked, not the first`() { + showReasons() + + composeTestRule.onNodeWithTag("action_report_reason_${ReportReason.Spam.name}") + .performClick() + composeTestRule.onNodeWithTag("action_report_reason_${ReportReason.Violence.name}") + .performClick() + composeTestRule.onNodeWithTag("action_report_reason_continue").performClick() + + assertEquals(ReportReason.Violence, chosen) + } + + @Test + fun `answers with Other rather than deciding what it costs`() { + showReasons() + + composeTestRule.onNodeWithTag("action_report_reason_${ReportReason.Other.name}") + .performClick() + composeTestRule.onNodeWithTag("action_report_reason_continue").performClick() + + assertEquals(ReportReason.Other, chosen) + } + + @Test + fun `calls the button what it will do`() { + showReasons() + + composeTestRule.onNodeWithTag("action_report_reason_${ReportReason.Spam.name}") + .performClick() + composeTestRule.onNodeWithTag("action_report_reason_continue") + .assertTextEquals( + composeTestRule.activity.getString(R.string.action_submitReport) + ) + + composeTestRule.onNodeWithTag("action_report_reason_${ReportReason.Other.name}") + .performClick() + composeTestRule.onNodeWithTag("action_report_reason_continue") + .assertTextEquals( + composeTestRule.activity.getString(com.flipcash.core.R.string.action_next) + ) + } + + @Test + fun `will not submit an empty description`() { + showDetails() + + composeTestRule.onNodeWithTag("action_report_submit").assertIsNotEnabled() + } + + @Test + fun `submits the description without its surrounding whitespace`() { + showDetails() + + composeTestRule.onNode(hasSetTextAction()) + .performTextInput(" they kept messaging me ") + composeTestRule.onNodeWithTag("action_report_submit").performClick() + + assertEquals("they kept messaging me", submitted) + } + + @Test + fun `counts down the room left in the description`() { + showDetails() + + composeTestRule.onNode(hasSetTextAction()).performTextInput("hello") + + composeTestRule.onNodeWithTag("report_details_counter") + .assertContentDescriptionEquals("${ReportDescription.MAX_DETAILS_LENGTH - 5}") + } + + @Test + fun `refuses a description that is over the limit`() { + showDetails(TextFieldState("x".repeat(ReportDescription.MAX_DETAILS_LENGTH + 1))) + + composeTestRule.onNodeWithTag("action_report_submit").assertIsNotEnabled() + composeTestRule.onNodeWithTag("action_report_submit").performClick() + assertNull(submitted) + } + + @Test + fun `submits once there is something to send`() { + showDetails() + + composeTestRule.onNode(hasSetTextAction()).performTextInput("spamming links") + + composeTestRule.onNodeWithTag("action_report_submit").assertIsEnabled() + } + + @Test + fun `leaves the first step by closing, not by going back`() { + showReasons() + + leadingControl("action_close").assertExists() + assertNoLeadingControl("action_back") + } + + @Test + fun `closing the first step asks to leave the flow`() { + showReasons() + + leadingControl("action_close").performClick() + + assertTrue(dismissed) + } + + @Test + fun `leaves the details step by going back, not by closing`() { + showDetails() + + leadingControl("action_back").assertExists() + assertNoLeadingControl("action_close") + } +} diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt deleted file mode 100644 index 400f4f2190..0000000000 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportSheetTest.kt +++ /dev/null @@ -1,192 +0,0 @@ -package com.flipcash.app.messenger.internal.screens - -import androidx.activity.ComponentActivity -import androidx.compose.ui.test.assertIsDisplayed -import androidx.compose.ui.test.assertIsNotEnabled -import androidx.compose.ui.test.assertCountEquals -import androidx.compose.ui.test.hasSetTextAction -import androidx.compose.ui.test.junit4.AndroidComposeTestRule -import androidx.compose.ui.test.junit4.createAndroidComposeRule -import androidx.compose.ui.test.onAllNodesWithTag -import androidx.compose.ui.test.onFirst -import androidx.compose.ui.test.onNodeWithContentDescription -import androidx.compose.ui.test.onNodeWithTag -import androidx.compose.ui.test.performClick -import androidx.compose.ui.test.performTextInput -import com.flipcash.app.theme.FlipcashPreview -import com.flipcash.features.messenger.R -import com.flipcash.reporting.ReportReason -import org.junit.Rule -import org.junit.Test -import org.junit.runner.RunWith -import org.robolectric.RobolectricTestRunner -import org.robolectric.annotation.Config -import kotlin.test.assertEquals -import kotlin.test.assertNull - -/** - * Every row but one is the whole interaction: the tap is the answer. [ReportReason.Other] is the - * exception, and the cases below are what that exception costs — it reveals instead of - * submitting, it carries what was typed, it will not submit nothing, and, because it is the only - * row that goes somewhere, it is the only one that has to offer a way back. - */ -@RunWith(RobolectricTestRunner::class) -@Config(sdk = [34], qualifiers = "w400dp-h800dp-xhdpi") -class ReportSheetTest { - - @get:Rule - val composeTestRule = createAndroidComposeRule() - - private var submitted: Pair? = null - - /** - * The editable node itself, reached by its set-text action rather than by tag: `TextInput` - * puts the caller's modifier on the box it draws around the field, and hands the field a - * modifier of its own making, so the focus and text semantics are on a node no tag can name. - * The sheet has one field, so the action identifies it. - */ - private fun AndroidComposeTestRule<*, *>.detailsField() = onNode(hasSetTextAction()) - - /** - * The back arrow, of which the semantics tree holds two: [TopAppBarBase] subcomposes its - * leading slot a second time as a `leftIcon_probe`, purely to measure whether a real control - * is there, and that copy is never placed. The placed one is subcomposed first. - */ - private fun AndroidComposeTestRule<*, *>.backArrow() = - onAllNodesWithTag("action_report_back").onFirst() - - private fun showSheet() { - composeTestRule.setContent { - FlipcashPreview { - ReportSheet( - onSubmit = { reason, details -> submitted = reason to details }, - onDismiss = {}, - ) - } - } - } - - @Test - fun `a plain reason submits on the tap, with no details`() { - showSheet() - - composeTestRule.onNodeWithTag("action_report_reason_Spam").performClick() - - assertEquals(ReportReason.Spam, submitted?.first) - assertNull(submitted?.second) - } - - @Test - fun `Other reveals the field instead of submitting`() { - showSheet() - - composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() - - assertNull(submitted) - composeTestRule.detailsField().assertIsDisplayed() - } - - @Test - fun `Other submits the typed details`() { - showSheet() - - composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() - composeTestRule.detailsField().performTextInput("they keep doing the thing") - composeTestRule.onNodeWithTag("action_submit_report").performClick() - - assertEquals(ReportReason.Other, submitted?.first) - assertEquals("they keep doing the thing", submitted?.second) - } - - @Test - fun `Other will not submit an empty field`() { - showSheet() - - composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() - composeTestRule.onNodeWithTag("action_submit_report").assertIsNotEnabled() - composeTestRule.onNodeWithTag("action_submit_report").performClick() - - assertNull(submitted) - } - - @Test - fun `the reason list has nothing to go back to`() { - showSheet() - - composeTestRule.onAllNodesWithTag("action_report_back").assertCountEquals(0) - } - - @Test - fun `the details step goes back to the reasons`() { - showSheet() - - composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() - composeTestRule.backArrow().performClick() - - composeTestRule.onNodeWithTag("action_report_reason_Spam").assertIsDisplayed() - assertNull(submitted) - } - - @Test - fun `going back keeps what was typed`() { - showSheet() - - composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() - composeTestRule.detailsField().performTextInput("half a thought") - composeTestRule.backArrow().performClick() - composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() - - composeTestRule.onNodeWithTag("action_submit_report").performClick() - assertEquals("half a thought", submitted?.second) - } - - private val expandLabel: String - get() = composeTestRule.activity.getString(R.string.action_expandReportDetails) - - private val collapseLabel: String - get() = composeTestRule.activity.getString(R.string.action_collapseReportDetails) - - private fun pressBack() = composeTestRule.runOnUiThread { - composeTestRule.activity.onBackPressedDispatcher.onBackPressed() - } - - @Test - fun `the field offers a way to go full screen`() { - showSheet() - - composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() - - composeTestRule.onNodeWithContentDescription(expandLabel).assertIsDisplayed() - } - - @Test - fun `one control both expands and collapses`() { - showSheet() - - composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() - composeTestRule.onNodeWithTag("action_report_expand").performClick() - composeTestRule.onNodeWithContentDescription(collapseLabel).assertIsDisplayed() - - composeTestRule.onNodeWithTag("action_report_expand").performClick() - composeTestRule.onNodeWithContentDescription(expandLabel).assertIsDisplayed() - } - - /** - * Expanding is a step of its own, so back unwinds it before the step that contains it. A back - * that jumped straight to the reasons would throw away a draft the user was mid-sentence in. - */ - @Test - fun `back leaves full screen before it leaves the details step`() { - showSheet() - - composeTestRule.onNodeWithTag("action_report_reason_Other").performClick() - composeTestRule.detailsField().performTextInput("still typing") - composeTestRule.onNodeWithTag("action_report_expand").performClick() - - pressBack() - composeTestRule.onNodeWithContentDescription(expandLabel).assertIsDisplayed() - - pressBack() - composeTestRule.onNodeWithTag("action_report_reason_Spam").assertIsDisplayed() - } -} From 2c81f8cc0ef117f0149169551b035ad08ec4b25b Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 00:35:40 -0400 Subject: [PATCH 06/14] fix(reporting): move the report close to the trailing edge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first step's dismiss was forced into the leading slot with leadingDismiss, which exists for a screen that is a modal dismiss rather than a flow root — currency-info is its one other caller. Report's first step is a flow root, and the app puts that dismiss on the trailing edge. Declaring the step's own FlowDismissStyle.Close is what AppBarWithTitle already reads to place it there. FlowHost cannot work it out here: it decides from the outer stack depth, and report is pushed over the chat rather than opened as the sheet's first entry. The declaration sits in the screen rather than the flow host so that it holds wherever the step is rendered, including under test. The new case pins the edge, which the existing ones could not: they assert which glyph shows, and both glyphs sit in the same slot. --- .../internal/screens/ReasonSelectionScreen.kt | 127 ++++++++++-------- .../internal/screens/ReportFlowTest.kt | 31 ++++- 2 files changed, 92 insertions(+), 66 deletions(-) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt index ab44e5e396..eeecae35a2 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt @@ -13,6 +13,7 @@ import androidx.compose.foundation.selection.selectableGroup import androidx.compose.foundation.verticalScroll import androidx.compose.material3.Text import androidx.compose.runtime.Composable +import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.saveable.rememberSaveable @@ -27,6 +28,8 @@ import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.sp import com.flipcash.features.messenger.R import com.flipcash.reporting.ReportReason +import com.getcode.navigation.flow.FlowDismissStyle +import com.getcode.navigation.flow.LocalFlowDismissStyle import com.getcode.theme.CodeTheme import com.getcode.theme.White05 import com.getcode.theme.extraSmall @@ -65,70 +68,76 @@ internal fun ReasonSelectionContent( // got you there. var selected by rememberSaveable { mutableStateOf(null) } - CodeScaffold( - topBar = { - AppBarWithTitle( - title = stringResource(R.string.title_report), - titleAlignment = Alignment.CenterHorizontally, - onBackIconClicked = onNavigateUp, - leadingDismiss = true, - ) - }, - bottomBar = { - CodeButton( - modifier = Modifier - .fillMaxWidth() - .testTag("action_report_reason_continue") - .padding(horizontal = CodeTheme.dimens.inset) - .navigationBarsPadding() - .padding(bottom = CodeTheme.dimens.grid.x3), - text = if (selected == ReportReason.Other) { - stringResource(com.flipcash.core.R.string.action_next) - } else { - stringResource(R.string.action_submitReport) - }, - enabled = selected != null, - onClick = { selected?.let(onChoose) }, - ) - }, - ) { padding -> - val scrollState = rememberScrollState() - - Column( - modifier = Modifier - .padding(padding) - .verticalScroll(scrollState) - .verticalScrollStateGradient( - scrollState = scrollState, - color = CodeTheme.colors.background, - isLongGradient = true, + // This screen is always the first step of the report flow, so its nav control leaves the flow + // rather than stepping back within it. AppBarWithTitle cannot infer that: it decides from the + // outer stack depth, and report is pushed over the chat rather than opened as the sheet's + // first entry. Declaring it here puts the close on the trailing edge, where the app's other + // dismiss controls sit. + CompositionLocalProvider(LocalFlowDismissStyle provides FlowDismissStyle.Close) { + CodeScaffold( + topBar = { + AppBarWithTitle( + title = stringResource(R.string.title_report), + titleAlignment = Alignment.CenterHorizontally, + onBackIconClicked = onNavigateUp, ) - .padding(horizontal = CodeTheme.dimens.inset) - .padding(top = CodeTheme.dimens.grid.x4), - ) { - Text( - modifier = Modifier.fillMaxWidth(), - text = stringResource(R.string.subtitle_report), - style = CodeTheme.typography.textSmall, - color = CodeTheme.colors.textSecondary, - textAlign = TextAlign.Center, - ) + }, + bottomBar = { + CodeButton( + modifier = Modifier + .fillMaxWidth() + .testTag("action_report_reason_continue") + .padding(horizontal = CodeTheme.dimens.inset) + .navigationBarsPadding() + .padding(bottom = CodeTheme.dimens.grid.x3), + text = if (selected == ReportReason.Other) { + stringResource(com.flipcash.core.R.string.action_next) + } else { + stringResource(R.string.action_submitReport) + }, + enabled = selected != null, + onClick = { selected?.let(onChoose) }, + ) + }, + ) { padding -> + val scrollState = rememberScrollState() Column( modifier = Modifier - .fillMaxWidth() - .padding(vertical = CodeTheme.dimens.grid.x4) - .selectableGroup(), - verticalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x2), - ) { - for (reason in ReportReason.entries) { - ReportReasonRow( - modifier = Modifier.testTag("action_report_reason_${reason.name}"), - label = stringResource(reason.labelRes), - description = stringResource(reason.descriptionRes), - selected = selected == reason, - onSelect = { selected = reason }, + .padding(padding) + .verticalScroll(scrollState) + .verticalScrollStateGradient( + scrollState = scrollState, + color = CodeTheme.colors.background, + isLongGradient = true, ) + .padding(horizontal = CodeTheme.dimens.inset) + .padding(top = CodeTheme.dimens.grid.x4), + ) { + Text( + modifier = Modifier.fillMaxWidth(), + text = stringResource(R.string.subtitle_report), + style = CodeTheme.typography.textSmall, + color = CodeTheme.colors.textSecondary, + textAlign = TextAlign.Center, + ) + + Column( + modifier = Modifier + .fillMaxWidth() + .padding(vertical = CodeTheme.dimens.grid.x4) + .selectableGroup(), + verticalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x2), + ) { + for (reason in ReportReason.entries) { + ReportReasonRow( + modifier = Modifier.testTag("action_report_reason_${reason.name}"), + label = stringResource(reason.labelRes), + description = stringResource(reason.descriptionRes), + selected = selected == reason, + onSelect = { selected = reason }, + ) + } } } } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt index 30685ed1be..d62934bd58 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt @@ -14,7 +14,9 @@ import androidx.compose.ui.test.hasSetTextAction import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.onAllNodesWithTag import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.getUnclippedBoundsInRoot import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.onRoot import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performTextInput import com.flipcash.features.messenger.R @@ -70,10 +72,10 @@ class ReportFlowTest { // tag alone says nothing about what is on screen. Only the real control is ever placed. private val placed = SemanticsMatcher("is placed") { it.layoutInfo.isPlaced } - private fun leadingControl(tag: String) = + private fun navControl(tag: String) = composeTestRule.onAllNodesWithTag(tag).filterToOne(placed) - private fun assertNoLeadingControl(tag: String) = + private fun assertNoNavControl(tag: String) = composeTestRule.onAllNodesWithTag(tag).filter(placed).assertCountEquals(0) @Test @@ -217,15 +219,30 @@ class ReportFlowTest { fun `leaves the first step by closing, not by going back`() { showReasons() - leadingControl("action_close").assertExists() - assertNoLeadingControl("action_back") + navControl("action_close").assertExists() + assertNoNavControl("action_back") + } + + @Test + fun `the close sits on the trailing edge`() { + showReasons() + val bar = composeTestRule.onRoot().getUnclippedBoundsInRoot() + val close = navControl("action_close").getUnclippedBoundsInRoot() + // Trailing, not leading: the close dismisses a layer, and this app puts that control at + // the far edge (see the sheet-root close in AppBarWithTitle) rather than where a back + // arrow would sit. + val midpoint = (bar.left + bar.right) / 2 + assertTrue( + "close starts at ${close.left}, expected past the midpoint $midpoint", + close.left > midpoint, + ) } @Test fun `closing the first step asks to leave the flow`() { showReasons() - leadingControl("action_close").performClick() + navControl("action_close").performClick() assertTrue(dismissed) } @@ -234,7 +251,7 @@ class ReportFlowTest { fun `leaves the details step by going back, not by closing`() { showDetails() - leadingControl("action_back").assertExists() - assertNoLeadingControl("action_close") + navControl("action_back").assertExists() + assertNoNavControl("action_close") } } From 7a37241de9da01e21d736e7c9966f3c3acb00239 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 08:14:27 -0400 Subject: [PATCH 07/14] feat(reporting): confirm the report before the flow closes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Submitting closed the flow in the same frame it called `submit`, so the confirmation had nowhere to land. Two things followed from that: a success prompt raised from a sheet only appeared once the sheet was gone, on whatever screen happened to be underneath, and a failure said nothing at all — the flow was already dismissed, taking the half-filled report with it. The flow now stays up until the report is acknowledged. `ReportViewModel` exposes `confirmed`, emitted when the success prompt is dismissed, and each step exits on it; a failure raises an error prompt and leaves the flow where it is, with the reason still picked, so the retry is one press. `submitting` drives the button's loading state for the wait that closing used to hide, and guards against a second press filing the same report twice. `showSuccess` replaces `showMessage` so the prompt carries an OK to dismiss — without one there is nothing to wait for. For the prompt to be seen at all, `NavMessagingEntryDecorator` had to stop refusing to host bars for every sheet entry. A sheet that covers the screen now hosts its own, as a route does; a partial sheet still defers to the route beneath it, where a bar would otherwise be clipped to the sheet or hidden behind it. That predicate is extracted as `hostsBars` and covered by `BarHostingTest`. --- .../decorators/NavMessagingEntryDecorator.kt | 21 +++- .../navigation/decorators/BarHostingTest.kt | 37 ++++++ .../app/messenger/ReportFlowScreen.kt | 39 +++++- .../app/messenger/internal/ReportViewModel.kt | 48 ++++++-- .../internal/screens/ReasonSelectionScreen.kt | 4 + .../internal/screens/ReportDetailsScreen.kt | 3 + .../messenger/internal/ReportViewModelTest.kt | 114 ++++++++++++++++++ 7 files changed, 251 insertions(+), 15 deletions(-) create mode 100644 apps/flipcash/app/src/test/kotlin/com/flipcash/app/internal/ui/navigation/decorators/BarHostingTest.kt create mode 100644 apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt diff --git a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/decorators/NavMessagingEntryDecorator.kt b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/decorators/NavMessagingEntryDecorator.kt index aac3befcfc..5329c1d339 100644 --- a/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/decorators/NavMessagingEntryDecorator.kt +++ b/apps/flipcash/app/src/main/kotlin/com/flipcash/app/internal/ui/navigation/decorators/NavMessagingEntryDecorator.kt @@ -25,8 +25,12 @@ fun NavMessagingEntryDecorator( val analytics = rememberAnalytics() Box { entry.Content() - val isTopEntry = entry.contentKey == backStack.lastOrNull()?.toString() - if (isTopEntry && entry.metadata[NavMetadataKeys.IsSheet.key] != true) { + val hosts = hostsBars( + isTopEntry = entry.contentKey == backStack.lastOrNull()?.toString(), + isSheet = entry.metadata[NavMetadataKeys.IsSheet.key] == true, + coversScreen = entry.metadata[NavMetadataKeys.IsFullscreenSheet.key] == true, + ) + if (hosts) { BottomBarContainer(barManager.barMessages) { message -> trace( message = "bottom bar message shown [${message.type.name}]", @@ -71,3 +75,16 @@ private fun NavKey.screenName(): String? { val name = this::class.java.name return name.removePrefix("$prefix$").replace('$', '.').takeIf { it != name } } + +/** + * Whether an entry is the one that puts the app's bars on screen. + * + * A sheet that covers the screen carries its own bars, exactly as a route does: the bar has the + * whole window to sit in, and its scrim has nothing left to leave showing. A partial sheet has + * neither — a bar inside one is clipped to the sheet, and one behind it is covered — so a partial + * sheet leaves bars to the route beneath it. That is why a bar raised from inside a partial sheet + * is only seen once the sheet has closed, and why a flow that wants to be acknowledged before it + * closes has to cover the screen. + */ +internal fun hostsBars(isTopEntry: Boolean, isSheet: Boolean, coversScreen: Boolean): Boolean = + isTopEntry && (!isSheet || coversScreen) diff --git a/apps/flipcash/app/src/test/kotlin/com/flipcash/app/internal/ui/navigation/decorators/BarHostingTest.kt b/apps/flipcash/app/src/test/kotlin/com/flipcash/app/internal/ui/navigation/decorators/BarHostingTest.kt new file mode 100644 index 0000000000..9db3248dd3 --- /dev/null +++ b/apps/flipcash/app/src/test/kotlin/com/flipcash/app/internal/ui/navigation/decorators/BarHostingTest.kt @@ -0,0 +1,37 @@ +package com.flipcash.app.internal.ui.navigation.decorators + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Which entry puts the app's bars on screen. + * + * The rule is easy to get wrong in the direction that costs a whole feature: get it wrong for a + * sheet and a bar raised from inside that sheet is never seen, because it renders behind the sheet + * or is clipped to it. Report asks for a confirmation before it closes, so it needs the answer to + * be yes while it is still open. + */ +class BarHostingTest { + + @Test + fun `a route hosts its own bars`() { + assertTrue(hostsBars(isTopEntry = true, isSheet = false, coversScreen = false)) + } + + @Test + fun `a sheet that covers the screen hosts its own bars`() { + assertTrue(hostsBars(isTopEntry = true, isSheet = true, coversScreen = true)) + } + + @Test + fun `a partial sheet leaves bars to the route beneath it`() { + assertFalse(hostsBars(isTopEntry = true, isSheet = true, coversScreen = false)) + } + + @Test + fun `an entry that is not on top never hosts bars`() { + assertFalse(hostsBars(isTopEntry = false, isSheet = false, coversScreen = false)) + assertFalse(hostsBars(isTopEntry = false, isSheet = true, coversScreen = true)) + } +} diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt index ce5d5f6b9d..e488a3ec51 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt @@ -3,6 +3,9 @@ package com.flipcash.app.messenger import android.os.Parcelable import androidx.compose.foundation.text.input.rememberTextFieldState import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation3.runtime.NavEntry import androidx.navigation3.runtime.NavKey import androidx.navigation3.runtime.entryProvider @@ -17,6 +20,7 @@ import com.flipcash.reporting.ReportReason import com.getcode.navigation.annotatedEntry import com.getcode.navigation.core.LocalCodeNavigator import com.getcode.navigation.flow.FlowHost +import com.getcode.navigation.flow.FlowNavigator import com.getcode.navigation.flow.flowSharedViewModel import com.getcode.navigation.flow.rememberFlowNavigator import com.getcode.navigation.flow.rememberInitialStack @@ -71,8 +75,12 @@ private fun reportEntryProvider( private fun FlowReasonSelection(subject: ReportSubject) { val viewModel = flowSharedViewModel() val flowNavigator = rememberFlowNavigator() + val submitting by viewModel.submitting.collectAsStateWithLifecycle() + + ExitOnceConfirmed(viewModel, flowNavigator) ReasonSelectionContent( + submitting = submitting, onChoose = { reason -> if (reason == ReportReason.Other) { flowNavigator.navigateTo(ReportStep.Details(reason)) @@ -80,7 +88,6 @@ private fun FlowReasonSelection(subject: ReportSubject) { // Every other reason is the whole report, so the pick is the submit. Asking for // more after someone has already said what is wrong would be asking twice. viewModel.submit(subject, reason, details = null) - flowNavigator.exitCanceled() } }, // `back` on its own: at the flow's root it exits the flow itself, so pairing it with an @@ -97,14 +104,36 @@ private fun FlowReportDetails(subject: ReportSubject, reason: ReportReason) { // typed — leaving by the back arrow is usually a second thought about the reason, not about // the words. val details = rememberTextFieldState() + val submitting by viewModel.submitting.collectAsStateWithLifecycle() + + ExitOnceConfirmed(viewModel, flowNavigator) ReportDetailsContent( state = details, - onSubmit = { typed -> - viewModel.submit(subject, reason, typed) - flowNavigator.exitCanceled() - }, + submitting = submitting, + onSubmit = { typed -> viewModel.submit(subject, reason, typed) }, // Not the flow's root, so this steps back to the reasons rather than leaving. onNavigateUp = { flowNavigator.back() }, ) } + +/** + * Closes the flow once the report has been acknowledged. + * + * What closes the flow is the confirmation being dismissed, not the submit being called. Reporting + * someone is not a throwaway action, and the flow used to vanish the instant the button was + * pressed, which left the confirmation to land on whatever happened to be behind it and said + * nothing at all when the send had failed. + * + * Collected per step rather than once around the host, because [FlowNavigator] is only in scope + * inside one — and the step that submitted is the one still composed while its confirmation is up. + */ +@Composable +private fun ExitOnceConfirmed( + viewModel: ReportViewModel, + flowNavigator: FlowNavigator, +) { + LaunchedEffect(viewModel) { + viewModel.confirmed.collect { flowNavigator.exitCanceled() } + } +} diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt index a17fb9a959..9096d93eac 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt @@ -12,6 +12,12 @@ import com.getcode.manager.BottomBarManager import com.getcode.util.resources.ResourceHelper import com.getcode.utils.trace import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch import javax.inject.Inject @@ -19,12 +25,14 @@ import javax.inject.Inject * Filing the report the flow has collected. * * Scoped to the flow rather than to either step, because the reason is chosen on one screen and — - * for [ReportReason.Other] — described on the next, and the submit can outlive the step that asked - * for it. Previously this lived in the chat's view model, which tied a report to the conversation - * it happened to be opened from and kept a half-written one in chat state. + * for [ReportReason.Other] — described on the next, and the submit outlives the step that asked for + * it. Previously this lived in the chat's view model, which tied a report to the conversation it + * happened to be opened from and kept a half-written one in chat state. * - * No state of its own: both screens own their input, and the outcome is a bottom bar prompt rather - * than something the flow renders, so there is nothing to hold across the exit. + * The flow stays up until the report has been acknowledged. It used to close in the same frame the + * submit was called, which left the confirmation to land on whatever was behind it — telling + * someone their report was sent on a screen that had nothing to do with reporting, and saying + * nothing at all if the send failed. */ @HiltViewModel internal class ReportViewModel @Inject constructor( @@ -32,8 +40,29 @@ internal class ReportViewModel @Inject constructor( private val resources: ResourceHelper, ) : ViewModel() { + private val _submitting = MutableStateFlow(false) + + /** True while the report is in flight, so the button can say so rather than look ignored. */ + val submitting: StateFlow = _submitting.asStateFlow() + + // Buffered rather than a shared flow: this fires once, from a bottom bar callback that can + // outlive the composition collecting it, and losing it would strand the flow open. + private val _confirmed = Channel(Channel.BUFFERED) + + /** + * Emits once the report has been acknowledged, which is what closes the flow. + * + * Dismissing the confirmation, not the send returning: the acknowledgement is the point of + * staying, so leaving before it is read would waste it. + */ + val confirmed: Flow = _confirmed.receiveAsFlow() + /** [details] is the reporter's own words, and only [ReportReason.Other] collects them. */ fun submit(subject: ReportSubject, reason: ReportReason, details: String?) { + // A second press while the first is in flight would file the same report twice and answer + // with two confirmations stacked on each other. + if (_submitting.value) return + val target = when (subject) { is ReportSubject.User -> ReportTarget.User(subject.userId) is ReportSubject.Chat -> ReportTarget.Chat(subject.chatId) @@ -43,27 +72,30 @@ internal class ReportViewModel @Inject constructor( ) } - // On the flow's scope rather than the caller's: the screen is being left in the same frame - // this is called, so a scope tied to it would cancel the request that leaving is for. viewModelScope.launch { + _submitting.value = true reporting.report(target, ReportDescription.build(reason, details)) .onSuccess { // The contract makes a duplicate report a no-op that answers OK, so this says // the same thing whether or not the report was the first one. That is the // intended reading: telling someone they had already reported this would be // answering a question they did not ask. - BottomBarManager.showMessage( + BottomBarManager.showSuccess( title = resources.getString(R.string.prompt_title_reportSubmitted), message = resources.getString(R.string.prompt_description_reportSubmitted), + onDismiss = { _confirmed.trySend(Unit) }, ) } .onFailure { trace("failed to report - ${it.localizedMessage}") + // No confirmation, so the flow stays where it is and the reason is still + // picked — the retry is one press, not the whole flow again. BottomBarManager.showError( title = resources.getString(R.string.error_title_failedToReport), message = resources.getString(R.string.error_description_failedToReport), ) } + _submitting.value = false } } } diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt index eeecae35a2..64a54001e4 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt @@ -63,6 +63,9 @@ import com.getcode.ui.theme.CodeScaffold internal fun ReasonSelectionContent( onChoose: (ReportReason) -> Unit, onNavigateUp: () -> Unit, + // The flow stays up until the report is acknowledged, so the button has to show the wait it + // used to hide by closing. + submitting: Boolean = false, ) { // Saved, so stepping forward to the details screen and back again doesn't lose the pick that // got you there. @@ -96,6 +99,7 @@ internal fun ReasonSelectionContent( stringResource(R.string.action_submitReport) }, enabled = selected != null, + isLoading = submitting, onClick = { selected?.let(onChoose) }, ) }, diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt index fc3d4d49d3..e123834cca 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt @@ -67,6 +67,8 @@ import com.getcode.ui.utils.rememberKeyboardController internal fun ReportDetailsContent( state: TextFieldState, onSubmit: (String) -> Unit, + // See [ReasonSelectionContent]: the send is waited on here rather than behind a closed flow. + submitting: Boolean = false, onNavigateUp: () -> Unit, ) { val keyboard = rememberKeyboardController() @@ -137,6 +139,7 @@ internal fun ReportDetailsContent( .imePadding(), text = stringResource(R.string.action_submitReport), enabled = typed.isNotBlank() && !isOverLimit, + isLoading = submitting, onClick = { keyboard.hideIfVisible { onSubmit(typed.trim()) } }, ) } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt new file mode 100644 index 0000000000..f49860cb40 --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt @@ -0,0 +1,114 @@ +package com.flipcash.app.messenger.internal + +import com.flipcash.app.core.chat.ReportSubject +import com.flipcash.reporting.ReportReason +import com.flipcash.services.controllers.ReportingController +import com.getcode.manager.BottomBarManager +import com.getcode.manager.SelectedBottomBarAction +import com.getcode.util.resources.ResourceHelper +import io.mockk.coEvery +import io.mockk.mockk +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test + +/** + * What a report does once it has been sent. + * + * The flow used to close the moment submit was called and let a message land behind it, which said + * the report was sent to a screen that was already gone. These pin the replacement: the send is + * confirmed while the flow is still up, and it is dismissing that confirmation which closes it. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class ReportViewModelTest { + + private val reporting = mockk(relaxed = true) + private val resources = mockk(relaxed = true) + private val subject = ReportSubject.User(listOf(1, 2, 3)) + + @Before + fun setUp() { + Dispatchers.setMain(UnconfinedTestDispatcher()) + BottomBarManager.clear() + } + + @After + fun tearDown() { + Dispatchers.resetMain() + BottomBarManager.clear() + } + + private fun viewModel() = ReportViewModel(reporting, resources) + + @Test + fun `a sent report is confirmed, not mentioned in passing`() = runTest { + coEvery { reporting.report(any(), any()) } returns Result.success(Unit) + + viewModel().submit(subject, ReportReason.Spam, details = null) + + val shown = BottomBarManager.messages.value.last() + assertEquals(BottomBarManager.BottomBarMessageType.SUCCESS, shown.type) + } + + @Test + fun `the flow closes when the confirmation is dismissed, not when the report lands`() = runTest { + coEvery { reporting.report(any(), any()) } returns Result.success(Unit) + val viewModel = viewModel() + + var closed = false + val collector = launch(UnconfinedTestDispatcher(testScheduler)) { viewModel.confirmed.collect { closed = true } } + + viewModel.submit(subject, ReportReason.Spam, details = null) + assertFalse("the flow closed before anyone saw the confirmation", closed) + + BottomBarManager.messages.value.last().onClose(SelectedBottomBarAction(index = 0)) + assertTrue("dismissing the confirmation left the flow open", closed) + + collector.cancel() + } + + @Test + fun `a failed report leaves the flow open to try again`() = runTest { + coEvery { reporting.report(any(), any()) } returns Result.failure(Throwable("nope")) + val viewModel = viewModel() + + var closed = false + val collector = launch(UnconfinedTestDispatcher(testScheduler)) { viewModel.confirmed.collect { closed = true } } + + viewModel.submit(subject, ReportReason.Spam, details = null) + + assertEquals( + BottomBarManager.BottomBarMessageType.ERROR, + BottomBarManager.messages.value.last().type, + ) + assertFalse("a failed report closed the flow", closed) + + collector.cancel() + } + + @Test + fun `the send is visible while it is in flight`() = runTest { + val inFlight = CompletableDeferred>() + coEvery { reporting.report(any(), any()) } coAnswers { inFlight.await() } + val viewModel = viewModel() + + assertFalse(viewModel.submitting.value) + + viewModel.submit(subject, ReportReason.Spam, details = null) + assertTrue("the button gives no sign the report is being sent", viewModel.submitting.value) + + inFlight.complete(Result.success(Unit)) + assertFalse("the button stayed busy after the report landed", viewModel.submitting.value) + } +} From 82a8802518fe4f5a3817d1f37b8358dd64f0ae41 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 08:37:23 -0400 Subject: [PATCH 08/14] feat(reporting): let the submit button carry the send MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The button only showed the wait; the checkmark every other confirm button in the app draws was skipped, and the confirmation came up the instant the send returned. `ReportViewModel` now exposes `LoadingSuccessState` rather than a bare `submitting` flag, and holds `success` for `SuccessHoldDuration` before raising the confirmation — the same beat the username-claim, name-entry and photo-selection screens wait. The checkmark is drawn on the frame after the state is set, so raising the prompt immediately would put a scrim over one nobody had seen. The success state is held rather than reset once the prompt is up. The flow is still on screen behind it, and `CodeButton` disables itself while `isSuccess`, so the button reads as done instead of offering to file the same report again. A failure goes back to idle, because the retry needs a pressable button; the double-submit guard now reads `!isIdle`, which covers the press after success as well as the one mid-flight. --- .../app/messenger/ReportFlowScreen.kt | 8 +- .../app/messenger/internal/ReportViewModel.kt | 36 +++++-- .../internal/screens/ReasonSelectionScreen.kt | 10 +- .../internal/screens/ReportDetailsScreen.kt | 8 +- .../messenger/internal/ReportViewModelTest.kt | 96 ++++++++++++++++--- 5 files changed, 125 insertions(+), 33 deletions(-) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt index e488a3ec51..4f18c512ff 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt @@ -75,12 +75,12 @@ private fun reportEntryProvider( private fun FlowReasonSelection(subject: ReportSubject) { val viewModel = flowSharedViewModel() val flowNavigator = rememberFlowNavigator() - val submitting by viewModel.submitting.collectAsStateWithLifecycle() + val progress by viewModel.state.collectAsStateWithLifecycle() ExitOnceConfirmed(viewModel, flowNavigator) ReasonSelectionContent( - submitting = submitting, + progress = progress, onChoose = { reason -> if (reason == ReportReason.Other) { flowNavigator.navigateTo(ReportStep.Details(reason)) @@ -104,13 +104,13 @@ private fun FlowReportDetails(subject: ReportSubject, reason: ReportReason) { // typed — leaving by the back arrow is usually a second thought about the reason, not about // the words. val details = rememberTextFieldState() - val submitting by viewModel.submitting.collectAsStateWithLifecycle() + val progress by viewModel.state.collectAsStateWithLifecycle() ExitOnceConfirmed(viewModel, flowNavigator) ReportDetailsContent( state = details, - submitting = submitting, + progress = progress, onSubmit = { typed -> viewModel.submit(subject, reason, typed) }, // Not the flow's root, so this steps back to the reasons rather than leaving. onNavigateUp = { flowNavigator.back() }, diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt index 9096d93eac..6985198d7d 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt @@ -11,8 +11,11 @@ import com.flipcash.services.models.ReportTarget import com.getcode.manager.BottomBarManager import com.getcode.util.resources.ResourceHelper import com.getcode.utils.trace +import com.getcode.view.LoadingSuccessState +import com.getcode.view.SuccessHoldDuration import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.channels.Channel +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -40,10 +43,16 @@ internal class ReportViewModel @Inject constructor( private val resources: ResourceHelper, ) : ViewModel() { - private val _submitting = MutableStateFlow(false) + private val _state = MutableStateFlow(LoadingSuccessState()) - /** True while the report is in flight, so the button can say so rather than look ignored. */ - val submitting: StateFlow = _submitting.asStateFlow() + /** + * What the submit button is showing: the send in flight, then that it landed. + * + * It stays on [LoadingSuccessState.success] rather than returning to idle, because the flow is + * still on screen behind the confirmation and a button back to "Submit Report" would be + * offering to file the same report again. + */ + val state: StateFlow = _state.asStateFlow() // Buffered rather than a shared flow: this fires once, from a bottom bar callback that can // outlive the composition collecting it, and losing it would strand the flow open. @@ -59,9 +68,9 @@ internal class ReportViewModel @Inject constructor( /** [details] is the reporter's own words, and only [ReportReason.Other] collects them. */ fun submit(subject: ReportSubject, reason: ReportReason, details: String?) { - // A second press while the first is in flight would file the same report twice and answer - // with two confirmations stacked on each other. - if (_submitting.value) return + // Covers a second press both while the first is in flight and after it has succeeded — + // either would file the same report twice and answer with two confirmations. + if (!_state.value.isIdle) return val target = when (subject) { is ReportSubject.User -> ReportTarget.User(subject.userId) @@ -73,9 +82,15 @@ internal class ReportViewModel @Inject constructor( } viewModelScope.launch { - _submitting.value = true + _state.value = LoadingSuccessState(loading = true) reporting.report(target, ReportDescription.build(reason, details)) .onSuccess { + _state.value = LoadingSuccessState(success = true) + // The checkmark is drawn on the frame after this is set, so the confirmation + // waits out the same hold every other confirm button in the app does. Raising + // it immediately would put a scrim over a checkmark nobody had seen yet. + delay(SuccessHoldDuration) + // The contract makes a duplicate report a no-op that answers OK, so this says // the same thing whether or not the report was the first one. That is the // intended reading: telling someone they had already reported this would be @@ -88,14 +103,15 @@ internal class ReportViewModel @Inject constructor( } .onFailure { trace("failed to report - ${it.localizedMessage}") - // No confirmation, so the flow stays where it is and the reason is still - // picked — the retry is one press, not the whole flow again. + // Back to idle, not to an error state: the flow stays where it is with the + // reason still picked, so the retry is one press on a button that has to be + // pressable to take it. + _state.value = LoadingSuccessState() BottomBarManager.showError( title = resources.getString(R.string.error_title_failedToReport), message = resources.getString(R.string.error_description_failedToReport), ) } - _submitting.value = false } } } diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt index 64a54001e4..917af9247e 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt @@ -38,6 +38,7 @@ import com.getcode.ui.core.verticalScrollStateGradient import com.getcode.ui.theme.CodeButton import com.getcode.ui.theme.CodeRadioButton import com.getcode.ui.theme.CodeScaffold +import com.getcode.view.LoadingSuccessState /** * Why this is being reported: one row per [ReportReason], picked before it is sent. @@ -63,9 +64,9 @@ import com.getcode.ui.theme.CodeScaffold internal fun ReasonSelectionContent( onChoose: (ReportReason) -> Unit, onNavigateUp: () -> Unit, - // The flow stays up until the report is acknowledged, so the button has to show the wait it - // used to hide by closing. - submitting: Boolean = false, + // The flow stays up until the report is acknowledged, so the button carries the send itself: + // the wait, then the checkmark that closing used to swallow. + progress: LoadingSuccessState = LoadingSuccessState(), ) { // Saved, so stepping forward to the details screen and back again doesn't lose the pick that // got you there. @@ -99,7 +100,8 @@ internal fun ReasonSelectionContent( stringResource(R.string.action_submitReport) }, enabled = selected != null, - isLoading = submitting, + isLoading = progress.loading, + isSuccess = progress.success, onClick = { selected?.let(onChoose) }, ) }, diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt index e123834cca..e9f05d6282 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt @@ -42,6 +42,7 @@ import com.getcode.ui.core.verticalScrollStateGradient import com.getcode.ui.theme.CodeButton import com.getcode.ui.theme.CodeScaffold import com.getcode.ui.utils.rememberKeyboardController +import com.getcode.view.LoadingSuccessState /** * The free-text step, reached only by picking [ReportReason.Other]. @@ -67,8 +68,8 @@ import com.getcode.ui.utils.rememberKeyboardController internal fun ReportDetailsContent( state: TextFieldState, onSubmit: (String) -> Unit, - // See [ReasonSelectionContent]: the send is waited on here rather than behind a closed flow. - submitting: Boolean = false, + // See [ReasonSelectionContent]: the send is shown here rather than behind a closed flow. + progress: LoadingSuccessState = LoadingSuccessState(), onNavigateUp: () -> Unit, ) { val keyboard = rememberKeyboardController() @@ -139,7 +140,8 @@ internal fun ReportDetailsContent( .imePadding(), text = stringResource(R.string.action_submitReport), enabled = typed.isNotBlank() && !isOverLimit, - isLoading = submitting, + isLoading = progress.loading, + isSuccess = progress.success, onClick = { keyboard.hideIfVisible { onSubmit(typed.trim()) } }, ) } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt index f49860cb40..589c4fe97b 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt @@ -6,13 +6,17 @@ import com.flipcash.services.controllers.ReportingController import com.getcode.manager.BottomBarManager import com.getcode.manager.SelectedBottomBarAction import com.getcode.util.resources.ResourceHelper +import com.getcode.view.SuccessHoldDuration import io.mockk.coEvery import io.mockk.mockk import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestCoroutineScheduler import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain @@ -27,8 +31,9 @@ import org.junit.Test * What a report does once it has been sent. * * The flow used to close the moment submit was called and let a message land behind it, which said - * the report was sent to a screen that was already gone. These pin the replacement: the send is - * confirmed while the flow is still up, and it is dismissing that confirmation which closes it. + * the report was sent to a screen that was already gone. These pin the replacement: the button + * carries the send and holds its checkmark long enough to be read, the confirmation follows while + * the flow is still up, and it is dismissing that confirmation which closes it. */ @OptIn(ExperimentalCoroutinesApi::class) class ReportViewModelTest { @@ -37,9 +42,13 @@ class ReportViewModelTest { private val resources = mockk(relaxed = true) private val subject = ReportSubject.User(listOf(1, 2, 3)) + // Shared with `runTest` below, so the hold the view model waits out is the same virtual clock + // the test advances. A dispatcher on its own scheduler would make that delay real time. + private val scheduler = TestCoroutineScheduler() + @Before fun setUp() { - Dispatchers.setMain(UnconfinedTestDispatcher()) + Dispatchers.setMain(UnconfinedTestDispatcher(scheduler)) BottomBarManager.clear() } @@ -52,24 +61,63 @@ class ReportViewModelTest { private fun viewModel() = ReportViewModel(reporting, resources) @Test - fun `a sent report is confirmed, not mentioned in passing`() = runTest { + fun `a sent report is confirmed, not mentioned in passing`() = runTest(scheduler) { coEvery { reporting.report(any(), any()) } returns Result.success(Unit) viewModel().submit(subject, ReportReason.Spam, details = null) + advanceUntilIdle() val shown = BottomBarManager.messages.value.last() assertEquals(BottomBarManager.BottomBarMessageType.SUCCESS, shown.type) } @Test - fun `the flow closes when the confirmation is dismissed, not when the report lands`() = runTest { + fun `the checkmark is held long enough to be read before the confirmation`() = runTest(scheduler) { + coEvery { reporting.report(any(), any()) } returns Result.success(Unit) + val viewModel = viewModel() + + viewModel.submit(subject, ReportReason.Spam, details = null) + + assertTrue("the button never showed the send succeeded", viewModel.state.value.success) + assertTrue( + "the confirmation covered the checkmark before it could be read", + BottomBarManager.messages.value.isEmpty(), + ) + + advanceTimeBy(SuccessHoldDuration) + advanceUntilIdle() + assertEquals( + BottomBarManager.BottomBarMessageType.SUCCESS, + BottomBarManager.messages.value.last().type, + ) + } + + @Test + fun `the button stays done while the confirmation is up`() = runTest(scheduler) { + coEvery { reporting.report(any(), any()) } returns Result.success(Unit) + val viewModel = viewModel() + + viewModel.submit(subject, ReportReason.Spam, details = null) + advanceUntilIdle() + + assertTrue( + "the button went back to inviting a second report of the same thing", + viewModel.state.value.success, + ) + } + + @Test + fun `the flow closes when the confirmation is dismissed, not when the report lands`() = runTest(scheduler) { coEvery { reporting.report(any(), any()) } returns Result.success(Unit) val viewModel = viewModel() var closed = false - val collector = launch(UnconfinedTestDispatcher(testScheduler)) { viewModel.confirmed.collect { closed = true } } + val collector = launch(UnconfinedTestDispatcher(testScheduler)) { + viewModel.confirmed.collect { closed = true } + } viewModel.submit(subject, ReportReason.Spam, details = null) + advanceUntilIdle() assertFalse("the flow closed before anyone saw the confirmation", closed) BottomBarManager.messages.value.last().onClose(SelectedBottomBarAction(index = 0)) @@ -79,36 +127,60 @@ class ReportViewModelTest { } @Test - fun `a failed report leaves the flow open to try again`() = runTest { + fun `a failed report leaves the flow open to try again`() = runTest(scheduler) { coEvery { reporting.report(any(), any()) } returns Result.failure(Throwable("nope")) val viewModel = viewModel() var closed = false - val collector = launch(UnconfinedTestDispatcher(testScheduler)) { viewModel.confirmed.collect { closed = true } } + val collector = launch(UnconfinedTestDispatcher(testScheduler)) { + viewModel.confirmed.collect { closed = true } + } viewModel.submit(subject, ReportReason.Spam, details = null) + advanceUntilIdle() assertEquals( BottomBarManager.BottomBarMessageType.ERROR, BottomBarManager.messages.value.last().type, ) assertFalse("a failed report closed the flow", closed) + assertTrue("the retry was left disabled", viewModel.state.value.isIdle) collector.cancel() } @Test - fun `the send is visible while it is in flight`() = runTest { + fun `the send is visible while it is in flight`() = runTest(scheduler) { val inFlight = CompletableDeferred>() coEvery { reporting.report(any(), any()) } coAnswers { inFlight.await() } val viewModel = viewModel() - assertFalse(viewModel.submitting.value) + assertTrue(viewModel.state.value.isIdle) + + viewModel.submit(subject, ReportReason.Spam, details = null) + assertTrue("the button gives no sign the report is being sent", viewModel.state.value.loading) + + inFlight.complete(Result.success(Unit)) + advanceUntilIdle() + assertFalse("the button was still spinning after the report landed", viewModel.state.value.loading) + } + + @Test + fun `a second press cannot file the same report twice`() = runTest(scheduler) { + val inFlight = CompletableDeferred>() + coEvery { reporting.report(any(), any()) } coAnswers { inFlight.await() } + val viewModel = viewModel() viewModel.submit(subject, ReportReason.Spam, details = null) - assertTrue("the button gives no sign the report is being sent", viewModel.submitting.value) + viewModel.submit(subject, ReportReason.Spam, details = null) inFlight.complete(Result.success(Unit)) - assertFalse("the button stayed busy after the report landed", viewModel.submitting.value) + advanceUntilIdle() + + assertEquals( + "the report was filed twice, and answered twice", + 1, + BottomBarManager.messages.value.size, + ) } } From 69b0e483dec660d4de532ebc63bae63631aa9a21 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 09:47:19 -0400 Subject: [PATCH 09/14] fix(reporting): let the send's progress show on the button CodeButton takes its content colour, and its clickability, from `enabled` rather than from `isLoading`/`isSuccess`. Left enabled, a Filled button keeps its white background: the spinner is drawn in the content colour and the checkmark in a hardcoded white, so both came out white on white and the button read as blank for the whole send. It also went on taking presses. Pair `enabled` with `progress.isIdle` on both steps, which is what every other confirm button in the app does. --- .../internal/screens/ReasonSelectionScreen.kt | 6 ++- .../internal/screens/ReportDetailsScreen.kt | 3 +- .../internal/screens/ReportFlowTest.kt | 45 ++++++++++++++++++- 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt index 917af9247e..d5b4396524 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReasonSelectionScreen.kt @@ -99,7 +99,11 @@ internal fun ReasonSelectionContent( } else { stringResource(R.string.action_submitReport) }, - enabled = selected != null, + // `progress.isIdle` as well as the pick: `CodeButton` reads its content + // colour and its clickability from `enabled`, not from `isLoading`/ + // `isSuccess`. Left enabled, the Filled button stays white, which hides a + // checkmark drawn in white and keeps taking presses through the send. + enabled = selected != null && progress.isIdle, isLoading = progress.loading, isSuccess = progress.success, onClick = { selected?.let(onChoose) }, diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt index e9f05d6282..5301e38b56 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ReportDetailsScreen.kt @@ -139,7 +139,8 @@ internal fun ReportDetailsContent( .padding(bottom = CodeTheme.dimens.grid.x3) .imePadding(), text = stringResource(R.string.action_submitReport), - enabled = typed.isNotBlank() && !isOverLimit, + // See [ReasonSelectionContent] on why `progress.isIdle` belongs here. + enabled = typed.isNotBlank() && !isOverLimit && progress.isIdle, isLoading = progress.loading, isSuccess = progress.success, onClick = { keyboard.hideIfVisible { onSubmit(typed.trim()) } }, diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt index d62934bd58..90425f1c68 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/ReportFlowTest.kt @@ -23,6 +23,7 @@ import com.flipcash.features.messenger.R import com.flipcash.reporting.ReportDescription import com.flipcash.reporting.ReportReason import com.flipcash.app.theme.FlipcashPreview +import com.getcode.view.LoadingSuccessState import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Assert.assertNull @@ -43,10 +44,11 @@ class ReportFlowTest { private var submitted: String? = null private var dismissed = false - private fun showReasons() { + private fun showReasons(progress: LoadingSuccessState = LoadingSuccessState()) { composeTestRule.setContent { FlipcashPreview { ReasonSelectionContent( + progress = progress, onChoose = { chosen = it }, onNavigateUp = { dismissed = true }, ) @@ -54,11 +56,15 @@ class ReportFlowTest { } } - private fun showDetails(state: TextFieldState = TextFieldState()) { + private fun showDetails( + state: TextFieldState = TextFieldState(), + progress: LoadingSuccessState = LoadingSuccessState(), + ) { composeTestRule.setContent { FlipcashPreview { ReportDetailsContent( state = state, + progress = progress, onSubmit = { submitted = it }, onNavigateUp = { dismissed = true }, ) @@ -254,4 +260,39 @@ class ReportFlowTest { navControl("action_back").assertExists() assertNoNavControl("action_close") } + + // `CodeButton` draws its spinner in the content colour and its checkmark in a hardcoded white, + // and picks the content colour from `enabled` rather than from `isLoading`/`isSuccess`. Left + // enabled, a Filled button keeps its white background and the checkmark is white on white. + // Every other confirm button in the app pairs the two — `enabled = && + // progress.isIdle` — and that pairing is what these hold in place. `CodeButton` also wires + // `clickable` to the raw `enabled`, so without it the button keeps taking presses as well. + @Test + fun `the reason button stops taking presses while the report is sending`() { + showReasons(LoadingSuccessState(loading = true)) + + composeTestRule.onNodeWithTag("action_report_reason_${ReportReason.Spam.name}") + .performClick() + + composeTestRule.onNodeWithTag("action_report_reason_continue").assertIsNotEnabled() + } + + @Test + fun `the reason button stops taking presses once the report has landed`() { + showReasons(LoadingSuccessState(success = true)) + + composeTestRule.onNodeWithTag("action_report_reason_${ReportReason.Spam.name}") + .performClick() + + composeTestRule.onNodeWithTag("action_report_reason_continue").assertIsNotEnabled() + } + + @Test + fun `the details button stops taking presses while the report is sending`() { + showDetails(progress = LoadingSuccessState(loading = true)) + + composeTestRule.onNode(hasSetTextAction()).performTextInput("something happened") + + composeTestRule.onNodeWithTag("action_report_submit").assertIsNotEnabled() + } } From 43eba2f537d5321e461ba6cb2dd01320b389c41c Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 09:47:34 -0400 Subject: [PATCH 10/14] fix(chat): clear the selection bar when a message is reported MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every other action on the bar dispatches an event whose reducer drops the selection. Report only pushed a route, and because it leaves the chat for a flow of its own, nothing on the way back dismissed the bar either — the reported message was still sitting highlighted when the flow closed over it. Give it the event the others have. It carries no subject: the bar's caller has already captured the message to build the route with, so all this does is dismiss. --- .../app/messenger/internal/ChatViewModel.kt | 7 +++++++ .../internal/screens/components/ChatTopBar.kt | 8 +++++++- .../internal/ChatMessageActionReducerTest.kt | 15 +++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt index 0b58b4229a..f48f6dc9bf 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ChatViewModel.kt @@ -466,6 +466,10 @@ internal class ChatViewModel @Inject constructor( data class EditMessage(val messageId: Long, val text: String) : Event data class DeleteMessage(val messageId: Long) : Event + // Carries nothing because it does nothing but dismiss the bar: report is its own top-level + // flow, and the bar's caller has already captured the message to push the route with. + data object ReportRequested : Event + data object SubmitEdit : Event data object CancelEdit : Event data object EditingEnded : Event @@ -2162,6 +2166,9 @@ internal class ChatViewModel @Inject constructor( is Event.CopyMessage -> { state -> state.copy(selection = null, confirmingDelete = false) } + Event.ReportRequested -> { state -> + state.copy(selection = null, confirmingDelete = false) + } is Event.EditMessage -> { state -> state.copy( selection = null, diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt index 3623de2d78..b17f26c8d9 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt @@ -309,7 +309,13 @@ private fun MessageSelectionBar( label = stringResource(R.string.title_report), icon = Icons.Outlined.Flag, testTag = "action_report_message", - onClick = onReport, + // Unlike the others this one leaves the chat, so nothing on the way back + // dismisses the bar. Clearing it here means the transcript is at rest behind + // the report flow, and still at rest when it closes. + onClick = { + dispatch(ChatViewModel.Event.ReportRequested) + onReport() + }, ) ) } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt index 790879ae2b..8070348935 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatMessageActionReducerTest.kt @@ -141,6 +141,21 @@ class ChatMessageActionReducerTest { assertNull(state.selection) } + @Test + fun `reporting clears the bar`() { + // Report leaves the chat for a flow of its own, so nothing here dismisses the bar on the + // way back. Without this the transcript is still holding the reported message highlighted + // when the flow closes over it. + val selected = reduce( + ChatViewModel.State(messagePolicy = unbounded), + ChatViewModel.Event.ToggleMessageSelection(bubble(1)), + ) + + val state = reduce(selected, ChatViewModel.Event.ReportRequested) + + assertNull(state.selection) + } + @Test fun `the delete confirmation holds the selection but drops the focus`() { // The bar still has its message, so the sheet's Cancel has something to return to. The From 5a9482f7b16fff76b70284c20a005f635ac66e13 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 09:47:44 -0400 Subject: [PATCH 11/14] fix(chat): stop the selection bar hiding actions it has room for MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a 400dp phone another participant's message put copy and report in the overflow while most of the bar stood empty. The budget is a fraction of the slot the actions lay out in, and that slot is the bar less the app bar's 5dp either side — so the sum was 0.35 of 390dp, or 136.5dp, against the 140dp that three 40dp buttons and their two 10dp gaps need. Three and a half short. Raise the fraction to 0.5, which clears three buttons down to a 320dp screen. It is a ceiling rather than a width: the row is laid out to its contents, so a bar leaving the budget unspent is no wider for it. Pull the arithmetic out as `inlineActionCapacity` so the widths that decide it can be tested without laying a bar out. `MessageActionCapacityTest` pins the device widths; `ChatSelectionBarTest` pins the rendering at 400dp, which the class already defaults to. --- .../internal/screens/components/ChatTopBar.kt | 31 +++++++--- .../internal/ChatSelectionBarTest.kt | 20 ++++++ .../components/MessageActionCapacityTest.kt | 62 +++++++++++++++++++ 3 files changed, 104 insertions(+), 9 deletions(-) create mode 100644 apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageActionCapacityTest.kt diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt index b17f26c8d9..73d1b51b18 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt @@ -351,14 +351,30 @@ private data class MessageAction( ) /** - * The share of the bar the actions may occupy before they start collapsing into the overflow. + * The share of the actions' slot they may occupy before the rest collapse into the overflow. * * A share rather than a slot count, so the answer tracks the screen: at 40dp a button and 10dp - * between them, a normal phone fits all three actions and a compact one keeps the first inline - * with the rest a tap away. It stays a minority of the bar so the cluster still reads as trailing - * and leaves room for a title, should the selection bar ever grow one. + * between them, every phone width fits the three actions another participant's message offers, and + * the two extra on your own collapse into the menu. + * + * It is a ceiling, not a width — the row is laid out to its contents, so a bar leaving the budget + * unspent is no wider for it. The figure only has to clear three buttons and their two gaps, 140dp, + * on the narrowest phone worth supporting. 0.35 did not, and the miss was small enough to look like + * it should have worked: the slot is the bar less the app bar's 5dp either side, so a 400dp device + * budgets against 390dp and lands on 136.5dp — three and a half short of three buttons, with most + * of the bar standing empty and copy and report in a menu. + */ +private const val ActionBudgetFraction = 0.5f + +/** + * How many actions stay as icons in a [barWidth] slot. Extracted so the widths that decide it can + * be tested without laying a bar out; see `MessageActionCapacityTest`. */ -private const val ActionBudgetFraction = 0.35f +internal fun inlineActionCapacity(barWidth: Dp, buttonSize: Dp, spacing: Dp): Int = + // n buttons cost n widths and n-1 gaps, so adding one gap to both sides makes it a division. + ((barWidth * ActionBudgetFraction + spacing) / (buttonSize + spacing)) + .toInt() + .coerceAtLeast(1) @Composable private fun MessageActions(actions: List) { @@ -370,10 +386,7 @@ private fun MessageActions(actions: List) { val spacing = CodeTheme.dimens.grid.x2 BoxWithConstraints { - // n buttons cost n widths and n-1 gaps, so adding one gap to both sides makes it a division. - val capacity = ((maxWidth * ActionBudgetFraction + spacing) / (buttonSize + spacing)) - .toInt() - .coerceAtLeast(1) + val capacity = inlineActionCapacity(maxWidth, buttonSize, spacing) // The overflow needs a slot of its own, so it only pays for itself when it is holding // something — the last action is not displaced by a menu that would contain only it. val inline = if (actions.size <= capacity) actions else actions.take(capacity - 1) diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt index 9eb976cc89..d7c0c16607 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt @@ -117,6 +117,26 @@ class ChatSelectionBarTest { composeTestRule.onAllNodesWithTag("action_report_message").assertCountEquals(0) } + // At the class's own 400dp — a real device width, and the one the budget used to tie on. + @Test + fun `another participant's message shows all three actions as icons`() { + select( + text( + isFromSelf = false, + capabilities = setOf( + MessageCapability.Copy, + MessageCapability.Reply, + MessageCapability.Report, + ), + ) + ) + + listOf("action_reply_message", "action_copy_message", "action_report_message") + .forEach { composeTestRule.onNodeWithTag(it).assertIsDisplayed() } + // Three fit, so there is nothing left for a menu to hold. + composeTestRule.onAllNodesWithTag("action_message_overflow").assertCountEquals(0) + } + private fun text(isFromSelf: Boolean, capabilities: Set) = ChatListItem.ContentBubble( messageId = 2, diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageActionCapacityTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageActionCapacityTest.kt new file mode 100644 index 0000000000..74cf6edba0 --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageActionCapacityTest.kt @@ -0,0 +1,62 @@ +package com.flipcash.app.messenger.internal.screens.components + +import androidx.compose.ui.unit.dp +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * How many of the selection bar's actions stay as icons before the rest collapse into the overflow. + * + * The number a phone lands on is the whole point of the budget, so these pin it at real device + * widths rather than at the fraction. Three is the figure that matters: another participant's text + * message offers reply, copy and report, and that is the common case. + */ +class MessageActionCapacityTest { + + /** + * The budget is measured on the slot the actions are laid out in, not on the screen. The app + * bar wraps that slot in 5dp either side, so the slot is 10dp short of the device — the gap + * that put a 400dp phone one capacity below three. + */ + private fun capacityOnDevice(deviceWidth: Int) = inlineActionCapacity( + // What the bar actually lays out: a 40dp circular button with 10dp between. + barWidth = (deviceWidth - 10).dp, + buttonSize = 40.dp, + spacing = 10.dp, + ) + + @Test + fun `a 400dp phone fits all three of another participant's actions`() { + // A real device width, and the one this was reported on: 0.35 of its 390dp slot came to + // 136.5dp against the 140dp three buttons and their two gaps need, so copy and report sat + // in a menu with most of the bar empty. + assertTrue(capacityOnDevice(400) >= 3) + } + + @Test + fun `the narrowest phone worth supporting still fits all three`() { + // 320dp is the floor, and the binding case: it is what the fraction has to clear. + assertEquals(3, capacityOnDevice(320)) + } + + @Test + fun `a typical phone fits all three`() { + assertTrue(capacityOnDevice(411) >= 3) + } + + @Test + fun `a tablet fits every action the bar has`() { + // Five is the most the bar ever offers — reply, delete, copy, edit and report on your own + // text. The budget is a ceiling rather than a width, so a roomier screen leaving it unspent + // costs nothing; what matters is only that nothing is forced into a menu. + assertTrue(capacityOnDevice(800) >= 5) + } + + @Test + fun `a bar with no room to speak of still offers one action`() { + // Below one button's width the budget goes negative; the bar shows an icon and a menu + // rather than a menu alone, which would hide reply behind two taps on every message. + assertEquals(1, inlineActionCapacity(barWidth = 0.dp, buttonSize = 40.dp, spacing = 10.dp)) + } +} From c21935a58b50458d1d270d4f39fffe0eb8d17d59 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 09:55:02 -0400 Subject: [PATCH 12/14] fix(chat): drop the ripple from the conversation title MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The click target is the whole title row, and `unboundedClickable` centres an unbounded ripple on it — so every tap washed a circle across the width of the bar on the way to the profile. `noRippleClickable` had no `role`, and swapping to it as it stood would have taken `Role.Button` off the row along with the indication. Give it an optional one, defaulting to null so no existing caller's semantics change, and pass `Role.Button` here. --- .../internal/screens/components/ChatTopBar.kt | 8 +- .../internal/ConversationTitleBarTest.kt | 80 +++++++++++++++++++ .../kotlin/com/getcode/ui/core/Modifier.kt | 25 ++++-- 3 files changed, 104 insertions(+), 9 deletions(-) create mode 100644 apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ConversationTitleBarTest.kt diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt index 73d1b51b18..ba7d530d91 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/ChatTopBar.kt @@ -41,6 +41,7 @@ import androidx.compose.ui.graphics.vector.ImageVector import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.Role import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.DpOffset import androidx.compose.ui.unit.dp @@ -62,7 +63,7 @@ import com.getcode.ui.components.AppBarDefaults import com.getcode.ui.components.AppBarWithTitle import com.getcode.ui.components.CircularIconButton import com.getcode.ui.core.measured -import com.getcode.ui.core.unboundedClickable +import com.getcode.ui.core.noRippleClickable import com.getcode.ui.utils.KeyboardController import com.getcode.ui.utils.rememberKeyboardController @@ -169,7 +170,10 @@ private fun ConversationTitleBar( .fillMaxWidth() .then( if (state.canViewProfile) { - Modifier.unboundedClickable { + // No indication: the target is the whole title row, so an unbounded + // ripple centred on it washed across the bar on every tap. The role + // is kept so the row still reads as a button. + Modifier.noRippleClickable(role = Role.Button) { chatActionHandler(ChatAction.ViewProfile) } } else { diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ConversationTitleBarTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ConversationTitleBarTest.kt new file mode 100644 index 0000000000..9af27f5dac --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ConversationTitleBarTest.kt @@ -0,0 +1,80 @@ +package com.flipcash.app.messenger.internal + +import androidx.activity.ComponentActivity +import androidx.compose.ui.semantics.Role +import androidx.compose.ui.semantics.SemanticsProperties +import androidx.compose.ui.test.SemanticsMatcher +import androidx.compose.ui.test.assert +import androidx.compose.ui.test.hasClickAction +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import com.flipcash.app.messenger.internal.screens.components.ChatTopBar +import com.flipcash.app.theme.FlipcashPreview +import com.flipcash.services.models.chat.ChatId +import com.flipcash.shared.chat.models.ChatAction +import com.flipcash.services.models.chat.ChatType +import com.getcode.navigation.core.CodeNavigator +import io.mockk.mockk +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import kotlin.test.assertEquals + +/** + * The conversation bar's title opens the subject's profile. It is the whole title row, so it draws + * no indication — an unbounded ripple centred on a row that wide washed across the bar on every + * tap. Dropping a ripple is easy to do by dropping the click's semantics with it, which is what + * these hold in place: the row still announces itself as a button and still acts on a tap. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], qualifiers = "w400dp-h800dp-xhdpi") +class ConversationTitleBarTest { + + @get:Rule + val composeTestRule = createAndroidComposeRule() + + private val group = ChatSubject.Group( + chatId = ChatId(byteArrayOf(1)), + groupTitle = "Bad Boys", + picture = null, + memberCount = 412L, + rules = null, + isMember = true, + ) + + private fun show(onAction: (ChatAction) -> Unit = {}) { + composeTestRule.setContent { + FlipcashPreview { + ChatTopBar( + navigator = mockk(relaxed = true), + state = ChatViewModel.State(chatType = ChatType.GROUP, subject = group), + onBarHeightChange = {}, + chatActionHandler = onAction, + dispatch = {}, + ) + } + } + } + + @Test + fun `the title still announces itself as a button`() { + show() + + composeTestRule.onNodeWithText("Bad Boys").assert( + SemanticsMatcher.expectValue(SemanticsProperties.Role, Role.Button) + ) + } + + @Test + fun `tapping the title opens the profile`() { + val actions = mutableListOf() + show(onAction = { actions += it }) + + composeTestRule.onNodeWithText("Bad Boys").assert(hasClickAction()).performClick() + + assertEquals(listOf(ChatAction.ViewProfile), actions.toList()) + } +} diff --git a/ui/core/src/main/kotlin/com/getcode/ui/core/Modifier.kt b/ui/core/src/main/kotlin/com/getcode/ui/core/Modifier.kt index d156b23ca7..9ef7692e6d 100644 --- a/ui/core/src/main/kotlin/com/getcode/ui/core/Modifier.kt +++ b/ui/core/src/main/kotlin/com/getcode/ui/core/Modifier.kt @@ -69,13 +69,24 @@ inline fun Modifier.addIf( } } -fun Modifier.noRippleClickable(enabled: Boolean = true, onClick: () -> Unit) = - this.clickable( - enabled = enabled, - indication = null, - interactionSource = null, - onClick = onClick - ) +/** + * A click with no indication at all. + * + * [role] defaults to null to leave every existing caller's semantics as they were; pass + * [Role.Button] when the target is one, so dropping the ripple does not also drop what a screen + * reader announces. + */ +fun Modifier.noRippleClickable( + enabled: Boolean = true, + role: Role? = null, + onClick: () -> Unit, +) = this.clickable( + enabled = enabled, + role = role, + indication = null, + interactionSource = null, + onClick = onClick +) fun Modifier.unboundedClickable( enabled: Boolean = true, From d67a68c170b2d96e3e48d6321d4e35e0065e6c0e Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 10:04:51 -0400 Subject: [PATCH 13/14] fix(reporting): give the send's spinner a floor so it is seen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A report is one unary call and usually answers inside a frame. The button crossfaded from its label straight to a checkmark, so the send read as never having happened. MinimumLoadingDuration puts a floor under the spinner, starting when the spinner does and running alongside the send, so a call that already outlasted it is not held back. It pairs with SuccessHoldDuration at the other end. Result.onSuccessWithDelay looked like the helper for this, but it starts its clock after the work has returned, so it always waits the full duration — latency on a slow call rather than a floor under a fast one. --- .../app/messenger/internal/ReportViewModel.kt | 18 ++++++++- .../messenger/internal/ReportViewModelTest.kt | 38 +++++++++++++++++++ .../kotlin/com/getcode/view/BaseViewModel.kt | 14 +++++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt index 6985198d7d..fc707f98b6 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/ReportViewModel.kt @@ -12,8 +12,10 @@ import com.getcode.manager.BottomBarManager import com.getcode.util.resources.ResourceHelper import com.getcode.utils.trace import com.getcode.view.LoadingSuccessState +import com.getcode.view.MinimumLoadingDuration import com.getcode.view.SuccessHoldDuration import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.async import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow @@ -83,8 +85,19 @@ internal class ReportViewModel @Inject constructor( viewModelScope.launch { _state.value = LoadingSuccessState(loading = true) - reporting.report(target, ReportDescription.build(reason, details)) + + // A report is a single unary call and usually answers inside a frame, so the button + // crossfaded from its label straight to a checkmark and the send read as never having + // happened. The floor starts with the spinner and runs alongside the send, so joining + // it waits only for whatever is left — a send that already outlasted it is not held + // back. Racing two `delay`s rather than reading a clock keeps this on whatever time + // source the caller's scheduler is using, virtual or real. + val floor = launch { delay(MinimumLoadingDuration) } + val sent = async { reporting.report(target, ReportDescription.build(reason, details)) } + + sent.await() .onSuccess { + floor.join() _state.value = LoadingSuccessState(success = true) // The checkmark is drawn on the frame after this is set, so the confirmation // waits out the same hold every other confirm button in the app does. Raising @@ -102,6 +115,9 @@ internal class ReportViewModel @Inject constructor( ) } .onFailure { + // Nothing is waiting on the floor down here: an error has its own bar to put + // up, and holding a spinner first would only delay it. + floor.cancel() trace("failed to report - ${it.localizedMessage}") // Back to idle, not to an error state: the flow stays where it is with the // reason still picked, so the retry is one press on a button that has to be diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt index 589c4fe97b..172387fc88 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ReportViewModelTest.kt @@ -6,6 +6,7 @@ import com.flipcash.services.controllers.ReportingController import com.getcode.manager.BottomBarManager import com.getcode.manager.SelectedBottomBarAction import com.getcode.util.resources.ResourceHelper +import com.getcode.view.MinimumLoadingDuration import com.getcode.view.SuccessHoldDuration import io.mockk.coEvery import io.mockk.mockk @@ -71,12 +72,49 @@ class ReportViewModelTest { assertEquals(BottomBarManager.BottomBarMessageType.SUCCESS, shown.type) } + @Test + fun `an instant send still shows its spinner`() = runTest(scheduler) { + // The report call answers inside a frame, so without a floor the button crossfaded from + // its label straight to the checkmark and the send read as having never happened. + coEvery { reporting.report(any(), any()) } returns Result.success(Unit) + val model = viewModel() + + model.submit(subject, ReportReason.Spam, details = null) + + assertTrue(model.state.value.loading) + advanceTimeBy(MinimumLoadingDuration.inWholeMilliseconds - 1) + assertTrue(model.state.value.loading) + advanceTimeBy(2) + assertTrue(model.state.value.success) + } + + @Test + fun `a slow send is not held back any further`() = runTest(scheduler) { + // The floor is a minimum, not an addition: a send that already outlasted it goes straight + // to the checkmark rather than sitting on a spinner for another half second. + val answer = CompletableDeferred>() + coEvery { reporting.report(any(), any()) } coAnswers { answer.await() } + val model = viewModel() + + model.submit(subject, ReportReason.Spam, details = null) + advanceTimeBy(MinimumLoadingDuration.inWholeMilliseconds * 2) + assertTrue(model.state.value.loading) + + answer.complete(Result.success(Unit)) + advanceTimeBy(1) + + assertTrue(model.state.value.success) + } + @Test fun `the checkmark is held long enough to be read before the confirmation`() = runTest(scheduler) { coEvery { reporting.report(any(), any()) } returns Result.success(Unit) val viewModel = viewModel() viewModel.submit(subject, ReportReason.Spam, details = null) + // Past the spinner's floor, which is where the checkmark's own hold starts. + // `advanceTimeBy` stops short of the endpoint, so the floor's own instant needs the +1. + advanceTimeBy(MinimumLoadingDuration.inWholeMilliseconds + 1) assertTrue("the button never showed the send succeeded", viewModel.state.value.success) assertTrue( diff --git a/ui/navigation/src/main/kotlin/com/getcode/view/BaseViewModel.kt b/ui/navigation/src/main/kotlin/com/getcode/view/BaseViewModel.kt index 26f639dea0..93a9b24752 100644 --- a/ui/navigation/src/main/kotlin/com/getcode/view/BaseViewModel.kt +++ b/ui/navigation/src/main/kotlin/com/getcode/view/BaseViewModel.kt @@ -83,6 +83,20 @@ abstract class BaseViewModel( */ val SuccessHoldDuration = 500.milliseconds +/** + * The least time a confirm button's spinner stays up once it has gone up. + * + * A floor, not a hold: work that already ran longer moves on as soon as it is done, and only work + * that answers faster than this is waited out. Pairs with [SuccessHoldDuration] at the other end — + * between them a fast call still reads as start, work, done, rather than crossfading from a label + * to a checkmark with nothing in between. + * + * Measure it from the frame the spinner was asked for. Measuring from the call's return, as + * `Result.onSuccessWithDelay` does, always waits the full duration, because by then the work is + * over — which adds latency to a slow call rather than putting a floor under a fast one. + */ +val MinimumLoadingDuration = 500.milliseconds + data class LoadingSuccessState( val loading: Boolean = false, val success: Boolean = false, From d9bd0095cd3118ca747258c606fb11bcde490f10 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 22 Sep 2026 10:06:32 -0400 Subject: [PATCH 14/14] fix(reporting): animate the report sheet out instead of cutting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Leaving the flow popped the entry outright, so the sheet vanished in a frame on every exit — close, back out of the root, and dismissing the confirmation alike. Hand the sheet navigator a dismiss instead. The scene animates the sheet down and pops the entry once that finishes, so onExit must not pop as well; that pair of pops is what used to land on the chat list. FlowHost has an animated exit already but gates it on isSheetRoot, which is backStack.size <= 1, and report opens over a chat. A sheet is no less a sheet for having something underneath, so that gate is wrong for every flow opened this way. Fixing it there changes exit behaviour for ten flows, three of which pop in onExit and would then pop twice, so it is left alone here. --- .../app/messenger/ReportFlowScreen.kt | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt index 4f18c512ff..d505631b32 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/ReportFlowScreen.kt @@ -19,6 +19,7 @@ import com.flipcash.app.messenger.internal.screens.ReportDetailsContent import com.flipcash.reporting.ReportReason import com.getcode.navigation.annotatedEntry import com.getcode.navigation.core.LocalCodeNavigator +import com.getcode.navigation.scenes.LocalSheetNavigator import com.getcode.navigation.flow.FlowHost import com.getcode.navigation.flow.FlowNavigator import com.getcode.navigation.flow.flowSharedViewModel @@ -47,6 +48,9 @@ fun ReportFlowScreen( resultStateRegistry: NavResultStateRegistry, ) { val navigator = LocalCodeNavigator.current + // Read before FlowHost overrides the locals below: non-null only because a sheet scene is what + // provides it, which makes its presence the answer to "is this flow being shown in a sheet". + val sheetNavigator = LocalSheetNavigator.current val keyboard = rememberKeyboardController() FlowHost( @@ -54,7 +58,26 @@ fun ReportFlowScreen( resultStateRegistry = resultStateRegistry, // The details step owns the keyboard, so put it away before the chat behind this is // uncovered — the same rule the chat and new-group flows follow. - onExit = { _, _ -> keyboard.hideIfVisible { navigator.pop() } }, + // + // Popping outright would take the sheet off in one frame. Handing the sheet navigator a + // dismiss instead lets the scene animate it down and pop the entry itself once that + // finishes, so this must not also pop — that pair of pops is what used to land us back on + // the chat list. + // + // FlowHost has an animated exit of its own but will not use it here: it gates on + // `isSheetRoot`, which is `backStack.size <= 1`, and report opens over a chat. A sheet is + // no less a sheet for having something underneath it, so that gate is wrong for every + // flow opened this way — fixing it there is a wider change than this one. + onExit = { _, _ -> + keyboard.hideIfVisible { + if (sheetNavigator != null) { + // Empty lambda: animate out, nothing to do afterwards. + sheetNavigator.pendingSheetDismiss = {} + } else { + navigator.pop() + } + } + }, entryProvider = reportEntryProvider(route.subject), sceneStrategies = listOf( ModalBottomSheetSceneStrategy(navigator.resultStore) { null },