Skip to content

when selecting a range in the heatmap chart, selection not visible - #1384

Merged
kodiakhq[bot] merged 2 commits into
mainfrom
brandon/add-back-selection-ui
Nov 19, 2025
Merged

when selecting a range in the heatmap chart, selection not visible#1384
kodiakhq[bot] merged 2 commits into
mainfrom
brandon/add-back-selection-ui

Conversation

@brandon-pereira

@brandon-pereirabrandon-pereira commented Nov 19, 2025

Copy link
Copy Markdown
Member

This was a regression from #1347. This adds back the existing styles.

Screenshot 2025-11-19 at 9 53 27 AM

Fixes HDX-2863

@brandon-pereira
when selecting a range in the heatmap chart, the box was not visible …
176e467
…after the reskinning of the app. This adds back the existing styles.
Fixes HDX-2863
@brandon-pereira
brandon-pereira requested review from a team and pulpdrew and removed request for a teamNovember 19, 2025 16:58
@changeset-bot

changeset-botBot commented Nov 19, 2025•
edited
Loading

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 685bc04

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

This PR includes changesets to release 2 packages
NameType
@hyperdx/appPatch
@hyperdx/apiPatch

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

vercelBot commented Nov 19, 2025•
edited
Loading

Copy link
Copy Markdown

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

ProjectDeploymentPreviewCommentsUpdated (UTC)
hyperdx-v2-oss-appReadyReadyPreviewCommentNov 19, 2025 5:04pm

pulpdrew
pulpdrew previously approved these changes Nov 19, 2025
View reviewed changes
@claude

claudeBot commented Nov 19, 2025•
edited
Loading

Copy link
Copy Markdown

PR Review

Critical Issues

  • ⚠️ Hardcoded colors violate semantic token system (lines 303-305 in app.scss) → Replace with semantic tokens: #50fa7b → use var(--color-text-primary) or var(--mantine-color-green-4), #50fa7a0d → use var(--color-bg-highlighted) with opacity, #2E224788 → use var(--color-text-muted)

  • 🎨 No light mode support → Wrap in [data-mantine-color-scheme='dark'] selector and add light mode variant, OR use semantic tokens that adapt to both themes

  • 🔧 Unnecessary !important flag on background → Remove unless absolutely required (check specificity issue)

Context

PR #1347 just introduced semantic design tokens to ensure consistent theming. This CSS directly contradicts that effort by hardcoding colors that won't adapt to light mode and bypass the token system.

Suggested Fix

Use semantic tokens: border: 1px solid var(--color-text-primary); background: var(--color-bg-highlighted); color: var(--color-text-muted); OR scope appropriately for dark/light modes.

@github-actions

github-actionsBot commented Nov 19, 2025•
edited
Loading

Copy link
Copy Markdown
Contributor

E2E Test Results

✅ All tests passed • 38 passed • 3 skipped • 397s

StatusCount
✅ Passed38
❌ Failed0
⚠️ Flaky2
⏭️ Skipped3

View full report →

@kodiakhq
kodiakhqBot merged commit f868c3c into mainNov 19, 2025
10 checks passed
@kodiakhq
kodiakhqBot deleted the brandon/add-back-selection-ui branch November 19, 2025 17:05
knudtty pushed a commit that referenced this pull request Apr 16, 2026
@brandon-pereira
when selecting a range in the heatmap chart, selection not visible (#…
4d7f165
…1384)
This was a regression from #1347. This adds back the existing styles.
<img width="1210" height="548" alt="Screenshot 2025-11-19 at 9 53 27 AM" src="https://github.com/user-attachments/assets/52343706-2b92-4779-94c0-bdccb1668f09" />
Fixes HDX-2863
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pulpdrewpulpdrewpulpdrew approved these changes

Assignees

No one assigned

Labels

automerge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@brandon-pereira@pulpdrew