Skip to content

fix(app): use categorical palette on histogram charts - #2949

Merged
elizabetdev merged 4 commits into
mainfrom
agent/histogram-chart-categorical-color
Aug 20, 2026
Merged

fix(app): use categorical palette on histogram charts#2949
elizabetdev merged 4 commits into
mainfrom
agent/histogram-chart-categorical-color

Conversation

@elizabetdev

@elizabetdev elizabetdev commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Histogram charts (including Request Latency on the Services dashboard) now use the categorical palette and the shared chart tooltip instead of a leftover neon green fill and a one-off tooltip.

Bars were hardcoded to #50FA7B, which is not in the chart palette. The tooltip also painted "Number of events" in that same color. They now resolve chart-blue through getColorFromCSSToken and render with ChartTooltipContainer / ChartTooltipItem, matching line, bar, and pie charts.

The tooltip's unused View events link is gone. generateSearchUrl was accepted on the inner histogram/tooltip but never passed from DBHistogramChart, so the link never appeared in production. The only caller (Services → HTTP Request Latency) has no search-URL builder for a duration bucket. Wiring a working link would be a separate feature (filter events to that latency range); until then the prop was dead code.

image

Test plan

  • Open Services → HTTP, switch Request Latency to histogram, and confirm bars are chart-blue (#437eef), not neon green
  • Hover a bucket: tooltip should match other charts (bordered surface, square swatch, formatted count)
  • Confirm the tooltip does not show a View events link
  • Click a bar to pin the tooltip, Esc to unpin
  • yarn workspace @hyperdx/app jest src/components/__tests__/DBHistogramChart.test.tsx

Compound Engineering
Cursor_Grok_4.6

Histogram bars used a hardcoded neon green outside the chart palette, and a one-off tooltip that inherited that color. Align both with other charts.

Co-authored-by: Cursor <cursoragent@cursor.com>
@changeset-bot

changeset-bot Bot commented Aug 20, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: cddd1fd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@hyperdx/app Patch
@hyperdx/api Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Aug 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview Aug 20, 2026 1:42pm
hyperdx-storybook Ready Ready Preview Aug 20, 2026 1:42pm

Request Review

@github-actions github-actions Bot added the review/tier-2 Low risk — AI review + quick human skim label Aug 20, 2026
@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

🔵 Tier 2 — Low Risk

Small, isolated change with no API route or data model modifications.

Why this tier:

  • Standard feature/fix — introduces new logic or modifies core functionality

Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns.
SLA: Resolve within 4 business hours.

Stats
  • Production files changed: 2
  • Production lines changed: 99 (+ 81 in test files, excluded from tier calculation)
  • Branch: agent/histogram-chart-categorical-color
  • Author: elizabetdev

To override this classification, remove the review/tier-2 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@greptile-apps

greptile-apps Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Histogram bars now use the categorical chart-blue token and render values through the shared chart tooltip components.

  • Replaces the hardcoded neon-green histogram fill with the first categorical hue.
  • Migrates histogram tooltip content and integer formatting to shared chart components.
  • Adds focused tooltip and color-resolution tests plus an application changeset.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the histogram now resolves chart-blue through the required color reader, addressing the prior review finding.

Important Files Changed

Filename Overview
packages/app/src/components/DBHistogramChart.tsx Resolves the histogram color through the chart token reader and adopts the shared tooltip; the previously reported direct palette access is fixed.
packages/app/src/components/tests/DBHistogramChart.test.tsx Adds coverage for categorical color resolution, shared tooltip rendering, formatting, fallback color, and inactive states.
packages/app/src/components/charts/ChartTooltip.tsx Adds a test identifier to the shared tooltip container without changing runtime behavior.
.changeset/histogram-chart-categorical-color.md Records the user-visible histogram palette and tooltip update as an application patch.

Reviews (4): Last reviewed commit: "Merge branch 'main' into agent/histogram..." | Re-trigger Greptile

Comment thread packages/app/src/components/DBHistogramChart.tsx
@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Deep Review

Scope: PR #2949 — a small, frontend-only change swapping the histogram chart's hardcoded #50FA7B bar fill for the categorical chart-blue palette token and replacing a one-off tooltip with the shared ChartTooltipContainer / ChartTooltipItem. Verified independently: the removed generateSearchUrl prop and "View Events" link were already dead code (the default export never passed them), so their removal is not a user-facing regression; and getColorFromCSSToken('chart-blue') short-circuits categorical tokens to a static hex lookup with no DOM access, so the module-load-time constant is safe (no SSR/crash concern).

✅ No critical issues found. No P0/P1 findings. The change is well-scoped and the new tooltip behavior (active/inactive, empty, undefined, and missing-color payloads) is directly tested.

🟡 P2 — recommended

  • packages/app/src/components/__tests__/DBHistogramChart.test.tsx:151 — the PR's headline behavior (bars render in chart-blue, not neon green) is only asserted via the exported HISTOGRAM_BAR_COLOR constant; no test renders the chart and confirms the <Bar> element actually consumes that constant as its fill.
    • Fix: add a render-level assertion that the rendered bar receives fill={HISTOGRAM_BAR_COLOR} so a future revert of the fill wiring is caught.
    • testing, correctness
🔵 P3 nitpicks (2)
  • packages/app/src/components/DBHistogramChart.tsx:29HISTOGRAM_BAR_COLOR is frozen at module load, which is safe today only because categorical tokens resolve from a static table; if the DOM-read path is ever reintroduced in getColorFromCSSToken, this constant would silently keep the import-time value.

    • Fix: add a short inline comment noting the categorical short-circuit is what makes the module-scope call safe, so future readers do not need to trace into utils.ts.
    • maintainability, kieran-typescript
  • packages/app/src/components/DBHistogramChart.tsx:182payload.map(...) with the key={p.name ?? index} fallback and per-item color fallback is only exercised with a single-item payload; the multi-item branch is untested (though a histogram is single-series in practice).

    • Fix: add a multi-item payload test if segmented histogram tooltips are ever expected, otherwise leave as-is.

Reviewers (5): correctness, testing, maintainability, project-standards, kieran-typescript.

Testing gaps: No test confirms the removed "View Events" Link/anchor is fully absent from the rendered tooltip footer; zero-count and fractional bucket-height formatting are not asserted.

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 305 passed • 1 skipped • 1181s

Status Count
✅ Passed 305
❌ Failed 0
⚠️ Flaky 2
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

Indexing COLORS skipped getColorFromCSSToken, so palette reorders
or future per-theme categorical overrides would miss this histogram.

Co-authored-by: Cursor <cursoragent@cursor.com>
Cover the color fallback and empty-payload tooltip branches, pin
the expected hex, and drop the unused View events link that was
never wired through DBHistogramChart.

Co-authored-by: Cursor <cursoragent@cursor.com>
@elizabetdev
elizabetdev merged commit 47fe0cd into main Aug 20, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants