Skip to content

stress: give the marker tile's reading the width the chip was holding - #2172

Merged
ryanbr merged 2 commits into
mainfrom
fix/2145-marker-tile-clipping
Sep 13, 2026
Merged

ryanbr merged 2 commits into
mainfrom
fix/2145-marker-tile-clipping

Conversation

@ryanbr

@ryanbr ryanbr commented Sep 13, 2026 •

Copy link
Copy Markdown
Owner

What was wrong

Reported by @bartmuskala with a screenshot that placed it exactly. On the Stress screen the RESTING HR tile read 53 … beside a chip reading ▲ +7 vs …, and HRV read 31 … beside ▼ −29 v…. Both halves of the row cut, neither winning.

The two tiles on the same rows that carry no delta chip, STRESS and CALM TIME, render in full at the same width. That places it in the row that holds a value beside a chip rather than in the tile or the screen.

Why

MarkerTile asked for compactDelta, which splits that row evenly. On a two-up tile there is about 140dp to give, and +7 vs base alone wants about 90dp of it, so neither side fits in its half.

The even split was itself the previous repair. Before #492 item 5 the chip took its natural width and left the reading as 4…, so the row was set to share instead. Sharing starves both sides rather than one, because a weighted child with fill = true is held to exactly its share: the value sits at half however little the chip turns out to need. There was no width at which both strings fit, so the row could only pick which one to cut.

The change

The chip carries the signed magnitude and nothing else. The section header directly above these tiles already reads vs 30-day baseline, so vs base was being bought at the reading's expense to repeat something the reader had just been given. At 12sp with at most three digits the chip measures under 45dp including its pill padding, which leaves the value the rest of the row at full size. The even split then goes, and the natural-width path comes back.

at baseline becomes ±0 for the same reason. TrendChip shows no glyph for it, having one only for a signed value.

No caller passes compactDelta any more. Its documentation now says so, and says to reach for a shorter chip before reaching for it, since the workouts feed had already left it behind by going full width.

A second bug, not fixed here

The value is drawn by AutoSizeValue, whose stated job is to shrink to 0.6x rather than truncate, added for this exact symptom in #319/#332. 53 bpm at 0.6x would have fitted the half it was given. It truncated instead, so that safety net is not firing, and it sits under every StatTile in the app rather than only these two.

This PR does not depend on it: the reading now fits at full size with no shrink required. I have not chased the cause, because it wants a device to confirm and I cannot run Compose here. Filed separately as #2171.

Localisation

vs base and at baseline were English literals on Android while the Apple twin passes both through String(localized:). The change takes both out of the UI without needing a new string, since a signed number needs no translation.

Tools/i18n_audit.py never saw them: its count is 234 before and after. That is the known blind spot for literals assigned outside a composable call, so the audit passing was not evidence they were fine.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Documentation
  • CI / tooling

How it was tested

compileFullDebugKotlin clean, doc lint clean, i18n_audit.py unchanged at 234.

This is a layout change and wants eyes on a device, which is the honest state of it: there is no unit test that measures a Compose row, and the screenshot that reports it came from a phone I do not have. What to look at on the Stress screen: RESTING HR and HRV should read 53 bpm and 31 ms in full, with a short chip beside each, and should stay intact at larger font scales, where the old split failed first.

One trade-off worth stating

A screen reader used to hear +7 vs base and now hears +7. The tile label and the section header still carry the context, but the chip alone is terser than it was. Restoring it properly means a description string localised across the eight values* files rather than a literal, which is more churn than the two words are worth in this PR. Happy to add it if you would rather have it.

Related issues

#2145. #2171 is the shrink helper, which is the wider of the two.

Re-review (85cb48d)

The tile claimed a fact it did not have. rhrDelta is null when today has no reading or there is no 30-day baseline to stand it against:

val rhrDelta = if (rhrT != null && meanRHR != null) rhrT - meanRHR else null

Both of those fell through to the at-baseline chip, so a tile with nothing in it put that chip beside the empty-value dash, and a first-week tile put a reading exactly on a baseline that did not exist yet. Shortening the chip to ±0 sharpened the false claim rather than causing it, which is what made it visible on a second read.

A null delta now draws no chip at all, which both StatTiles already support: the Kotlin one renders the pill inside if (delta != null), the Swift one inside if let delta. The at-baseline branch is reached only with a real delta now, so it says "measured, and it is zero" rather than standing in for "nothing to measure".

The Swift twin had the identical flaw, so both are corrected together rather than leaving one side claiming it. This widens the PR past the Android layout it started as, deliberately: it is one defect in one tile with two implementations. iOS keeps its wording, not being the surface that clips, so the shortened chip stays Android only.

The first pass missed it because the reported screenshot came from a strap with a month of history behind it, where the delta is always present. The empty and first-week states are the ones nobody photographs.

Checked: parity ledger unmoved, Strand/** being outside the scanned globs. Kotlin compiles, doc lint clean. The Swift change is app-target, which does not build on this machine, so it rests on the macOS leg of CI. Adding a Swift file also moves this PR onto the larger check roster.

The Resting HR and HRV tiles clipped both halves of their row: "53 …" beside
"▲ +7 vs …" where the tile should read "53 bpm" and "▲ +7". The two tiles on
the same rows without a delta chip, Stress and Calm time, were never affected,
which is what places it in the row that holds a value beside a chip.

#492 item 5 had already been here. Before it the chip took its natural width,
about 90dp of a row with about 140dp to give, and left the reading as "4…", so
the row was set to split evenly instead. That starves both sides rather than
one: a weighted child with fill = true is held to exactly its share, so the
value sits at half however little the chip needs, and half is not enough for
either "53 bpm" or "+7 vs base".

The chip now carries the signed magnitude and nothing else. The section header
directly above it already reads "vs 30-day baseline", so the words were being
bought at the reading's expense to say something the reader had just been
given. At 12sp with at most three digits the chip measures under 45dp, which
leaves the value the rest of the row at full size, so the even split goes and
the natural-width path comes back.

"at baseline" becomes "±0" for the same reason, and TrendChip shows no glyph
for it, having one only for a signed value.

Both strings were English literals on Android while the Apple twin passes both
through String(localized:), so this also takes two untranslated words out of
the UI without needing a new string. Tools/i18n_audit.py never saw them, its
count being 234 before and after, which is the known blind spot for literals
assigned outside a composable call.

No caller passes compactDelta now. Its documentation says so, and says to
reach for a shorter chip first.
Re-review. The tile claimed a fact it did not have. rhrDelta is null when
today has no reading or there is no 30-day baseline to stand it against, and
both fell through to the at-baseline chip, so a tile with nothing in it
put the at-baseline chip beside the empty-value dash, and a first-week tile put a reading exactly on a baseline that
did not exist yet.

Null now draws no chip at all, which both StatTiles already support: the
Kotlin one renders the pill inside `if (delta != null)` and the Swift one
inside `if let delta`. The at-baseline branch is reached only with a real
delta now, so it says "measured, and it is zero" rather than standing in for
"nothing to measure".

The same flaw was in the Swift twin, so both are done together rather than
leaving one side claiming it. iOS keeps its wording, not being the surface
that clips; the shortened chip stays Android only.

Not caught by the previous pass because the screenshot came from a strap with
a month of history behind it, where the delta is always present.
@ryanbr
ryanbr merged commit 77480be into main Sep 13, 2026
5 checks passed
@ryanbr
ryanbr deleted the fix/2145-marker-tile-clipping branch September 13, 2026 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant