Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 55 additions & 16 deletions android/app/src/main/java/com/noop/ui/Charts.kt
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,15 @@ private fun pointsFor(
topPad: Float,
bottomPad: Float,
yDomain: ClosedFloatingPointRange<Double>? = null,
timestamps: List<Long>? = null,
): List<Offset> {
val clean = values.filter { it.isFinite() }
if (clean.size < 2 || width <= 0f || height <= 0f) return emptyList()
// A supplied domain never HIDES a reading: it widens to contain anything outside it, so anchoring can
// only ever change the scale, never clip a value off the chart.
val lo = yDomain?.let { minOf(it.start, clean.min()) } ?: clean.min()
val hi = yDomain?.let { maxOf(it.endInclusive, clean.max()) } ?: clean.max()
return pointsFor(clean, width, height, topPad, bottomPad, lo, hi)
return pointsFor(clean, width, height, topPad, bottomPad, timestamps, lo, hi)
}

private fun pointsFor(
Expand All @@ -127,17 +128,18 @@ private fun pointsFor(
height: Float,
topPad: Float,
bottomPad: Float,
timestamps: List<Long>?,
minV: Double,
maxV: Double,
): List<Offset> {
if (values.size < 2 || width <= 0f || height <= 0f) return emptyList()

val span = (maxV - minV)
val usableH = (height - topPad - bottomPad).coerceAtLeast(1f)
val stepX = if (values.size > 1) width / (values.size - 1) else width
val fractions = xFractions(values.size, timestamps)

return values.mapIndexed { i, v ->
val x = stepX * i
val x = fractions[i] * width
val norm = if (span > 0.0) ((v - minV) / span).toFloat() else 0.5f
val y = topPad + (1f - norm) * usableH
Offset(x, y)
Expand Down Expand Up @@ -283,14 +285,19 @@ fun LineChart(
else values.indices.filter { values[it].isFinite() }.map { segmentIds[it] }
}
var selectedIndex by remember(cleanValues) { mutableIntStateOf(-1) }
// Hoisted, not recomputed per gesture event: the drag handler fires every frame and this allocates a
// list the length of the series. Keyed on both inputs so the gesture handlers can be keyed on IT:
// keying them on `cleanValues` alone would let a timestamp change leave them hit-testing against
// stale positions, highlighting one day while labelling another.
val xFracs = remember(cleanValues, cleanTimestamps) { xFractions(cleanValues.size, cleanTimestamps) }
val interactiveModifier = if (selectionEnabled) {
Modifier
.pointerInput(cleanValues) {
.pointerInput(cleanValues, xFracs) {
detectTapGestures(
onTap = { offset ->
if (cleanValues.size >= 2 && size.width > 0) {
selectedIndex = nearestIndexForX(
count = cleanValues.size,
fractions = xFracs,
width = size.width.toFloat(),
x = offset.x,
)
Expand All @@ -300,20 +307,20 @@ fun LineChart(
}
.then(
if (dragSelectionEnabled) {
Modifier.pointerInput(cleanValues) {
Modifier.pointerInput(cleanValues, xFracs) {
detectHorizontalDragGestures(
onDragStart = { start ->
if (cleanValues.size < 2 || size.width <= 0f) return@detectHorizontalDragGestures
selectedIndex = nearestIndexForX(
count = cleanValues.size,
fractions = xFracs,
width = size.width.toFloat(),
x = start.x,
)
},
onHorizontalDrag = { change, _ ->
if (cleanValues.size < 2 || size.width <= 0f) return@detectHorizontalDragGestures
selectedIndex = nearestIndexForX(
count = cleanValues.size,
fractions = xFracs,
width = size.width.toFloat(),
x = change.position.x,
)
Expand Down Expand Up @@ -369,7 +376,7 @@ fun LineChart(
val strokePx = 2.5f
val topPad = strokePx + 4f
val bottomPad = strokePx + 4f
val pts = pointsFor(cleanValues, size.width, size.height, topPad, bottomPad, yDomain)
val pts = pointsFor(cleanValues, size.width, size.height, topPad, bottomPad, yDomain, cleanTimestamps)
if (pts.isEmpty()) {
onDrawBehind { drawBaseline() }
} else {
Expand Down Expand Up @@ -441,7 +448,7 @@ fun LineChart(
val strokePx = 2.5f
val topPad = strokePx + 4f
val bottomPad = strokePx + 4f
val pts = pointsFor(cleanValues, size.width, size.height, topPad, bottomPad, yDomain)
val pts = pointsFor(cleanValues, size.width, size.height, topPad, bottomPad, yDomain, cleanTimestamps)
if (selectedIndex in pts.indices) {
val p = pts[selectedIndex]
drawLine(
Expand Down Expand Up @@ -507,7 +514,9 @@ fun MultiLineChart(
val bottomPad = strokePx + 4f

cleanSeries.forEach { line ->
val pts = pointsFor(line.values, size.width, size.height, topPad, bottomPad, minV, maxV)
// MultiLineChart keeps index spacing: its series are index-aligned to each other, not to a
// shared clock, so positioning by time would need a per-series timeline it does not have.
val pts = pointsFor(line.values, size.width, size.height, topPad, bottomPad, null, minV, maxV)
if (pts.isEmpty()) return@forEach
val path = Path().apply {
moveTo(pts.first().x, pts.first().y)
Expand All @@ -522,12 +531,42 @@ fun MultiLineChart(
}
}

private fun nearestIndexForX(count: Int, width: Float, x: Float): Int {
if (count <= 1 || width <= 0f) return 0
val step = width / (count - 1)
/**
* Where each point sits horizontally, as a 0..1 fraction of the width.
*
* Index spacing puts every reading an equal step apart, so a four-day gap is drawn exactly like a one-day
* step. With per-point timestamps the position is proportional to TIME instead, which is what makes a gap
* occupy the width it actually spans, and what makes breaking the stroke across one read as "nothing
* measured here" rather than as a chopped line.
*
* Falls back to index spacing whenever time cannot order the points: no timestamps, a length mismatch, a
* zero span (every reading at the same instant), or a non-ascending sequence. Refusing rather than
* guessing, the same stance the rest of this file takes.
*/
internal fun xFractions(count: Int, timestamps: List<Long>?): List<Float> {
if (count <= 1) return List(count) { 0f }
val indexFractions = { List(count) { it.toFloat() / (count - 1) } }
if (timestamps == null || timestamps.size != count) return indexFractions()
if (timestamps.zipWithNext().any { (a, b) -> b < a }) return indexFractions()
val span = (timestamps.last() - timestamps.first()).toDouble()
if (span <= 0.0) return indexFractions()
return timestamps.map { ((it - timestamps.first()) / span).toFloat() }
}

/**
* The nearest point to a tapped x. Takes the SAME fractions the geometry used: deriving it from a uniform
* step independently would select the wrong reading the moment spacing stopped being uniform.
*/
private fun nearestIndexForX(fractions: List<Float>, width: Float, x: Float): Int {
if (fractions.size <= 1 || width <= 0f) return 0
val clampedX = x.coerceIn(0f, width)
val raw = (clampedX / step).roundToInt()
return raw.coerceIn(0, count - 1)
var best = 0
var bestDist = Float.MAX_VALUE
fractions.forEachIndexed { i, f ->
val d = kotlin.math.abs(f * width - clampedX)
if (d < bestDist) { bestDist = d; best = i }
}
return best
}

/** The tap/drag pinpoint label: the caller's display formatter when supplied (#463), else the raw
Expand Down
4 changes: 4 additions & 0 deletions android/app/src/main/java/com/noop/ui/HealthScreen.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2101,6 +2101,10 @@ fun VitalDetailScreen(vm: AppViewModel, key: String) {
// two are equal in length by construction rather than by luck — `LineChart` drops
// mismatched labels SILENTLY, which is a failure that looks exactly like doing nothing.
selectionLabels = dayLabels,
// Position by DATE, not by reading index: a four-day gap now occupies four days of
// width, which is what makes the break across it read as "nothing measured here"
// rather than as a chopped line. Days that do not parse fall back to index spacing.
timestamps = dayEpochSeconds(filteredReadings),
// #1662: the metric's OWN formatter AND unit — byte-for-byte what the Min/Avg/Max
// row below renders. Without it the scrub read-out falls back to LineChart's
// default, which prints a decimal for any non-integer, so a rounded metric answered
Expand Down
16 changes: 16 additions & 0 deletions android/app/src/main/java/com/noop/ui/HealthVitalDetailLogic.kt
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,22 @@ internal fun vitalChartYDomain(key: String): ClosedFloatingPointRange<Double>? =
else -> null
}

/**
* Per-reading epoch seconds from the day keys, or null when any day fails to parse.
*
* All-or-nothing on purpose: a partially-parsed list would position some points by time and the rest by a
* fallback, which is a worse lie than either rule applied consistently. Returning null makes the chart use
* index spacing, exactly as before.
*/
internal fun dayEpochSeconds(readings: List<VitalReading>): List<Long>? {
val out = ArrayList<Long>(readings.size)
for (r in readings) {
val day = runCatching { java.time.LocalDate.parse(r.day) }.getOrNull() ?: return null
out += day.toEpochDay() * 86_400L
}
return out
}

/**
* Is this metric EXPECTED to have a reading every day?
*
Expand Down
107 changes: 107 additions & 0 deletions android/app/src/test/java/com/noop/ui/ChartXSpacingTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
package com.noop.ui

import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test

/**
* Positioning by date is what makes breaking the line across a missing day mean anything. With index
* spacing a gap has ZERO width, so the break reads as a chopped line and an isolated day as an orphan dot
* floating in space, which is exactly what shipping the break without the spacing produced.
*/
class ChartXSpacingTest {

private fun day(s: String) = java.time.LocalDate.parse(s).toEpochDay() * 86_400L

/** No timestamps: every existing chart keeps the even spacing it has always had. */
@Test
fun `without timestamps the points stay evenly spaced`() {
assertEquals(listOf(0f, 0.5f, 1f), xFractions(3, null))
}

/**
* The reported shape. 3 Sep, 5 Sep, 6 Sep: the first gap is TWO days and the second is one, so the
* first must occupy twice the width. Under index spacing both were half the chart.
*/
@Test
fun `a two-day gap takes twice the width of a one-day step`() {
val f = xFractions(3, listOf(day("2026-09-03"), day("2026-09-05"), day("2026-09-06")))
assertEquals(0f, f[0], 1e-6f)
assertEquals(2f / 3f, f[1], 1e-6f) // two days of three
assertEquals(1f, f[2], 1e-6f)
}

/** Consecutive days are evenly spaced, so a fully-measured stretch is unchanged. */
@Test
fun `consecutive days are evenly spaced`() {
val f = xFractions(3, listOf(day("2026-09-01"), day("2026-09-02"), day("2026-09-03")))
assertEquals(listOf(0f, 0.5f, 1f), f)
}

// --- refusing rather than guessing ---

@Test
fun `a length mismatch falls back to index spacing`() {
assertEquals(listOf(0f, 0.5f, 1f), xFractions(3, listOf(day("2026-09-01"))))
}

/** Every reading at the same instant has no time order to use. */
@Test
fun `a zero span falls back to index spacing`() {
val t = day("2026-09-01")
assertEquals(listOf(0f, 0.5f, 1f), xFractions(3, listOf(t, t, t)))
}

/** Out-of-order timestamps would place points backwards; index spacing is the honest fallback. */
@Test
fun `a non-ascending sequence falls back to index spacing`() {
val f = xFractions(3, listOf(day("2026-09-03"), day("2026-09-01"), day("2026-09-05")))
assertEquals(listOf(0f, 0.5f, 1f), f)
}

@Test
fun `fractions always span the full width`() {
val f = xFractions(4, listOf(day("2026-09-01"), day("2026-09-04"), day("2026-09-05"), day("2026-09-09")))
assertEquals(0f, f.first(), 1e-6f)
assertEquals(1f, f.last(), 1e-6f)
assertTrue(f.zipWithNext().all { (a, b) -> b > a })
}

// --- the day-key conversion ---

/** All-or-nothing: one unparseable day makes the whole chart fall back rather than mixing rules. */
@Test
fun `an unparseable day disables date spacing entirely`() {
assertNull(
dayEpochSeconds(
listOf(VitalReading("2026-09-01", 1.0, "d"), VitalReading("nope", 2.0, "d")),
),
)
}

@Test
fun `parseable days convert to ascending epoch seconds`() {
val t = dayEpochSeconds(
listOf(VitalReading("2026-09-01", 1.0, "d"), VitalReading("2026-09-03", 2.0, "d")),
)
assertEquals(listOf(day("2026-09-01"), day("2026-09-03")), t)
}

/**
* The geometry and the hit-testing must agree, or the chart highlights one day and labels another.
* Both now derive from this one function, so pinning it pins the pairing: the nearest index to a
* point's own x is that point.
*/
@Test
fun `each point is the nearest index to its own position`() {
val ts = listOf(day("2026-08-26"), day("2026-09-01"), day("2026-09-05"), day("2026-09-08"))
val f = xFractions(4, ts)
val width = 1000f
f.forEachIndexed { i, frac ->
val x = frac * width
val nearest = f.withIndex().minByOrNull { kotlin.math.abs(it.value * width - x) }!!.index
assertEquals("point $i", i, nearest)
}
}
}