mirror of
https://github.com/permissionlesstech/bitchat-android.git
synced 2026-08-15 06:56:30 +00:00
Key message list state per conversation
Switching channels reused every piece of state in MessagesList, because none of it was keyed on which conversation was being shown: - The LazyListState carried the previous channel's scroll offset, so the new channel opened at a stale position and then corrected itself. - hasScrolledToInitialPosition and followIncomingMessages carried over, so a channel entered after scrolling up in another one did not land on its newest message at all. - The arrival tracker had never seen the incoming channel's ids, so a backlog of six or fewer messages was treated as six simultaneous arrivals and each one slid in. - previousMessageCount carried over, arming placement animation for the relayout that the switch itself caused. All of it is now keyed on a conversationKey derived the same way displayMessages is. The tracker also detects a list sharing no ids with the previous one and adopts it silently, which covers /clear and any caller that does not supply a distinct key. Adds 4 tests for wholesale replacement, including the case that the burst cap cannot catch on its own.
This commit is contained in:
parent
bcfc33f35c
commit
fa43821cf4
@ -187,6 +187,20 @@ fun ChatScreen(viewModel: ChatViewModel) {
|
||||
}
|
||||
}
|
||||
|
||||
// Identity of the timeline on screen, derived exactly like displayMessages above. Drives the
|
||||
// per-conversation scroll position and animation state in MessagesList.
|
||||
val conversationKey = when {
|
||||
currentChannel != null -> "channel:$currentChannel"
|
||||
else -> {
|
||||
val locationChannel = selectedLocationChannel
|
||||
if (locationChannel is com.bitchat.android.geohash.ChannelID.Location) {
|
||||
"geo:${locationChannel.channel.geohash}"
|
||||
} else {
|
||||
"mesh"
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Determine whether to show media buttons (only hide in geohash location chats)
|
||||
val showMediaButtons = when {
|
||||
currentChannel != null -> true
|
||||
@ -229,6 +243,7 @@ fun ChatScreen(viewModel: ChatViewModel) {
|
||||
currentUserNickname = nickname,
|
||||
meshService = viewModel.meshServiceFacade,
|
||||
modifier = Modifier.fillMaxSize(),
|
||||
conversationKey = conversationKey,
|
||||
contentPadding = PaddingValues(
|
||||
top = statusBarHeight + headerHeight +
|
||||
(if (showNotesStrip) notesStripHeight else 0.dp),
|
||||
|
||||
@ -154,6 +154,9 @@ fun LocationChannelsSheet(
|
||||
val nearbyChannels = remember(availableChannels) {
|
||||
availableChannels.filter { it.level != GeohashChannelLevel.BUILDING }
|
||||
}
|
||||
val selectedChannelOutsideNearby = remember(selectedChannel, nearbyChannels) {
|
||||
selectedLocationChannelOutsideNearby(selectedChannel, nearbyChannels)
|
||||
}
|
||||
val showNearbyLoading = nearbyChannels.isEmpty() &&
|
||||
permissionState == LocationChannelManager.PermissionState.AUTHORIZED &&
|
||||
locationServicesEnabled
|
||||
@ -272,6 +275,35 @@ fun LocationChannelsSheet(
|
||||
shape = AboutCardShape
|
||||
) {
|
||||
Column {
|
||||
selectedChannelOutsideNearby?.let { channel ->
|
||||
val coverage = coverageString(channel.geohash.length)
|
||||
val name = bookmarkNames[channel.geohash]
|
||||
val subtitle = "#${channel.geohash} • $coverage" +
|
||||
(name?.let { " • ${formattedNamePrefix(channel.level)}$it" } ?: "")
|
||||
val participantCount = geohashParticipantCounts[channel.geohash] ?: 0
|
||||
val isBookmarked = bookmarksStore.isBookmarked(channel.geohash)
|
||||
|
||||
ChannelOptionRow(
|
||||
title = geohashTitleWithCount(channel, participantCount),
|
||||
subtitle = subtitle,
|
||||
isSelected = true,
|
||||
participantCount = participantCount,
|
||||
titleColor = standardGreen,
|
||||
titleBold = participantCount > 0,
|
||||
trailingContent = {
|
||||
ChannelBookmarkButton(
|
||||
bookmarked = isBookmarked,
|
||||
onClick = { bookmarksStore.toggle(channel.geohash) }
|
||||
)
|
||||
},
|
||||
onClick = {
|
||||
locationManager.select(ChannelID.Location(channel))
|
||||
onDismiss()
|
||||
}
|
||||
)
|
||||
SheetCardDivider()
|
||||
}
|
||||
|
||||
if (locationServicesEnabled) {
|
||||
if (nearbyChannels.isNotEmpty()) {
|
||||
nearbyChannels.forEachIndexed { index, channel ->
|
||||
@ -952,6 +984,24 @@ private fun isChannelSelected(channel: GeohashChannel, selectedChannel: ChannelI
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the active location channel when it has no row in the nearby-channel list.
|
||||
*
|
||||
* This commonly happens after teleporting to a remote geohash. Keeping the selected channel in the
|
||||
* main channel card makes its selection and bookmark action available even before it is bookmarked.
|
||||
*/
|
||||
internal fun selectedLocationChannelOutsideNearby(
|
||||
selectedChannel: ChannelID?,
|
||||
nearbyChannels: List<GeohashChannel>
|
||||
): GeohashChannel? {
|
||||
val selected = (selectedChannel as? ChannelID.Location)?.channel ?: return null
|
||||
return selected.takeUnless { active ->
|
||||
nearbyChannels.any { nearby ->
|
||||
nearby.geohash.equals(active.geohash, ignoreCase = true)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private fun validateGeohash(geohash: String): Boolean {
|
||||
if (geohash.isEmpty() || geohash.length > 12) return false
|
||||
val allowed = "0123456789bcdefghjkmnpqrstuvwxyz".toSet()
|
||||
|
||||
@ -887,6 +887,7 @@ fun PrivateChatSheet(
|
||||
currentUserNickname = nickname,
|
||||
meshService = viewModel.meshServiceFacade,
|
||||
modifier = Modifier.weight(1f),
|
||||
conversationKey = "dm:$peerID",
|
||||
forceScrollToBottom = forceScrollToBottom,
|
||||
onScrolledUpChanged = { isUp -> isScrolledUp = isUp },
|
||||
onNicknameClick = { /* handle mention */ },
|
||||
|
||||
@ -29,8 +29,8 @@ import androidx.compose.foundation.layout.fillMaxWidth
|
||||
import androidx.compose.foundation.layout.padding
|
||||
import androidx.compose.foundation.layout.size
|
||||
import androidx.compose.foundation.lazy.LazyColumn
|
||||
import androidx.compose.foundation.lazy.LazyListState
|
||||
import androidx.compose.foundation.lazy.itemsIndexed
|
||||
import androidx.compose.foundation.lazy.rememberLazyListState
|
||||
import androidx.compose.foundation.shape.CircleShape
|
||||
import androidx.compose.material3.ColorScheme
|
||||
import androidx.compose.material3.Icon
|
||||
@ -42,6 +42,7 @@ import androidx.compose.runtime.derivedStateOf
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.saveable.rememberSaveable
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
@ -153,6 +154,12 @@ internal fun MessageArrivalTracker.arrivals(messages: List<BitchatMessage>): Set
|
||||
return emptySet()
|
||||
}
|
||||
|
||||
// A list with nothing in common with the last one is a different conversation, not a burst of
|
||||
// arrivals — /clear, or a switch the caller did not give us a distinct key for. Adopt it
|
||||
// silently rather than sliding in every message at once.
|
||||
val isWholesaleReplacement =
|
||||
messages.isNotEmpty() && known.isNotEmpty() && messages.none { it.id in known }
|
||||
|
||||
// `HashSet.add` reports whether the id was new, so this both diffs and updates in one pass.
|
||||
val added = messages.filter { known.add(it.id) }
|
||||
|
||||
@ -163,6 +170,7 @@ internal fun MessageArrivalTracker.arrivals(messages: List<BitchatMessage>): Set
|
||||
}
|
||||
|
||||
return when {
|
||||
isWholesaleReplacement -> emptySet()
|
||||
added.isEmpty() || added.size > MaxAnimatedArrivals -> emptySet()
|
||||
else -> added.mapTo(HashSet(added.size)) { it.id }
|
||||
}
|
||||
@ -181,6 +189,14 @@ fun MessagesList(
|
||||
* has to reserve room for their heights here rather than by shrinking the viewport.
|
||||
*/
|
||||
contentPadding: PaddingValues = PaddingValues(0.dp),
|
||||
/**
|
||||
* Identity of the conversation being shown — a channel, a geohash, a peer.
|
||||
*
|
||||
* Everything below that is per-conversation state is keyed on this. Without it, switching
|
||||
* channels reused the previous conversation's scroll offset, follow flag and seen-message set,
|
||||
* so the new channel opened at a stale position and then animated itself into place.
|
||||
*/
|
||||
conversationKey: Any? = null,
|
||||
forceScrollToBottom: Boolean = false,
|
||||
onScrolledUpChanged: ((Boolean) -> Unit)? = null,
|
||||
onNicknameClick: ((String) -> Unit)? = null,
|
||||
@ -188,11 +204,19 @@ fun MessagesList(
|
||||
onCancelTransfer: ((BitchatMessage) -> Unit)? = null,
|
||||
onImageClick: ((String, List<String>, Int) -> Unit)? = null
|
||||
) {
|
||||
val listState = rememberLazyListState()
|
||||
|
||||
// A fresh scroll position per conversation. Sharing one state meant a switch inherited the
|
||||
// previous channel's offset and then had to correct itself, which is what the jump was.
|
||||
//
|
||||
// Passing the key as an *input* rather than as `key =` is deliberate: it discards the saved
|
||||
// offset on every switch, so a conversation always opens on its newest message instead of
|
||||
// wherever the reader happened to be some time ago, with unseen messages below them.
|
||||
val listState = rememberSaveable(conversationKey, saver = LazyListState.Saver) {
|
||||
LazyListState()
|
||||
}
|
||||
|
||||
// Track if this is the first time messages are being loaded
|
||||
var hasScrolledToInitialPosition by remember { mutableStateOf(false) }
|
||||
var followIncomingMessages by remember { mutableStateOf(true) }
|
||||
var hasScrolledToInitialPosition by remember(conversationKey) { mutableStateOf(false) }
|
||||
var followIncomingMessages by remember(conversationKey) { mutableStateOf(true) }
|
||||
|
||||
// Smart scroll: auto-scroll to bottom for initial load, then follow unless user scrolls away
|
||||
LaunchedEffect(messages.size) {
|
||||
@ -208,7 +232,7 @@ fun MessagesList(
|
||||
}
|
||||
|
||||
// Track whether user has scrolled away from the latest messages
|
||||
val isAtLatest by remember {
|
||||
val isAtLatest by remember(listState) {
|
||||
derivedStateOf {
|
||||
val firstVisibleIndex = listState.layoutInfo.visibleItemsInfo.firstOrNull()?.index ?: -1
|
||||
firstVisibleIndex <= 2
|
||||
@ -230,8 +254,10 @@ fun MessagesList(
|
||||
|
||||
// Recomputed only when the list actually gains or loses a message, and synchronously, so the
|
||||
// arriving item can read its cue during the same composition pass in which it first appears.
|
||||
val arrivalTracker = remember { MessageArrivalTracker() }
|
||||
val enteringIds = remember(messages.size, messages.lastOrNull()?.id) {
|
||||
// Reset per conversation, so a switch adopts the incoming messages silently instead of
|
||||
// treating a whole channel's backlog as brand-new arrivals and sliding each one in.
|
||||
val arrivalTracker = remember(conversationKey) { MessageArrivalTracker() }
|
||||
val enteringIds = remember(conversationKey, messages.size, messages.lastOrNull()?.id) {
|
||||
arrivalTracker.arrivals(messages)
|
||||
}
|
||||
|
||||
@ -240,9 +266,9 @@ fun MessagesList(
|
||||
// growing a line — and animating those made the whole conversation lurch. So it is armed only
|
||||
// briefly around a genuine change to the list, and is otherwise off, letting items track the
|
||||
// viewport exactly.
|
||||
var placementArmed by remember { mutableStateOf(false) }
|
||||
var previousMessageCount by remember { mutableStateOf<Int?>(null) }
|
||||
LaunchedEffect(messages.size) {
|
||||
var placementArmed by remember(conversationKey) { mutableStateOf(false) }
|
||||
var previousMessageCount by remember(conversationKey) { mutableStateOf<Int?>(null) }
|
||||
LaunchedEffect(conversationKey, messages.size) {
|
||||
val previous = previousMessageCount
|
||||
previousMessageCount = messages.size
|
||||
// Skip the first composition: the list settling into its initial padding is not a change
|
||||
|
||||
@ -0,0 +1,49 @@
|
||||
package com.bitchat.android.ui
|
||||
|
||||
import com.bitchat.android.geohash.ChannelID
|
||||
import com.bitchat.android.geohash.GeohashChannel
|
||||
import com.bitchat.android.geohash.GeohashChannelLevel
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Test
|
||||
|
||||
class LocationChannelsSheetTest {
|
||||
|
||||
@Test
|
||||
fun `teleported channel gets a standalone row`() {
|
||||
val nearby = listOf(channel("u33dc"))
|
||||
val teleported = channel("dr5ru")
|
||||
|
||||
assertEquals(
|
||||
teleported,
|
||||
selectedLocationChannelOutsideNearby(ChannelID.Location(teleported), nearby)
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `nearby selected channel is not duplicated`() {
|
||||
val nearby = channel("u33dc")
|
||||
|
||||
assertNull(
|
||||
selectedLocationChannelOutsideNearby(
|
||||
ChannelID.Location(channel("U33DC")),
|
||||
listOf(nearby)
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `mesh selection has no standalone location row`() {
|
||||
assertNull(
|
||||
selectedLocationChannelOutsideNearby(
|
||||
ChannelID.Mesh,
|
||||
listOf(channel("u33dc"))
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
private fun channel(geohash: String) = GeohashChannel(
|
||||
level = GeohashChannelLevel.CITY,
|
||||
geohash = geohash
|
||||
)
|
||||
}
|
||||
@ -126,6 +126,51 @@ class MessageArrivalTrackerTest {
|
||||
assertEquals(setOf("a"), tracker.arrivals(listOf(msg("a"))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a wholesale replacement animates nothing`() {
|
||||
// Switching channels, or /clear followed by fresh content: the incoming list shares no ids
|
||||
// with the outgoing one, so it is a different conversation rather than a burst of arrivals.
|
||||
val tracker = MessageArrivalTracker()
|
||||
tracker.arrivals(listOf(msg("a"), msg("b")))
|
||||
|
||||
val other = listOf(msg("x"), msg("y"))
|
||||
assertTrue(
|
||||
"a different conversation must not slide every message in",
|
||||
tracker.arrivals(other).isEmpty()
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a small replacement below the burst cap still animates nothing`() {
|
||||
// The burst cap alone would not catch this: two messages is well under it.
|
||||
val tracker = MessageArrivalTracker()
|
||||
tracker.arrivals(listOf(msg("a"), msg("b"), msg("c")))
|
||||
|
||||
assertTrue(tracker.arrivals(listOf(msg("x"), msg("y"))).isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a replacement that overlaps is treated as normal arrivals`() {
|
||||
// Still the same conversation if anything carries over, so genuine new messages animate.
|
||||
val tracker = MessageArrivalTracker()
|
||||
val kept = msg("a")
|
||||
tracker.arrivals(listOf(kept, msg("b")))
|
||||
|
||||
assertEquals(setOf("c"), tracker.arrivals(listOf(kept, msg("c"))))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `after a wholesale replacement, later arrivals animate normally`() {
|
||||
val tracker = MessageArrivalTracker()
|
||||
tracker.arrivals(listOf(msg("a")))
|
||||
|
||||
val switched = mutableListOf(msg("x"))
|
||||
tracker.arrivals(switched) // adopted silently
|
||||
|
||||
switched += msg("y")
|
||||
assertEquals(setOf("y"), tracker.arrivals(switched))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the known set never outgrows the conversation`() {
|
||||
val tracker = MessageArrivalTracker()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user