Skip to content

fix(search): make excluded filter pills readable in the light theme#2478

Merged
kodiakhq[bot] merged 2 commits into
mainfrom
alex/fix-exclude-filter-pill-contrast
Jun 17, 2026
Merged

fix(search): make excluded filter pills readable in the light theme#2478
kodiakhq[bot] merged 2 commits into
mainfrom
alex/fix-exclude-filter-pill-contrast

Conversation

@alex-fedotyev

@alex-fedotyev alex-fedotyev commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Excluded ("!=") filter pills above the search results were drawn with a saturated red background plus red text and a red remove button. In the light theme that meant the != operator, the value, and the x were red on red and effectively unreadable. This swaps the excluded pill to Mantine's red-light variant: a soft red tint with its paired readable accent.

Summary

  • ActiveFilterPills excluded pills now use --mantine-color-red-light for the background and --mantine-color-red-light-color for the != operator and the remove x, instead of --color-bg-danger / --color-text-danger.
  • The change is local to the pill, so the shared --color-bg-danger / --color-text-danger tokens (also used by GroupTabBar, the SQL editor border, the filter panel label, and dashboards) are unchanged.
  • Included pills stay neutral and the inactive/not-applied pill keeps its strikethrough and dashed border, so the three states stay distinct. An exclude still reads as an exclude (red hue), not as a disabled filter.

Why red-light

red-light / red-light-color is Mantine's scheme-aware light variant (autoContrast is on), already used here for destructive buttons and menu items. It is a guaranteed-legible background and text pair, so one local change fixes both light and dark, and both the hyperdx and clickstack themes.

Verification

Rendered the pill states in Storybook and read the computed colors of the excluded pill in both themes:

  • Light, before: background rgb(224,49,49), remove button rgb(224,49,49) (identical, invisible)
  • Light, after: background rgb(255,227,227), accent rgb(201,42,42) (clear contrast)
  • Dark, after: background rgb(101,21,21), accent rgb(255,245,245)

Before/after screenshots in light and dark are attached below.

image

after

image

Test plan

  • eslint (changed files) clean
  • yarn tsc --noEmit
  • jest ActiveFilterPills (27 passing, including a new assertion that the excluded pill uses the soft red-light background and the included pill stays neutral)
  • Storybook: pill states checked in light and dark, hyperdx and clickstack

Excluded ("!=") filter pills layered --color-text-danger text and remove
button on a --color-bg-danger background. In the light theme both tokens
resolve to red-8, so the operator, value, and "x" were red on red and
unreadable (dark theme was weak too). Switch the excluded pill to Mantine's
red-light variant (a soft tint with its paired readable accent color), which
is scheme-aware and legible in both themes. The change is local to the pill,
so the shared danger tokens used by other components are untouched, and the
included and inactive pill styles stay distinct.

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

changeset-bot Bot commented Jun 17, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 36bdd55

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 Jun 17, 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, Comment Jun 17, 2026 1:51am
hyperdx-storybook Ready Ready Preview, Comment Jun 17, 2026 1:51am

Request Review

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

github-actions Bot commented Jun 17, 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: 1
  • Production lines changed: 10 (+ 29 in test files, excluded from tier calculation)
  • Branch: alex/fix-exclude-filter-pill-contrast
  • Author: alex-fedotyev

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 Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a readability problem where excluded (!=) filter pills rendered saturated red text on an identical red background in the light theme, making the operator, value, and remove button effectively invisible. The fix swaps three inline CSS variable references in FilterPill to Mantine's scheme-aware red-light pair.

  • ActiveFilterPills.tsx: backgroundColor for excluded pills changes from --color-bg-danger to --mantine-color-red-light; the != operator Text and the ActionIcon remove button both change their color override from --color-text-danger to --mantine-color-red-light-color. The shared danger tokens used elsewhere in the codebase are untouched.
  • ActiveFilterPills.test.tsx: New test asserts on raw getAttribute('style') strings to verify the correct CSS variable names, avoiding the jsdom toHaveStyle no-op issue flagged in the previous review.

Confidence Score: 5/5

Safe to merge — the change is three CSS variable token swaps isolated to excluded filter pills, with no logic or data-flow modifications.

The change is purely presentational: three inline style string replacements inside a single render path. The shared --color-bg-danger / --color-text-danger tokens used elsewhere in the codebase are untouched. The new test correctly uses getAttribute('style') string matching to avoid jsdom CSS-variable resolution issues, making the assertions meaningful rather than no-ops.

No files require special attention.

Important Files Changed

Filename Overview
packages/app/src/components/ActiveFilterPills.tsx Swaps three CSS variable references for excluded pills: background from --color-bg-danger → --mantine-color-red-light and text/icon color from --color-text-danger → --mantine-color-red-light-color. No logic changes.
packages/app/src/components/tests/ActiveFilterPills.test.tsx Adds a new styling assertion test that uses getAttribute('style') string matching (not toHaveStyle) to verify the correct CSS variable tokens are applied to excluded vs. included pills, fully addressing the previous review comment.
.changeset/fix-exclude-filter-pill-contrast.md Standard patch-level changeset entry for the contrast fix — correctly scoped to @hyperdx/app.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[FilterPill rendered] --> B{isInvalid?}
    B -- yes --> C[transparent bg\ndashed border\n0.75 opacity\nstrikethrough text]
    B -- no --> D{isExcluded?}
    D -- yes --> E[--mantine-color-red-light bg\n--mantine-color-red-light-color\nfor operator & remove icon]
    D -- no --> F[--color-bg-hover bg\ndimmed operator color]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[FilterPill rendered] --> B{isInvalid?}
    B -- yes --> C[transparent bg\ndashed border\n0.75 opacity\nstrikethrough text]
    B -- no --> D{isExcluded?}
    D -- yes --> E[--mantine-color-red-light bg\n--mantine-color-red-light-color\nfor operator & remove icon]
    D -- no --> F[--color-bg-hover bg\ndimmed operator color]
Loading

Reviews (2): Last reviewed commit: "test(search): assert excluded-pill token..." | Re-trigger Greptile

Comment thread packages/app/src/components/__tests__/ActiveFilterPills.test.tsx Outdated
@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 200 passed • 3 skipped • 1246s

Status Count
✅ Passed 200
❌ Failed 0
⚠️ Flaky 3
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

Address review feedback on the new test: check the excluded pill's
background and remove-button accent tokens by inspecting the raw inline
style string instead of toHaveStyle, so the assertion stays meaningful
regardless of how the jsdom version handles unresolved CSS custom
properties. Also covers the accent token, not just the background.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@alex-fedotyev alex-fedotyev self-assigned this Jun 17, 2026
@kodiakhq kodiakhq Bot merged commit 052315b into main Jun 17, 2026
22 checks passed
@kodiakhq kodiakhq Bot deleted the alex/fix-exclude-filter-pill-contrast branch June 17, 2026 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge 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