Skip to content

Sleep: show the duration trend as a bar chart (#85)#7

Closed
digitalerdude wants to merge 2 commits into
mainfrom
sleep-duration-bar-chart
Closed

Sleep: show the duration trend as a bar chart (#85)#7
digitalerdude wants to merge 2 commits into
mainfrom
sleep-duration-bar-chart

Conversation

@digitalerdude

Copy link
Copy Markdown
Owner

Summary

  • Addresses Bar diagram instead of graph for the Sleep duration ryanbr/noop#85.
  • The Sleep tab's "Hours asleep" trend now renders as a bar histogram instead of the line/area chart, matching the issue's mockup.
  • The shared TrendChart component already had a fully-built BarMark mode (showsBars) with a truthful zero-baseline Y-domain — it just wasn't turned on anywhere. Flipping it on for this one call site is fully isolated: no other TrendChart caller (XiaomiBandView, WorkoutDetailView, StressView ×2, AppleHealthView, MetricExplorerView, TrendsView) passes showsBars, so none of them are affected.
  • Also factors the bar-mode zero-floor domain logic — previously an inline, untested ternary in .chartYScale — into a named plotYDomain computed property, and adds test coverage for both the floored (bar) and unfloored (line) cases.

Test plan

  • swift test for the StrandDesign package: 32/32 tests pass, including 2 new ones (testTrendChartBarModeFloorsPlotDomainAtZero, testTrendChartLineModeDoesNotFloorPlotDomain).
  • macOS (Strand) and iOS (NOOPiOS) targets both build clean via xcodebuild.
  • Could not get a live visual check of the rendered bar chart in this environment (no GUI access) — worth a manual look before merge.

🤖 Generated with Claude Code

The "Hours asleep" trend used the shared TrendChart's default
line/area mode. TrendChart already has a fully-built BarMark mode
(showsBars) with a truthful zero-baseline Y-domain, just unused by
any call site — flip it on for this chart to match the requested
histogram look. No other TrendChart caller passes showsBars, so this
is isolated to the Sleep tab.

Also factors the bar-mode zero-floor domain logic (previously inline
in an untested `.chartYScale` ternary) into a named, tested
`plotYDomain` property.
@digitalerdude

Copy link
Copy Markdown
Owner Author

Closing this fork-internal PR to declutter — the real submission lives at ryanbr/noop (upstream). See that repo's PR list for the current, actionable version of this work.

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