nik/bitchat-android
1
0
Fork 0
mirror of https://github.com/permissionlesstech/bitchat-android.git synced 2026-08-15 06:56:30 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix layout jumpiness in chat and geohash people list

Browse Source 
Three separate causes of things moving when they should not:

- Chat lurched whenever a bottom sheet closed. Placement animation is meant
  to soften insertions and removals, but any relayout moves every item --
  a sheet's text field opening the keyboard changes the chat's IME inset,
  and closing it changes it back. Placement animation is now armed only
  briefly around a real change to the message list, so items otherwise
  track the viewport exactly.

- Anon list changed height as participants churned. Rows sized to their
  content, so any reorder could change the card's height; and the card
  sized to the live anon count, which moves constantly in a busy geohash.
  Rows now have an exact height, and a trimmed anon card reserves the full
  capped height regardless of how many are present beyond the cap.

- Anons are now their own trailing section rather than a tail on each of
  "on location" and "teleported in", which had pushed the few recognisable
  names out of view twice over. Self is never grouped as an anon.

Adds 7 tests covering the sectioning and the fixed-length behaviour.
This commit is contained in:
callebtc 2026-07-27 10:09:37 +02:00
parent 0cb28a4f3a
commit 9703ebfcaf
5 changed files with 191 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
app/src/main/java/com/bitchat/android/ui/AboutSections.kt
View File

@ -73,6 +73,15 @@ internal val SheetRowLeadingSlot = 22.dp
internal val SheetRowLeadingGutter = 16.dp
internal val SheetRowHorizontal = 16.dp
internal val SheetRowVertical = 13.dp
/**
* Exact height of a people-list row.
*
* Fixed rather than derived from content: these lists reorder themselves constantly, and a row
* whose height depends on its content makes the whole card change height every time the order
* changes. Equals the leading glyph plus [SheetRowVertical] above and below.
*/
internal val SheetRowHeight = SheetRowLeadingSlot + SheetRowVertical * 2
internal val SheetRowDividerInset = SheetRowHorizontal + SheetRowLeadingSlot + SheetRowLeadingGutter
/** Selection indicator sized for [SheetRowLeadingSlot]. */
internal val SheetRowSelectedDot = 12.dp

129
app/src/main/java/com/bitchat/android/ui/GeohashPeopleList.kt
View File

@ -3,6 +3,7 @@ package com.bitchat.android.ui
import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.filled.Email
import androidx.compose.material.icons.outlined.Explore
import androidx.compose.material.icons.outlined.Person
import androidx.compose.material.icons.outlined.LocationOn
import android.util.Log
import androidx.compose.animation.AnimatedVisibility
@ -125,7 +126,15 @@ fun GeohashPeopleList(
viewModel.isPersonTeleported(person.id)
}
val (teleportedPeople, localPeople) = orderedPeople.partition { personTeleported(it) }
// Anonymous participants form their own trailing section rather than a tail on each
// of the others. A busy geohash is mostly anons, and splitting them across "on
// location" and "teleported in" pushed the few recognisable names out of view twice
// over. Self is never grouped as an anon even when unnamed — you always want to find
// yourself where you actually are.
val isSelf: (GeoPerson) -> Boolean = { myHex != null && it.id == myHex }
val namedPeople = orderedPeople.filter { isSelf(it) || !it.isAnonymous() }
val anonPeople = orderedPeople.filter { !isSelf(it) && it.isAnonymous() }
val (teleportedPeople, localPeople) = namedPeople.partition { personTeleported(it) }
@Composable
fun personRow(person: GeoPerson) {
@ -164,6 +173,17 @@ fun GeohashPeopleList(
)
PeopleCard(people = teleportedPeople, row = { personRow(it) })
}
if (anonPeople.isNotEmpty()) {
SheetIconSectionHeader(
icon = Icons.Outlined.Person,
title = stringResource(R.string.section_anonymous),
modifier = Modifier.padding(
top = if (localPeople.isNotEmpty() || teleportedPeople.isNotEmpty()) 20.dp else 0.dp
)
)
PeopleCard(people = anonPeople, capped = true, row = { personRow(it) })
}
}
}
}
@ -183,26 +203,29 @@ internal fun GeoPerson.isAnonymous(): Boolean {
}
/**
* One grouped card of people, with a cap on how many anonymous participants are shown.
* One grouped card of people.
*
* A popular geohash can hold dozens of anons, which pushed everyone worth recognising off screen and
* turned the sheet into a wall of near-identical rows. Named participants are always listed in full;
* anons are trimmed to [MaxVisibleAnons], and the overflow is collapsed behind a count. The last
* visible anon fades out under a gradient so the truncation is legible as truncation rather than
* looking like the list simply ended.
* When [capped] the list is trimmed to [MaxVisibleAnons] rows and the remainder is collapsed behind
* a count. That matters for the anonymous section: a popular geohash can hold dozens of anons, which
* pushed everyone worth recognising off screen and turned the sheet into a wall of near-identical
* rows.
*
* The capped card is a **fixed height** — [MaxVisibleAnons] rows plus the overflow line, always,
* regardless of how many anons are currently present beyond the cap. Anons join and leave a busy
* geohash constantly, and sizing to the live count made the card grow and shrink under the reader
* every few seconds.
*/
@Composable
private fun PeopleCard(
people: List<GeoPerson>,
row: @Composable (GeoPerson) -> Unit
row: @Composable (GeoPerson) -> Unit,
capped: Boolean = false
) {
val palette = LocalBitchatPalette.current
val named = people.filterNot { it.isAnonymous() }
val anons = people.filter { it.isAnonymous() }
val visibleAnons = anons.take(MaxVisibleAnons)
val hiddenAnonCount = anons.size - visibleAnons.size
val visible = named + visibleAnons
val visible = if (capped) people.take(MaxVisibleAnons) else people
val hiddenCount = people.size - visible.size
val isTrimmed = capped && people.size > MaxVisibleAnons
Surface(
modifier = Modifier
@ -213,50 +236,57 @@ private fun PeopleCard(
shape = AboutCardShape
) {
Column {
AnimatedRowColumn(items = visible, key = { it.id }) { index, person ->
Column {
if (index > 0) SheetCardDivider()
if (hiddenAnonCount > 0 && index == visible.lastIndex) {
// Fade only the final row, so the gradient reads as "the list continues"
// rather than dimming content that is still meant to be read.
Box {
row(person)
Box(
modifier = Modifier
.matchParentSize()
.background(
Brush.verticalGradient(
listOf(
palette.surface.copy(alpha = 0f),
palette.surface.copy(alpha = 0.85f)
Box(
// Reserve the full capped height up front so the card cannot resize as anons
// churn. Rows are a fixed height, so this is exact rather than an estimate.
modifier = if (isTrimmed) {
Modifier.height(SheetRowHeight * MaxVisibleAnons)
} else {
Modifier
}
) {
AnimatedRowColumn(items = visible, key = { it.id }) { index, person ->
Column {
if (index > 0) SheetCardDivider()
if (isTrimmed && index == visible.lastIndex) {
// Fade only the final row, so the gradient reads as "the list
// continues" rather than dimming content still meant to be read.
Box {
row(person)
Box(
modifier = Modifier
.matchParentSize()
.background(
Brush.verticalGradient(
listOf(
palette.surface.copy(alpha = 0f),
palette.surface.copy(alpha = 0.85f)
)
)
)
)
)
)
}
} else {
row(person)
}
} else {
row(person)
}
}
}
AnimatedVisibility(
visible = hiddenAnonCount > 0,
enter = fadeIn(tween(BitchatMotion.STANDARD_MS)),
exit = fadeOut(tween(BitchatMotion.QUICK_MS))
) {
Text(
text = stringResource(R.string.people_n_more, hiddenAnonCount),
if (isTrimmed) {
// Always laid out when trimmed, so the count changing never moves anything.
// Only the number itself animates.
AnimatedCountLabel(
count = hiddenCount,
text = stringResource(R.string.people_n_more, hiddenCount),
fontFamily = FontFamily.Monospace,
fontSize = 11.sp,
color = palette.textTertiary,
modifier = Modifier
.fillMaxWidth()
.padding(
start = SheetRowHorizontal,
end = SheetRowHorizontal,
bottom = SheetRowVertical
)
modifier = Modifier.padding(
start = SheetRowHorizontal,
end = SheetRowHorizontal,
bottom = SheetRowVertical
)
)
}
}
@ -298,8 +328,11 @@ private fun GeohashPersonItem(
Row(
modifier = Modifier
.fillMaxWidth()
// Exact height, not padding: a row that sizes to its content makes the card change
// height whenever the list reorders.
.height(SheetRowHeight)
.clickable(onClick = onTap)
.padding(horizontal = SheetRowHorizontal, vertical = SheetRowVertical),
.padding(horizontal = SheetRowHorizontal),
verticalAlignment = Alignment.CenterVertically
) {
Box(

29
app/src/main/java/com/bitchat/android/ui/MessageComponents.kt
View File

@ -72,6 +72,7 @@ import com.bitchat.android.ui.theme.BitchatMotion
import com.bitchat.android.ui.theme.LocalBitchatPalette
import com.bitchat.android.ui.theme.MessageBodyTextStyle
import com.bitchat.android.ui.theme.MessageSenderTextStyle
import kotlinx.coroutines.delay
import java.text.SimpleDateFormat
import java.util.Locale
@ -106,6 +107,14 @@ private val MessagePlacementSpec: FiniteAnimationSpec<IntOffset> = spring(
/** Removals are not worth dwelling on. */
private val MessageFadeOutSpec: FiniteAnimationSpec<Float> = tween(BitchatMotion.QUICK_MS)
/**
* How long placement animation stays armed after the list gains or loses a message.
*
* Comfortably longer than [MessagePlacementSpec] takes to settle, so an arrival's push is never cut
* short.
*/
private const val PlacementArmWindowMs = 600L
/**
* Above this many simultaneous arrivals, entry animations are skipped.
*
@ -226,6 +235,24 @@ fun MessagesList(
arrivalTracker.arrivals(messages)
}
// Placement animation exists to soften insertions and removals. But *any* relayout moves every
// item — the keyboard opening behind a bottom sheet, that sheet closing again, the composer
// 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) {
val previous = previousMessageCount
previousMessageCount = messages.size
// Skip the first composition: the list settling into its initial padding is not a change
// worth animating.
if (previous == null || previous == messages.size) return@LaunchedEffect
placementArmed = true
delay(PlacementArmWindowMs)
placementArmed = false
}
val layoutDirection = LocalLayoutDirection.current
LazyColumn(
state = listState,
@ -294,7 +321,7 @@ fun MessagesList(
// Entry fade is handled by entryModifier, together with the slide, so the
// two cannot drift out of step.
fadeInSpec = null,
placementSpec = MessagePlacementSpec,
placementSpec = if (placementArmed) MessagePlacementSpec else null,
fadeOutSpec = MessageFadeOutSpec
)
.then(entryModifier)

1
app/src/main/res/values/strings.xml
View File

@ -316,6 +316,7 @@
<string name="people_count_title">People (%1$d)</string>
<string name="section_on_location">On location</string>
<string name="section_teleported_in">Teleported in</string>
<string name="section_anonymous">Anonymous</string>
<string name="people_n_more">%1$d more\u2026</string>
<string name="grant_location_permission">Grant location permission</string>

72
app/src/test/java/com/bitchat/android/ui/GeohashAnonOrderingTest.kt
View File

@ -102,6 +102,78 @@ class GeohashAnonOrderingTest {
assertEquals(12 - MaxVisibleAnons, anons.size - visible.size)
}
// MARK: - Sectioning
/** Mirrors the grouping in GeohashPeopleList: self is never treated as an anon. */
private fun sections(people: List<GeoPerson>, myId: String?): Triple<List<String>, List<String>, List<String>> {
val isSelf: (GeoPerson) -> Boolean = { myId != null && it.id == myId }
val named = people.filter { isSelf(it) || !it.isAnonymous() }
val anons = people.filter { !isSelf(it) && it.isAnonymous() }
return Triple(
named.map { it.displayName },
anons.map { it.displayName },
people.map { it.displayName }
)
}
@Test
fun `anons are grouped out of the named sections entirely`() {
val people = listOf(person("alice"), person("anon1"), person("bob"), person("anon2"))
val (named, anons, _) = sections(people, myId = null)
assertEquals(listOf("alice", "bob"), named)
assertEquals(listOf("anon1", "anon2"), anons)
}
@Test
fun `self stays in the named sections even when unnamed`() {
// You always want to find yourself where you actually are, not buried in the anon section.
val me = person("anon")
val people = listOf(me, person("alice"), person("anon2"))
val (named, anons, _) = sections(people, myId = me.id)
assertTrue("self must not be grouped as an anon", named.contains("anon"))
assertFalse(anons.contains("anon"))
assertEquals(listOf("anon2"), anons)
}
@Test
fun `a list of only anons yields no named section`() {
val people = (1..4).map { person("anon$it") }
val (named, anons, _) = sections(people, myId = null)
assertTrue(named.isEmpty())
assertEquals(4, anons.size)
}
// MARK: - Stable length
@Test
fun `a trimmed anon list always renders exactly the cap`() {
// The reserved height is MaxVisibleAnons rows whenever trimmed, so the card cannot resize
// as anons churn above the cap.
for (total in listOf(MaxVisibleAnons + 1, MaxVisibleAnons + 7, MaxVisibleAnons + 40)) {
val anons = (1..total).map { person("anon$it") }
val visible = anons.take(MaxVisibleAnons)
assertEquals(
"row count must not depend on how many anons are present beyond the cap",
MaxVisibleAnons,
visible.size
)
assertEquals(total - MaxVisibleAnons, anons.size - visible.size)
}
}
@Test
fun `reordering never changes how many rows are rendered`() {
val anons = (1..9).map { person("anon$it", secondsAgo = it.toLong()) }
val byRecency = anons.sortedByDescending { it.lastSeen }.take(MaxVisibleAnons)
val reversed = anons.sortedBy { it.lastSeen }.take(MaxVisibleAnons)
assertEquals(byRecency.size, reversed.size)
assertEquals(MaxVisibleAnons, byRecency.size)
}
@Test
fun `a short anon list is shown in full with nothing hidden`() {
val people = listOf(person("alice"), person("anon1"), person("anon2"))