Skip to content

feat(charts): drag-to-zoom, fullscreen expand, timezone-aware time controls#105

Open
JeremyFunk wants to merge 49 commits into
mainfrom
feat/chart-time-range-ux
Open

feat(charts): drag-to-zoom, fullscreen expand, timezone-aware time controls#105
JeremyFunk wants to merge 49 commits into
mainfrom
feat/chart-time-range-ux

Conversation

@JeremyFunk

Copy link
Copy Markdown
Collaborator

Time-range UX for the dashboard, service-detail, and home-overview charts. (Arrow-key keyboard navigation is split into a follow-up PR stacked on this one.)

What's included

  • Drag-to-zoom — drag across any time-series chart to narrow the window to that selection. Click vs drag is decided by pointer travel (≥6px), so a plain click never zooms. Shared useChartDragZoom hook drives all chart types; read-only history previews don't mutate the live range.
  • Expand to fullscreen — a maximize button (revealed on card hover) opens any chart in a near-fullscreen centered modal (ChartExpandModal). Also fixes the pre-existing widget-header hover-reveal (a group/card class was never declared, so actions only appeared after a click) and the action-button alignment.
  • Timezone selector — the time-range picker footer is now a searchable popover: search by zone name or UTC offset, "System" and "UTC" pinned first, sorted, with aligned offset columns.
  • Timezone-correct chart times — x-axis ticks, tooltips, and deploy-marker times render in the user's selected timezone (threaded through formatBucketLabel), not the browser's. Incoming buckets are normalized to UTC upstream, so re-projection is correct for any source offset.
  • Custom-range fix — selecting a custom range no longer jumps by the UTC offset (timezone-aware round-trip); a reversed start/end is ordered ascending.

Notes

  • Base PR of a stack; the keyboard navigation PR (arrow-key pan/zoom + F-to-expand) targets this branch.
  • Verified end-to-end in-browser across all three surfaces. bun typecheck at the branch baseline (web 23 pre-existing, ui 0); lint/format clean.

🤖 Generated with Claude Code

JeremyFunk and others added 18 commits June 20, 2026 00:41
Extracted to its own PR (fix/effect-workerd-clock-patch, #96). Restores the
effect@4.0.0-beta.70 patch to main's McpServer-only version.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extracted to its own PR (fix/worker-flush-telemetry-macrotask, #97). Reverts
the two flush call sites to telemetry.flush(env) and removes the
flushTelemetry helper. The VCS queue/scheduled handlers stay.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extracted to its own PR (fix/local-trace-quickfilters, #98). Reverts the HTTP
semconv coalescing / display-name-aware span-name filtering in traces-shared.ts,
query-helpers.ts, errors.ts and ch.test.ts to main's behavior. Unrelated
oxlint/series-cap changes in these files stay.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Cover previously-untested branches in the GitHub/VCS sync path:
- backfill value-correctness: sinceMs window, continuation carry-through,
  staleAttempts increment/reset, and the watermark-stall guard
- webhook signature rejection reasons (missing/malformed header, no secret)
- fetchBranches error propagation (transient/rate-limited/installation-gone)
- repository upsert-conflict reactivation, cross-org isolation, scope filtering
- retry-after HTTP-date / x-ratelimit-reset parsing; queue delay clamping

Make the recording queue apply clampQueueDelaySeconds so delay regressions are
observable in tests. Consolidate the two VcsSyncJob round-trip tests into one
union smoke test and trim duplicate assertions from the push/unsuspend tests.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts:
#	packages/query-engine/src/ch/queries/errors.ts
Repo-wide formatting normalization (line-width reflow); no behavior changes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…, fill test gaps

Review-driven follow-ups on the GitHub deactivated/suspended state work:

- Import GithubConnectionState from @maple/domain/http instead of
  hand-duplicating the literal union in GithubConnectService — removes the
  drift risk between the domain schema and the service return type.
- Wrap repositoriesOf in Effect.fn so its N+1 branch resolution carries a
  span (it was a bare Effect.gen returned from a plain function).
- Add the missing getStatus -> "suspended" assertion and parametrize the
  reconnect-reactivation orchestrator test over "updated" and "created"
  (the prior comment claimed both but only exercised "updated").

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts:
#	packages/domain/src/http/integrations.ts
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The release/deploy reference lines on the service metric charts (latency,
throughput, apdex, error rate) were bare dashed lines whose only commit info
was a plain "Deploy: <short-sha>" string in the chart tooltip. Wire them into
the VCS commit-sync feature so each marker now carries an interactive flag that
opens the rich CommitShaHoverCard — resolving the release's commit when the repo
is connected/synced, and falling back to the short SHA as plain text otherwise.

- packages/ui: add a shared renderReferenceLines() helper (replaces the block
  duplicated across all four charts) that renders an optional interactive marker
  at the top of each line via <foreignObject>, keeping the design-system package
  free of app-specific data fetching. ChartReferenceLine gains `sha`; BaseChartProps
  gains the `renderReferenceMarker` render-prop.
- apps/web: thread renderReferenceMarker through MetricsGrid and have the service
  detail page supply a CommitShaHoverCard keyed on the marker's full SHA.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Demo telemetry carried no deployment.commit_sha, so demo services showed no
deploy markers at all — and there was no way to populate vcs_commits locally,
so any marker's commit hover card would read "not found". Give the demo seed
coherent end-to-end data for reviewing the deploy-marker → commit-hover-card
feature:

- fixtures: rotate deployment.commit_sha across three full-SHA demo "releases"
  over the seeded window (one dominant baseline + two later deploys), so every
  demo service's charts surface deploy markers via detectReleaseMarkers.
- DemoService: seed a matching demo repo + vcs_commits (one per release) so the
  markers' hover cards resolve to real commit details via the stored fast path.
  Best-effort — a failure (e.g. un-migrated local D1) only costs the markers
  their resolved cards, never the telemetry seed. The demo installation is
  marked disconnected so it never competes with a real connected repo for the
  integration card's "active" slot.
- app: provide VcsRepository to DemoServiceLive (its Database requirement bubbles
  to MainLive like VcsDataLive; Layer memoization shares the one instance).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rged

The mark-every-release fix (#100) is now on main, so all three demo releases
surface as markers, not just the two non-baseline ones.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Removes the demo seeding added for reviewing the deploy-marker → commit-hover-card
feature: the rotating deployment.commit_sha on demo telemetry, the demo GitHub
installation/repo/vcs_commits seed in DemoService, and its VcsRepository wiring in
app.ts. The demo now generates plain telemetry again (no commit SHAs, no VCS rows).

The actual feature — the commit hover card on deploy markers — is unchanged; it
just relies on real connected-repo data rather than synthetic demo data.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ntrols

Adds time-range UX to the dashboard, service-detail, and home charts:

- Drag-to-zoom: drag across any time-series chart to narrow the window to
  that selection (click vs drag decided by pointer travel, so a click never
  zooms). Shared `useChartDragZoom` hook; read-only previews don't mutate.
- Expand to fullscreen: a maximize button (revealed on card hover) opens any
  chart in a near-fullscreen centered modal.
- Timezone selector: the time-range picker footer is a searchable popover
  (search by name or offset, System/UTC pinned, aligned offset columns).
- Timezone-correct chart times: x-axis ticks, tooltips, and deploy-marker
  times now render in the user's selected timezone, not the browser's.
- Custom-range fix: selecting a custom range no longer jumps by the UTC
  offset (timezone-aware round-trip); reversed start/end is ordered.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one minor suggestion inline.

Reviewed changes — the initial Review of the time-range UX work (drag-to-zoom, fullscreen expand, and timezone-aware time controls) across the dashboard, service-detail, and home-overview charts.

  • Add useChartDragZoom — a shared Recharts gesture hook in packages/ui driving drag-to-zoom on all 7 time-series charts, with a 6px click-vs-drag threshold and ascending-ordered onZoomSelect payloads.
  • Thread onZoomSelect end-to-end — new BaseChartProps.onZoomSelect, wired through chart-widget/metrics-grid into routes/index.tsx, routes/services/$serviceName.tsx, and dashboard-canvas (gated off in readOnly), with zoomRangeToWarehouse converting buckets and no-op'ing degenerate selections.
  • Add ChartExpandModal + renderExpanded — a near-fullscreen chart modal exposed via a render-prop on WidgetShell/WidgetFrame/MetricsGrid; also repairs the pre-existing group/card hover-reveal and action-button alignment.
  • Rebuild the timezone selectortimezone-display.tsx becomes a searchable popover (search by zone name or UTC offset, System/UTC pinned, offset-sorted) backed by formatZoneOffsetLabel/getZoneOffsetMinutes.
  • Timezone-correct labels and custom rangeformatBucketLabel gains a timeZone param threaded through every tick/tooltip/deploy-marker; custom-range-picker round-trips wall-clock ↔ UTC via new timezone-format helpers (zonedWallClockToUtc/utcToZonedWallClock), and $serviceName.tsx drops the "12h" preset default so the custom-range indicator lights up after a zoom.

The timezone math holds up under scrutiny: the single-pass zonedWallClockToUtc correction is correct for fractional-offset zones (+5:30, +5:45, +12:45/+13:45), and the only approximation is the unavoidable DST ambiguous/skipped hour, which is documented in-code. Bucket values reach formatBucketLabel as Z-suffixed ISO instants, so the toLocaleString({ timeZone }) re-projection does not double-shift on the live data path. The drag-zoom state machine is robust — leave/up double-commit is idempotent, synced charts don't cross-fire the gesture, and non-parseable labels degrade to a no-op. The presetValue default removal on the service-detail route is a deliberate, correct enabler for the custom-range indicator rather than a regression.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

)}
{canExpand && (
<ChartExpandModal open={expanded} onOpenChange={setExpanded} title={title}>
{renderExpanded()}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ renderExpanded() is invoked on every WidgetShell render, even while the modal is closed. The chart is only mounted when open (ChartExpandModal renders {open ? children : null}), but the element returned here is still allocated each render — which the renderExpanded JSDoc ("only mounted while the modal is open") implies should be lazy. The allocation is shallow (the discarded subtree isn't reconciled), so this is a minor polish item, not a correctness issue.

Technical details
# Eager `renderExpanded()` call defeats the documented laziness

## Affected sites
- `apps/web/src/components/dashboard-builder/widgets/widget-shell.tsx:212``{renderExpanded()}` passed as `children` to `ChartExpandModal`, evaluated every render.
- `apps/web/src/components/dashboard/metrics-grid.tsx``renderExpanded` is a new closure per render wrapping `<Suspense>{renderChart(true)}</Suspense>`, so the element tree is rebuilt on each `MetricsGrid` re-render (e.g. on syncId hover).

## Required outcome
- The expanded-chart element tree should not be constructed until the modal is open, matching the `WidgetShellProps.renderExpanded` contract.

## Suggested approach (optional)
Guard the call by `expanded` at the call site (`{expanded && renderExpanded()}`) or move the invocation inside `ChartExpandModal` so it runs only on the `open` branch.

Base automatically changed from feat/github-integration to main June 22, 2026 16:03
Resolved conflicts:
- Charts (apdex/error-rate/throughput area + latency line): kept this branch's
  drag-to-zoom + timeZone feature on top of main's renderReferenceLines/
  renderReferenceMarker refactor (HEAD was a strict superset).
- dashboard/metrics-grid: kept the renderChart() helper (already passes
  renderReferenceMarker + timeZone + onZoomSelect) over main's inline chart.
- services/$serviceName: kept onZoomSelect wiring into the metrics grid.
- api/alchemy.run.ts: took main's WorkerStub import + chat-flue stub binding.
- DB migrations: dropped this branch's 0026_vcs_integration.sql (byte-identical
  to main's 0027_late_otto_octavius); kept main's chain 0026_special_silver_surfer
  (OpenRouter drop) -> 0027_late_otto_octavius (VCS tables). Merged schema source
  matches (OpenRouter defs dropped, VCS tables present).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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