Skip to content

fix(platform-insights): Neutral chart color too light#95102

Merged
ArthurKnaus merged 1 commit into
masterfrom
aknaus/fix/platform-insights/neutral-chart-color-too-light
Jul 9, 2025
Merged

fix(platform-insights): Neutral chart color too light#95102
ArthurKnaus merged 1 commit into
masterfrom
aknaus/fix/platform-insights/neutral-chart-color-too-light

Conversation

@ArthurKnaus

Copy link
Copy Markdown
Member

In the new UI the color gray200 is much lighter than before and unfit for usage in chart. This PR adds a hook for calculating a neutral chart color until the new theme supports this natively.

Light mode

Screenshot 2025-07-09 at 12 01 18

Dark mode

Screenshot 2025-07-09 at 12 01 07

@ArthurKnaus ArthurKnaus requested a review from a team as a code owner July 9, 2025 10:16
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 9, 2025

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: Color Lightening Method Accepts Invalid Range

The Color.lighten() method is called with 1.3, which is outside the standard 0-1 range for color lightening. This can cause unexpected behavior, errors, or the color being clamped (e.g., to white), leading to inconsistent rendering across environments or with different Color library versions.

static/app/views/insights/pages/platform/shared/useNeutralChartColor.tsx#L17-L18

? Color(theme.gray400).darken(0.35)
: Color(theme.gray400).lighten(1.3);

Fix in CursorFix in Web


Was this report helpful? Give feedback by reacting with 👍 or 👎

@shellmayr shellmayr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good! Should we also use it in Laravel, Next.js and the generic backend view? (edit: ah I just realized the base widget takes care of that)

@ArthurKnaus ArthurKnaus merged commit e613b2d into master Jul 9, 2025
48 checks passed
@ArthurKnaus ArthurKnaus deleted the aknaus/fix/platform-insights/neutral-chart-color-too-light branch July 9, 2025 11:40
@JonasBa

JonasBa commented Jul 9, 2025

Copy link
Copy Markdown
Contributor

@matejminar @shellmayr @ArthurKnaus

Would you mind please surfacing these issues in ui2 or design engineering channels next time?

While the approach here is fine, we need to tackle these problems at a systematic level, else we'll just end up with different looking chart colors everywhere. A custom hook here is also the wrong approach, because we now have two different places where chart color definitions live (we want these to be centralized). The problem with hooks like these is that in the context of the Sentry codebase, they are virtually non discoverable, so nobody besides you will know they exist.

andrewshie-sentry pushed a commit that referenced this pull request Jul 14, 2025
In the new UI the color gray200 is much lighter than before and unfit
for usage in chart. This PR adds a hook for calculating a neutral chart
color until the new theme supports this natively.
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants