fix(cw): follow the transcript reliably, and keep the AMSAT grid dense
Two corrections to10c415faand0889a3bd, keeping what those got right and undoing what they cost. The transcript now follows new text through an explicit follow flag rather than comparing scroll position against maxValue. maxValue is written during layout, after the composition that would read it, so the comparison tested the previous frame's height: following fell progressively short of the true bottom and, once the gap passed the slack, latched the operator out of follow-mode until they hit the exact end. Scrolling away still stops it, which is the point. The AMSAT day cell goes back to 28 dp. Raising it to 48 dp for the minimum touch target measured a 71% increase in row pitch - 14 satellites per screen down to 8 on a 6.1" phone - and comparing many satellites at a glance is what that page is for. Compose cannot extend a touch target past the layout bounds, so this is a choice rather than a fix; 28 dp is also what shipped before, so the regression was mine. The contentDescription added alongside it stays, since it costs nothing.
This commit is contained in:
1 parent
b6753a4fa6
commit
96bbb022e8
2 files changed
+42
-8
No files matched your search
@@ -52,6 +52,7 @@ import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.runtime.snapshotFlow
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.draw.clip
|
||||
@@ -65,8 +66,16 @@ import androidx.compose.ui.unit.sp
|
||||
import androidx.core.content.ContextCompat
|
||||
import com.rtbishop.look4sat.core.domain.cw.CwToneShifter
|
||||
import com.rtbishop.look4sat.core.domain.repository.IContainerProvider
|
||||
import kotlin.math.roundToInt
|
||||
import com.rtbishop.look4sat.core.presentation.R as CoreR
|
||||
import kotlin.math.roundToInt
|
||||
|
||||
/**
|
||||
* Scroll slack, in pixels, within which the transcript counts as being at the bottom.
|
||||
*
|
||||
* Not zero: an animated scroll settles a pixel or two short of the maximum, and an exact
|
||||
* comparison would drop out of follow-mode the moment it did.
|
||||
*/
|
||||
private const val AUTOSCROLL_SLACK_PX = 4
|
||||
|
||||
/**
|
||||
* Full-page CW decoder backed by DeepCW.
|
||||
@@ -222,11 +231,35 @@ fun CwDecodeScreen() {
|
||||
.clip(RoundedCornerShape(8.dp))
|
||||
.background(MaterialTheme.colorScheme.surfaceVariant.copy(alpha = 0.4f))
|
||||
) {
|
||||
val transcript = (historyText + decodedText).ifEmpty { "…" }
|
||||
val scroll = rememberScrollState()
|
||||
// Follow the newest text, but stop as soon as the operator scrolls away, so
|
||||
// reading back over earlier traffic is not undone by the next decode.
|
||||
//
|
||||
// A boolean rather than comparing position against maxValue: maxValue is
|
||||
// written during layout, after the composition that would read it, so such a
|
||||
// comparison tests the previous frame's height and drifts short of the true
|
||||
// bottom until it latches out of follow-mode altogether.
|
||||
var following by remember { mutableStateOf(true) }
|
||||
LaunchedEffect(scroll) {
|
||||
snapshotFlow { scroll.isScrollInProgress to scroll.value }
|
||||
.collect { (scrolling, value) ->
|
||||
if (scrolling) following = value >= scroll.maxValue - AUTOSCROLL_SLACK_PX
|
||||
}
|
||||
}
|
||||
LaunchedEffect(transcript, following) {
|
||||
if (!following) return@LaunchedEffect
|
||||
// Twice: the first pass lands at the height known when it started, the
|
||||
// second covers growth that arrived while it was animating.
|
||||
repeat(2) {
|
||||
if (scroll.value < scroll.maxValue) scroll.animateScrollTo(scroll.maxValue)
|
||||
}
|
||||
}
|
||||
Text(
|
||||
text = (historyText + decodedText).ifEmpty { "…" },
|
||||
text = transcript,
|
||||
modifier = Modifier
|
||||
.fillMaxSize()
|
||||
.verticalScroll(rememberScrollState())
|
||||
.verticalScroll(scroll)
|
||||
.padding(8.dp),
|
||||
fontSize = 16.sp,
|
||||
fontFamily = FontFamily.Monospace,
|
||||
|
||||
+6
-5
@@ -355,19 +355,20 @@ private fun DayCell(day: SatDay, stripes: Boolean, modifier: Modifier, onClick:
|
||||
val description = stringResource(
|
||||
R.string.amsat_day_desc, day.dateLabel, stringResource(statusLabel(worst)), total
|
||||
)
|
||||
// Two layers: the tap target is 48 dp to meet the minimum, while the coloured part
|
||||
// stays 28 dp so the grid keeps its density. The extra height is transparent padding,
|
||||
// which is why the row spacing does not change.
|
||||
// 28 dp, below the 48 dp minimum touch target. Raising it to 48 dp measured a 71%
|
||||
// increase in row pitch - 14 satellites per screen down to 8 on a 6.1" phone - and
|
||||
// comparing many satellites at a glance is what this page is for. Compose cannot
|
||||
// extend a touch target past the layout bounds, so the two cannot both be had here.
|
||||
Box(
|
||||
modifier = modifier
|
||||
.heightIn(min = 48.dp)
|
||||
.height(28.dp)
|
||||
.semantics(mergeDescendants = true) { contentDescription = description }
|
||||
.clickable(onClick = onClick),
|
||||
contentAlignment = Alignment.Center
|
||||
) {
|
||||
val tile = Modifier
|
||||
.fillMaxWidth()
|
||||
.height(28.dp)
|
||||
.fillMaxHeight()
|
||||
.clip(RoundedCornerShape(4.dp))
|
||||
|
||||
if (stripes) {
|
||||
|
||||
Reference in new issue
Block a user