Repository navigation
Deep Timeline x-axis labels, with the fine ticks kept off the Today card - #2520
Merged
Merged
Conversation
…imeline on Android and iOS/macOS
chartTimeTicks is shared. The Deep Timeline passes its pinch-zoom window, but Today's HR card passes the RENDERED extent of its banked buckets rather than a nominal window, so adding tiers below an hour reached Today as well: a morning holding ten minutes of HR drew ten 1-minute dotted gridlines on a small card. Labels are thinned by an overlap skip; the gridlines are not. The sub-hour tiers now sit behind deepZoom, off by default and set only by the surface that actually zooms. That also makes the retained comment true again, the one about thresholds sitting below the nominal Today windows so a window covering slightly less than nominal still lands on its intended interval. Swift keys that on zoomBounds rather than zoomDomain, for the reason OverviewHRChart already records twice: zoomBounds is set once by the zooming call sites, while zoomDomain stays nil until the first pinch, so keying on it would hand a not-yet-zoomed Deep Timeline the static tiers and then switch them under the user (#829). The Swift function now returns dates and no longer builds a DateFormatter per call. Every label it formatted was discarded by the caller's map, once per body evaluation, which during a pinch is once per frame. Its doc no longer claims the two platforms read identically: positions are shared, labels are not, because Swift draws them with AxisValueLabel() in the viewer's locale where the Kotlin side formats HH:mm itself. Tests: the three deep-zoom tiers now pass the flag, and both platforms pin the Today guarantee. ChartTimeTicksTests is new, because eight thresholds written twice in two languages drift, and a drift between them is invisible in a build.
8 of 12 tasks
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.
Summary
Stacked on @andigandhi's #2445. Both of their commits are preserved with their authorship and messages unchanged.
Their change gives the Deep Timeline x-axis labels and per-minute ticks at deep zoom, on Android and Apple, with the tick positions shared between the platforms and the gridlines and label strip driven from one tick list so they cannot drift. The Android half also pins three new tiers in
ChartTimeTicksTest.The follow-up commit
chartTimeTicksis shared with the Today HR card. The Deep Timeline hands it a pinch-zoom window, but Today hands itvisBuckets.first().bucket … visBuckets.last().bucket, the RENDERED extent of banked buckets rather than a nominal window. So tiers below an hour reached Today too: a morning holding ten minutes of HR drew ten 1-minute dotted gridlines on a small card. Labels are thinned by an overlap skip, the gridlines are not. The sub-hour tiers now sit behinddeepZoom, off by default and set only by the surface that zooms, which also makes the retained comment about thresholds sitting below the nominal Today windows true again.Swift keys that on
zoomBounds, notzoomDomain, for the reasonOverviewHRChartalready records twice:zoomBoundsis set once by the zooming call sites, whilezoomDomainstays nil until the first pinch, so keying on it would hand a not-yet-zoomed Deep Timeline the static tiers and then switch them under the user (#829).The Swift function formatted labels nothing read.
xTicksdiscarded every one of them with.map { $0.date }, while the function built a freshDateFormatterper call.xTicksis read insidebody, so during a pinch that is a formatter construction plus N string formats per frame, all thrown away. It now returns dates and formats nothing, and its doc no longer claims the platforms read identically: positions are shared, labels are not, since Swift draws them withAxisValueLabel()in the viewer's locale where Kotlin formatsHH:mmitself.Also worth stating plainly for whoever reads this later: because
OverviewHRChartis shared, this PR changes the Today HR card's Apple axis from.automatic(desiredCount: 5)to round wall-clock ticks. That is a deliberate keep, since Android's Today card already ticks this way, so it closes a platform difference rather than opening one.Verification
./gradlew :app:testFullDebugUnitTest --no-build-cache --rerun-tasks: 760 suites, 6479 tests, 0 failures, 0 errors.ChartTimeTicksTestis 13 of 13.python3 Tools/doc_comment_lint.py: no new detached doc comments.ChartTimeTicksTestsis new on the Swift side, pinning all eight tiers, the Today guarantee and an inverted window. Eight thresholds written twice in two languages drift, and a drift is invisible in a build. It runs on thetest (StrandDesign)leg.Refs #575