Give the three hero columns one width, and the label back the space it lost (#1502) - #1507
Merged
Merged
Conversation
…t lost (#1502) Two reports, one cause: "REST" ellipsised to "R..." under the Today hero, and the three rings did not look the same size. Charge and Effort sized their columns to their own label rows. Rest alone sat in a Box pinned to the ring diameter, because that box anchors the card's source badge - "the badge may grow leftward, but its trailing edge always matches the Rest vessel". So the three columns came out different widths, which reads as rings of different sizes even though one `ring` value feeds all three vessels, and Rest's label had the LEAST room of the three despite REST being the shortest of the three words. That is the tell: if the labels were simply too long, CHARGE and EFFORT would break first. All three columns now take one shared `col`, derived from the same maxWidth the ring is. The badge keeps the vessel-aligned trailing edge it exists for by insetting exactly the slack the wider column introduces. The label was also paying for a counterweight. #937 wanted the WORD centred on the ring's axis rather than the word-plus-chevron block, and balanced the row with an invisible LEADING twin of the chevron - correct, but it spent a second 14.dp slot plus its gap on nothing. Inside Rest's ring-width box that left the text barely 50.dp on a compact screen, which is where "REST" ran out of room at a larger font scale. Centring in a Box gets the same axis for free: the column is symmetric, so a centred word sits on the ring's centre by construction and the chevron can float at the trailing edge instead of being counterweighted. The word reserves 16.dp each side so it can never run under the chevron at any label length. Net on a 360.dp screen the label goes from 50.8.dp to 68.0.dp, +34%; on a 432.dp screen from 66.0 to 92.0. REST at a large font scale fits with room to spare. The tap target grows to the full column and stays one target; RTL still mirrors (Box alignments and the AutoMirrored icon both flip); TalkBack loses nothing - the invisible twin was deliberately description-less, so nothing was announced. Android only; iOS lays the hero out separately and UI parity is feature-level. Full suite 4173 tests 0 failures; doc lint and i18n --ci clean. NOT visually verified. I cannot run the app, the arithmetic above is the whole argument, and the truncation is font-scale dependent - the reporter's device settings would confirm it. Worth a look on a device before trusting it. Reported by @mailingjash.
The equal-column change had a crash in it. `col` is (maxWidth - 2*gap)/3 but `ring` carries a 90.dp floor, so once the hero is under about 298.dp wide the floor exceeds the column. The badge inset is (col - ring)/2, which then goes negative, and Modifier.padding throws on a negative value - taking out the Today screen rather than merely looking wrong. Reachable on a small phone, a split-screen pane, or a foldable's cover display: at 288.dp of hero width the inset is -1.7.dp. Ring is now capped at the column width, so a narrow hero shrinks its vessels with their columns instead of spilling out of them, and the inset is clamped at zero as a second line. Checked across 240-432.dp: the inset never goes negative, the ring never exceeds its column, and three columns plus two gaps still come to exactly maxWidth. Also moved `modifier` after the required parameters to match the rest of the file (TodayCardDismissButton and its neighbours); every call site names its arguments, so the order is free to fix. Full suite 4173 tests 0 failures; doc lint and i18n --ci clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1502 — "REST" showing as "R…" under the Today hero, and the three rings not looking the same size.
One cause, both symptoms
Charge and Effort size their columns to their own label rows. Rest alone sits in a box pinned to the
ring diameter, because that box anchors the card's source badge:
So the three columns come out different widths — which reads as rings of different sizes, even though a
single
ringvalue feeds all three vessels — and Rest's label has the least room of the three.The tell is that REST is the shortest of the three words. If the labels were simply too long for the
layout, CHARGE and EFFORT would break first.
Fix
All three columns now take one shared
col, derived from the samemaxWidththe ring is. The badge keepsthe vessel-aligned trailing edge it exists for, by insetting exactly the slack the wider column introduces.
The label was also paying for a counterweight. #937 wanted the word centred on the ring's axis rather
than the word-plus-chevron block, and balanced the row with an invisible leading twin of the chevron —
correct, but it spends a second 14.dp slot plus its gap on nothing. Inside Rest's ring-width box that left
the text barely 50.dp on a compact screen, which is where "REST" ran out of room at a larger font scale.
Centring in a
Boxgets the same axis for free: the column is symmetric, so a centred word sits on thering's centre by construction and the chevron floats at the trailing edge instead of being counterweighted.
The word reserves 16.dp each side so it can never run under the chevron at any label length.
The tap target grows to the full column and stays one target. RTL still mirrors (Box alignments and the
AutoMirroredicon both flip). TalkBack loses nothing — the invisible twin was deliberatelydescription-less, so it never announced anything.
Android only; iOS lays its hero out separately and UI parity is feature-level.
Narrow heroes
colis(maxWidth - 2*gap)/3butringcarries a 90.dp floor, so under ~298.dp of hero width the floorexceeds the column and the badge inset
(col - ring)/2goes negative — andModifier.paddingthrows ona negative value. That's a small phone, a split-screen pane, or a foldable's cover display; at 288.dp the
inset is −1.7.dp. Caught on re-review, before it went anywhere.
Ring is now capped at the column width — a narrow hero shrinks its vessels with their columns instead of
spilling out of them — and the inset is clamped at zero as a second line. Checked across 240–432.dp: the
inset never goes negative, the ring never exceeds its column, and three columns plus two gaps still come to
exactly
maxWidth.Verification
Full suite — 4,173 tests, 0 failures.
doc_comment_lintandi18n_audit --ciclean. No new strings.Not visually verified. I can't run the app; the arithmetic above is the whole argument. The truncation
is font-scale dependent, so it's worth a look on a device before trusting this — and @mailingjash's display
size / font size settings would confirm the diagnosis outright.
Reported by @mailingjash.